-
Notifications
You must be signed in to change notification settings - Fork 231
refactor ast validations #2730
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
refactor ast validations #2730
Conversation
…aph/RedisGraph into refactor-ast-validations
Codecov ReportBase: 92.26% // Head: 92.12% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2730 +/- ##
==========================================
- Coverage 92.26% 92.12% -0.14%
==========================================
Files 264 264
Lines 25259 25091 -168
==========================================
- Hits 23306 23116 -190
- Misses 1953 1975 +22
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@@ -84,7 +84,7 @@ def test_missing_sections_reduction(self): | |||
try: | |||
self.graph.query(q).result_set | |||
except ResponseError as e: | |||
self.env.assertIn("n not defined", str(e)) | |||
self.env.assertIn("Unknown function 'reduce'", str(e)) |
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.
Unknown function 'reduce'
seems like a misleading error message, as the function reduce
does exists...
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 parser doesn't recognize the reduce pattern, so it is parsed as a function call and thus the error-message.
I think this can be a good sign to the user that he did something wrong, since as you said, reduce
is not a function.
I can add a check for this if you disagree.
src/ast/ast_visitor.h
Outdated
( | ||
void *ctx, // context to attach to the new visitor | ||
ast_visitor_mapping mapping // mapping between ast-node-types to visiting functions | ||
ast_visitor *visitor // visitor |
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.
const
src/ast/ast_visitor.c
Outdated
( | ||
void *ctx, // context to attach to the new visitor | ||
ast_visitor_mapping mapping // mapping between ast-node-types to visiting functions | ||
ast_visitor *visitor // visitor |
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.
const
src/ast/ast_visitor.c
Outdated
// creates a new visitor | ||
ast_visitor *AST_Visitor_new | ||
// get the context of a visitor | ||
void *AST_VisitorGetContext |
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.
Rename to: AST_Visitor_GetContext
* wip refactor ast validations * WIP Introduced validations for reduce and union * Reduce validation fix * forget prior env in WITH clause * fixed CALL validations * fix index validations * fix rel-pattern validation * reformating and comment-fixing * fix tck tests * leak fix * fix leak 2 * leak fix & union type support * leak fix * fix and formatting * fix list comprehension, pattern comprehension * added test for redeclaration in merge * short circuit validation * Added fast-error-folding * Revert 'short circuit validation' and 'Added fast-error-folding' commits * design change * promote CALL validations * fix * leak fix * unified mapping initialization, unified cypher-whitelist check * review changes * Addressing review comments * fix * Addressing comments * small fix * fix * fix2 Co-authored-by: razmon <razmonsonego2@gmail.com> Co-authored-by: Raz Monsonego <74051729+raz-mon@users.noreply.github.com> Co-authored-by: swilly22 <roi@redislabs.com> Co-authored-by: Roi Lipman <swilly22@users.noreply.github.com>
#2052