jadell / neo4jphp

PHP wrapper of the Neo4j REST interface
Other
532 stars 137 forks source link

Batch addLabels support #95

Closed mcuadros closed 10 years ago

mcuadros commented 11 years ago

Hi,

Many thanks for this excellent library, is impressive the pattern used on it, i like it a lot.

Today i was implementing in my code the batch-mode and i saw that addLabels doent works under it. So this is my pr:

I added all the unit test required and i test it in real implementation.

I hope you like it. If you need any change just tell me.

Regards and thanks

mcuadros commented 11 years ago

Hi,

I have another possible pr, you want wait for the review of that? or i send it to you too?

https://github.com/mcuadros/neo4jphp/commits/get_or_create

Is related to uniqueness indexes.

Regards

jadell commented 11 years ago

I won't be adding support for index uniqueness over REST. See my comment here for an explanation. https://github.com/jadell/neo4jphp/pull/74#issuecomment-25665276

For this PR, I have been contemplating marking batches as deprecated for a future release of neo4jphp. Batch functionality is complex, error-prone, and is redundant now that the Cypher transaction endpoint is available (I'll be merging support for transactions into master as soon as I finish writing documentation.)

The reason the current add/remove labels commands use Cypher instead of the REST endpoints is to get around this bug https://github.com/neo4j/neo4j/issues/1279.

I'd prefer not to put any more work into batches if it can be avoided. Is there a reason you are using batches in Neo4j 2.0 instead of transactional Cypher?

mcuadros commented 11 years ago

Oh i see, is my fault, i am very new with neo4j.

About this PR i was working in a massive load and make in batch looks like the only method to make it in a fast way. And without labels was unusable for me.

If you make with cypher you will need to change the implementation of all commands. Right?

Regards

jadell commented 11 years ago

Most of the commands will probably become Cypher in the future. For now, there are some things that are easier to do in Cypher and some that are easier to do with REST. In general, Neo4j is moving towards using Cypher for all data manipulation tasks.

mcuadros commented 11 years ago

But in the meanwhile not exists a method to make a batch process right? Because if you want create a method in the new method you must create by hand the create query. Right?

I can help you with this?

fractalien commented 10 years ago

Has there ever been a solution to this? If there is no intention to complete the batch-mode functionality because it's considered doomed, how are efficient mass insertions and updates to be performed?

jadell commented 10 years ago

Check out using Cypher transactions.

fractalien commented 10 years ago

I have tried them, but wrapping commands in a transaction does not prevent them from being executed one at a time way before committing. Collecting them in an array and passing that to addStatements(), however, does the trick – thanks! The Neo4j documentation could be clearer about this, though.