[2.1] record.rid returning an object instead of proper RID

[dmarcelino]

Hi guys,

I've tried to add 2.1-SNAPSHOT to waterline-orientdb builds and it breaks some of my tests. I've looked deeper into it and it seems that OrientDB 2.1 is returning different things than 2.0.2 in certain conditions.

For a SELECT that fetches a vertex linked to another through an edge and using a fetch plan, I get the following.

1.7.10 and 2.0.2 return:

[ { '@type': 'd',
    rid: { cluster: 11, position: 0 },
    name: 'Luca',
    out_Eat: 
     { '@type': 'd',
       '@class': 'Eat',
       someProperty: 'something',
       out: [Object],
       in: [Object],
       '@rid': [Object] },
    '@rid': { cluster: -2, position: 1 } } ]

2.1 returns:

[ { '@type': 'd',
    rid: 
     { '@type': 'd',
       '@class': 'Person',
       name: 'Luca',
       out_Eat: [Object],
       '@rid': [Object] },
    name: 'Luca',
    out_Eat: [ [Object] ],
    '@rid': { cluster: -2, position: 0 } } ]

The test in 17e368 passes on 1.7.10 and 2.0.2 but fails on 2.1.

[phpnode]

@dmarcelino could be an Oriento bug, suspect that rid is being replaced with its target because it's not called @rid

[dmarcelino]

@phpnode, does Oriento run different code between OrientDB v2.0.2 and v2.1? I assumed the protocol didn't change... I've tested with both Oriento master and latest npm versions.

[phpnode]

@dmarcelino no it's the same code for both, probably something has changed but use the result of db.exec(yourQuery) instead of db.query(...) to check as it doesn't do any transformations.

[dmarcelino]

Indeed OrientDB is returning something different. After trying .exec():

1.7.10

{ status: { code: 0, sessionId: 43 },
  results: 
   [ { type: 'p',
       content: 
        { classId: 0,
          type: 'd',
          cluster: 13,
          position: 0,
          version: 1,
          value: 
           { '@type': 'd',
             '@class': 'Eat',
             someProperty: 'something',
             out: [Object],
             in: [Object] } } },
     { type: 'p',
       content: 
        { classId: 0,
          type: 'd',
          cluster: 12,
          position: 0,
          version: 2,
          value: 
           { '@type': 'd',
             '@class': 'Restaurant',
             name: 'Dante',
             type: 'Pizza',
             in_Eat: [Object] } } },
     { type: 'r',
       content: 
        { classId: 0,
          type: 'd',
          cluster: -2,
          position: 1,
          version: 0,
          value: { '@type': 'd', rid: [Object], name: 'Luca', out_Eat: [Object] } } },
     payloadStatus: 0 ] }

vs. 2.1

{ status: { code: 0, sessionId: 2 },
  results: 
   [ { type: 'p',
       content: 
        { classId: 0,
          type: 'd',
          cluster: 13,
          position: 0,
          version: 3,
          value: 
           { '@type': 'd',
             '@class': 'Eat',
             someProperty: 'something',
             out: [Object],
             in: [Object] } } },
     { type: 'p',
       content: 
        { classId: 0,
          type: 'd',
          cluster: 11,
          position: 0,
          version: 2,
          value: 
           { '@type': 'd',
             '@class': 'Person',
             name: 'Luca',
             out_Eat: [Object] } } },
     { type: 'p',
       content: 
        { classId: 0,
          type: 'd',
          cluster: 12,
          position: 0,
          version: 2,
          value: 
           { '@type': 'd',
             '@class': 'Restaurant',
             name: 'Dante',
             type: 'Pizza',
             in_Eat: [Object] } } },
     { type: 'r',
       content: 
        { classId: 0,
          type: 'd',
          cluster: -2,
          position: 0,
          version: 0,
          value: { '@type': 'd', rid: [Object], name: 'Luca', out_Eat: [Object] } } },
     payloadStatus: 0 ] }

Notice that there is one extra type: 'p' record, namely the one with name: 'Luca'.

Also, in both cases the rid within the record type: 'r' was rid: { cluster: 11, position: 0 }

[dmarcelino]

