Neo4j: Update with non-deprecated cypher methods, and new method to associate relationship embeddings

[RafaelXokito]

Description: At the moment neo4j wrapper is using setVectorProperty, which is deprecated (link). I replaced with the non-deprecated version.

Neo4j recently introduced a new cypher method to associate embeddings into relations using "setRelationshipVectorProperty" method. In this PR I also implemented a new method to perform this association maintaining the same format used in the "add_embeddings" method which is used to associate embeddings into Nodes.
I also included a test case for this new method.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
langchain	⬜️ Ignored (Inspect)	Visit Preview		Jul 17, 2024 9:05am

[RafaelXokito]

@ccurme Please review this PR.

[tomasonjo]

What's the reason for adding relationship embedding method if it isn't used anywhere? We also dont plan it

[RafaelXokito]

Thank you for your feedback, @tomasonjo.

The introduction of the new method for creating relationship embeddings in langchain is essential to keep pace with Neo4j's latest features. This enhancement ensures that the langchain library remains up-to-date and useful for the community. Given that we already have a method to create embeddings for nodes, it is logical and beneficial to include a corresponding method for relationships.

Please let me know your thoughts on this.

[tomasonjo]

If it isn't used in any methods, and we don't really plan any support either, then there is no need for the method

[RafaelXokito]

@tomasonjo I reverted the changes regarding the relationship embeddings method.
I will open an issue to discuss it's inclusion.

[RafaelXokito]

@ccurme please review this PR.

[tomasonjo]

Why do we need return in subquery?

[RafaelXokito]

@tomasonjo thank you for your feedback. Could you please clarify your concern? Neither RETURN statement can be removed without causing an error.

I believe you are referring to the following code:

        import_query = (
            "UNWIND $data AS row "
            "CALL { WITH row "
            f"MERGE (c:`{self.node_label}` {{id: row.id}}) "
            "WITH c, row "
            f"CALL db.create.setNodeVectorProperty(c, "
            f"'{self.embedding_node_property}', row.embedding) "
            f"SET c.`{self.text_node_property}` = row.text "
            "SET c += row.metadata "
            "RETURN c "
            "} IN TRANSACTIONS OF 1000 ROWS "
            "RETURN c "
        )

If the first RETURN statement inside the subquery is removed, an error occurs because the variable c must be returned to be accessible in the outer context.

If the second RETURN statement is removed, an error occurs indicating that the outer query cannot end with a CALL statement; it must have a RETURN clause to conclude properly.

Therefore, both RETURN statements are necessary to ensure the query is valid and functions correctly.

Give me your feedback on this.

[tomasonjo]

We definitely dont want to return so much information as it isnt needed. Ideally we would get rid of both return statements

[RafaelXokito]

@tomasonjo Thank you for your feedback. I believe I have found a solution. Please leave your feedback on the changes that I made.

[tomasonjo]

Looks good, i will run tests later V sre., 17. jul. 2024, 11:07 je oseba Rafael Pereira < ***@***.***> napisala:

…

@tomasonjo <https://github.com/tomasonjo> Thank you for your feedback. I believe I have found a solution. Please leave your feedback on the changes that I made. — Reply to this email directly, view it on GitHub <#23725 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEYGGTIBQ3UMP4BZLWNCA5TZMYX5RAVCNFSM6AAAAABKGACSX6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZSHAYTGNRRGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[jexp]

LGTM