Use uniqueness constraint for KG writer temp ids - #554
Conversation
AmirLayegh
left a comment
AmirLayegh
left a comment
There was a problem hiding this comment.
Thanks for the contribution! I left one comment that may be worth addressing before merging.
Also, would this fix be worth adding to CHANGELOG.md under Fixed? It changes the handling of duplicate temporary node IDs and fixes #159.
Could you also please sign the Contributor License Agreement as advised here?
| """) | ||
| self.driver.execute_query( | ||
| "DROP INDEX __entity__tmp_internal_id IF EXISTS", | ||
| database_=self.neo4j_database, |
There was a problem hiding this comment.
Should _db_cleaning() target self.neo4j_database as well? _db_setup(), _upsert_nodes() and _upsert_relationships() all explicitly use it, but _db_cleaning() opens self.driver.session() without specifying a database, which would use the user's home database.
If neo4j_database is different from the home database, the temporary IDs would be written to the target DB but cleaned from the home DB. With this uniqueness constraint, a subsequent write with the same temporary IDs will fail.
There was a problem hiding this comment.
Hey! Yes, you're right, I have pushed a fix to open the cleanup session with database=self.neo4j_database (Also rebased onto latest main to clear merge conflict). Thanks!
_db_cleaning opened a session without a database, so cleanup ran against the user's home database while _db_setup and the upserts targeted self.neo4j_database. When neo4j_database differed from the home database, __tmp_internal_id values were never cleared from the target DB and the new uniqueness constraint made subsequent writes fail. Pass database=self.neo4j_database to session(). Also adds the missing CHANGELOG entries for the constraint change (neo4j#159) and this fix.
31ad7c8 to
8592f38
Compare
2 participants
Description
Fixes #159
Replaces the KG writer’s temporary
__KGBuilder__.__tmp_internal_idrange index with a uniqueness constraint.Neo4jWriteruses__tmp_internal_idto match nodes when creating relationships during graph writes. A plain index improves lookup performance, but does not prevent duplicate temporary ids. The new uniqueness constraint preserves indexed lookup behavior while also enforcing that temporary ids are unique during the write process.This uses a uniqueness constraint rather than a node key constraint because
__tmp_internal_idis removed during writer cleanup.Type of Change
Complexity
Complexity: Low
How Has This Been Tested?
Checklist
The following requirements should have been met (depending on the changes in the branch):