@phpnode, regarding that extra 'p' record, is that something Oriento should handle or is OrientDB sending extraneous data given the same data already exists the 'r' record?

     { type: 'p',
       content: 
        { classId: 0,
          type: 'd',
          cluster: 11,
          position: 0,
          version: 2,
          value: 
           { '@type': 'd',
             '@class': 'Person',
             name: 'Luca',
             out_Eat: [Object] } } },

[phpnode]

@dmarcelino I think it's an OrientDB bug, it shouldn't prefetch a record that exists in the main result set.

[dmarcelino]

Makes sense, thanks.

[lvca]

What does mean [Object] ?

[dmarcelino]

Hi @lvca, by default node's util.inspect() only expands objects until a certain depth to avoid putting out too much output. [Object] means the referenced object is beyond that depth and because of that it's not expanded.

[dmarcelino]

@lvca, this looks like a regression, can we add it to the 2.1 milestone please?

[dmarcelino]

@lvca, sorry for insisting, but given v2.1 is not out yet and given this is a regression introduced in v2.1 I believe it's best to fix this issue before the release. This has the potential to impact anyone running fetch plans with oriento and by extension it will break class associations in waterline-orientdb.

@phpnode, what are your thoughts on this?

[lvca]

@tglman oriento is based on this and 2.1 breaks compatibility with oriento. Could you please take a look at this soon?

[dmarcelino]

Thanks @lvca, @tglman.

@tglman, the key issue is that OrientDB is sending and extraneous prefetched record (type: 'p') equal to a record existing in the main result set (type: 'r' record).

[tglman]

Hi @dmarcelino,

We have done some fix in the fetch plan for 2.1 that may changes some behaviour,for the specific case is possible that the record with name 'Luca' will be returned as type='p' because some other record refer to it, and the fact that is there a result(the type 'r' ) that came from the same record it's not wrong because is just a projection of that record, and not the whole record.

by the way can you post here the fetch plan that is causing this, we have done a lot of fix since 2.0.

[dmarcelino]

Hi @tglman, my full test is in dmarcelino/oriento@17e3682, and the query/fetch plan is:

this.db.query('SELECT @rid, name, out_Eat from Person WHERE name = "Luca"', { fetchPlan: 'out_Eat:1 out_Eat.in:1' }).all()

So it sounds like OrientDB is slightly changing its API for 2.1 in what concerns returned records.

[dmarcelino]

@tglman, if I understand correctly there is no relevant difference between the "Luca" type: 'r' and type: 'p' records. So this means that for a fetch plan like this OrientDB is effectively sending duplicate data. If this is just one operation done occasionally that doesn't sound too bad but if we are talking of thousands of fetch plans on a busy server we are potentially talking of a lot of redundant data sending across the wire consuming extra bandwidth. Maybe I'm misunderstanding something but this does sound like an ideal solution.

[tglman]

i just written the same test case in java and it's seems correct it just send one the result plus the 'Eat' and 'Resturant' record, i'm going to try to run your node.js test and i'll let you know

[tglman]

hi,

I checked out the https://github.com/dmarcelino/oriento on the branch 3619-rid_object and i tried to run the npm test, but it look like the test for reproduce the problem is not running, i'm not the most expert of nodejs, am i doing something wrong ?

[dmarcelino]

That's weird... I can see it when I run npm test. Funny enough now it seems to pass:

  Bug #3619: returning rid as object
    Query a graph with fetchplan
      ✓ should return valid @rid with @this.toJSON(fetchPlan) 
people: [ { '@type': 'd',
    rid: { cluster: 11, position: 0 },
    name: 'Luca',
    out_Eat: [ [Object] ],
    '@rid': { cluster: -2, position: 0 } } ]
      ✓ should "rid" with proper RecordID when running query 
      ✓ should "rid" with proper RecordID 

I wonder if some other change (in OrientDB or Oriento) fixed this.

Actually here is a travis build where you can see the same:
https://travis-ci.org/dmarcelino/oriento/jobs/59267838#L1082

[tglman]

ok, closing it.
reopen it if the issue return

[dmarcelino]

Thanks @tglman, I'll need to check what else happened with 2.1 because sails-orientdb tests still break: https://travis-ci.org/appscot/sails-orientdb/builds/58795321

Anyway I hope you still add the java test you've created to test suite as that will be a good way to prevent regressions.

[lvca]

I guess @tglman forgot to close it... Closing it.