orientechnologies / PhpOrient

PhpOrient - PHP binary client for OrientDB
Other
107 stars 13 forks source link

Keep getting the wrong $cluster from $record->getRid() #5

Closed nightowl77 closed 9 years ago

nightowl77 commented 9 years ago

Hi Guys

I'm not sure if my OrientDB is not compatible, but I keep getting a clusterID of -2 for a record that should have a clusterID of 12.

I checked, it happens right in Operations.php. It is misreading the raw protocol data.

My OrientDB version is OrientDB Server v2.0.3

Any guidance on how I could fix that would be appreciated.

Thank you

Ostico commented 9 years ago

Can you provide a snippet of your code to reproduce the issue? Tnx

nightowl77 commented 9 years ago

I'm working on a client's database and he's pretty paranoid about sharing his idea, so I cannot share test data but I did manage to write you a test against a database we all have. During my test writing I've found the problem. This happens when you place properties in the SQL query. For example, this won't work

select song_type, name from V 

but this will

select  from V 

Against the gratefuldead database, that actually throws an error.

If I run the code below against my client's database, I get "-2" as a clusterID for all queries against all classes.

<?php
use PhpOrient\PhpOrient;

// Use composer autoloader to load vendor classes
require_once __DIR__ . '/../vendor/autoload.php';

$client = new PhpOrient('localhost', 2424);
$client->username = 'root';
$client->password = '********';
$client->connect();

$client->dbOpen( 'GratefulDeadConcerts', 'admin', 'admin' );

$records = $client->query( 'select song_type, name from V ' );

foreach ($records as $rec)
{
    echo "Cluster: ".$rec->getRid()->cluster  . "<br>\n";   

}

The error this will produce is:

Catchable fatal error: Argument 1 passed to PhpOrient\Protocols\Binary\Data\Record::setOData() must be of the type array, null given, called in /mnt/workspace/Sites/Phalcon//vendor/ostico/phporient/src/PhpOrient/Protocols/Common/ConfigurableTrait.php on line 18 and defined in /mnt/workspace/Sites/Phalcon//vendor/ostico/phporient/src/PhpOrient/Protocols/Binary/Data/Record.php on line 110 

Furthermore, in trying to hunt down the problem I also noticed that compser installed https://github.com/Ostico/PhpOrient.git and not this repository, but when I checked the composer file of this repo, it again referenced https://github.com/Ostico/PhpOrient.git. I'm now a bit confused as to what the "official" repo should be.

For now this is not a big issue, if anyone battles with this, they can just remove the properties from the queries.

Ostico commented 9 years ago

Thank you, i fixed now.

For your information, https://github.com/Ostico/PhpOrient.git is the main upstream repo, but orientechnologies gave to the developers the master access to the "official" cloned repository of the drivers, so we can track issues from both.

My repositories are every time in up-to-date status so you can clone from both.

nightowl77 commented 9 years ago

Thank you so much Ostico!

nightowl77 commented 9 years ago

Hi Ostico

Please excuse me if I'm being an idiot, but it seems from your tests that you are indeed expecting $rid->cluster to be -2.

On line 216 of your test SQLCommandsTest.php you AssertEquals -2 on cluster. Is this an intended design feature that I somehow missed in OrientDB tutorials?

 public function testWrongClusterID(){
$client = new PhpOrient('localhost', 2424);
$client->dbOpen( 'GratefulDeadConcerts', 'admin', 'admin' );
$records = $client->query( 'select song_type, name from V ' );
/**
* @var Record[] $records
*/
foreach ($records as $k => $rec) {
  if ( $client->getTransport()->getProtocolVersion() < 26 )
    $this->assertEquals( $k +1, $rec->getRid()->position );
  else
    $this->assertEquals( $k, $rec->getRid()->position );
  $this->assertEquals( -2, $rec->getRid()->cluster );
  }
}

I have a situation where I need the correct ClusterID and the only way around that is to manually include @rid in the select statement, like this:

select @rid, song_type, name from V

Is this they correct way?

Ostico commented 9 years ago

Yes, As far as i know this is the right way. By the way, if you need the right rid you can make the query without explicit fields, i think you could be interested to this discussion: https://github.com/orientechnologies/orientdb/issues/3468

Let me know.