Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix complex MERGE causes crash (#897) #961

Merged
merged 1 commit into from
Jun 5, 2023

Conversation

jrgemignani
Copy link
Contributor

Fixed the complex MERGE crash, which was due to storing variables in tuple positions that did not exist. This happened on MERGE commands where there wasn't a RETURN clause.

Added logic to catch these and handle appropriately. Meaning, discarding the variable results, instead of storing them.

Fixing this issue has highlighted the need to fix variable reuse in the MERGE transform itself. This is because MERGE preprocesses the path before handing it off to anything else.

Added regression tests.

Fixed the complex MERGE crash, which was due to storing variables
in tuple positions that did not exist. This happened on MERGE
commands where there wasn't a RETURN clause.

Added logic to catch these and handle appropriately. Meaning,
discarding the variable results, instead of storing them.

Fixing this issue has highlighted the need to fix variable reuse
in the MERGE transform itself. This is because MERGE preprocesses
the path before handing it off to anything else.

Added regression tests.
@dehowef
Copy link
Member

dehowef commented Jun 2, 2023

I looked over it, and good job preventing the crash. If @muhammadshoaib could take a look too, that'd be great

@jrgemignani
Copy link
Contributor Author

Added @MuhammadTahaNaveed as well.

Copy link
Member

@MuhammadTahaNaveed MuhammadTahaNaveed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jrgemignani just a small question.

regress/sql/cypher_merge.sql Show resolved Hide resolved
@dehowef dehowef merged commit 69c275e into apache:master Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants