-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Remove glr parser #5290
Remove glr parser #5290
Conversation
aa0ae73
to
3c941d2
Compare
4741bac
to
534ba7e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, prefer to test the case in #5221 and following tests:
with [1,2] as a, 2 as b, [1] as c return (a)-[b]-(c);
with [1,2] as a, 2 as b, [1] as c return (a)- [b] -(c);
| parenthesized_expression { | ||
auto& e = $1; | ||
if (e->kind() != Expression::Kind::kLabel) { | ||
throw nebula::GraphParser::syntax_error(@1, "Invalid node pattern"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete $1 before throw exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expression memory is managed by ObjectPool, and there is no need for delete, which would result in double free.
small fix fix tck fmt fix tck add test cases add tck
6454197
to
e34e932
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
This fixed some patten expressibility broblems, but also introduce some compatibility(which is not very common), we should notice the change in release note when 3.5 released. @Sophie-Xie |
@MuYiYong pls review this incompatible pr, thanks. |
The previous implementation of pattern expression introduce bison glr parser to handle conflicts, but that is actually unnecessary and harmful. This pr refactors the implementation and fix some related bugs.
What type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
#5235
Description:
Checklist:
Tests:
Affects: