-
Notifications
You must be signed in to change notification settings - Fork 63
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
Add experimental draft support for GPML-style graph query #652
Conversation
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.
Overall LGTM, added some minor comments.
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.
Mostly looks good to me. Have some nits and some questions related to how some of the nodes/edges are being parsed.
@@ -585,7 +590,7 @@ internal const val DIGIT_CHARS = "0" + NON_ZERO_DIGIT_CHARS | |||
|
|||
@JvmField internal val E_NOTATION_CHARS = allCase("E") | |||
|
|||
internal const val NON_OVERLOADED_OPERATOR_CHARS = "^%=@+" | |||
internal const val NON_OVERLOADED_OPERATOR_CHARS = "^%=@+~" | |||
internal const val OPERATOR_CHARS = NON_OVERLOADED_OPERATOR_CHARS + "-*/<>|!" |
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.
Hmm, just noticed the OPERATOR_CHARS
variable isn't used anywhere. Can it be removed?
} catch (e: ParserException) { | ||
null | ||
} | ||
patterns.add(nodeLeft) | ||
do { | ||
val edge = try { | ||
parseEdge() | ||
} catch (e: ParserException) { | ||
null | ||
} | ||
patterns.add(edge) | ||
val nodeRight = try { | ||
parseNode() | ||
} catch (e: ParserException) { | ||
null |
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.
Why do these return null
when there's a ParserException
?
Co-authored-by: Alan Cai <caialan@amazon.com>
Part of the exploration of #689
This adds draft parser/AST support for a subset of GPML as outlined by Graph Pattern Matching in GQL and SQL/PGQ. The use within the grammar is based on the assumption of a new graph data type being added to the specification of data types within PartiQL, and should be considered experimental until the semantics of the graph data type are specified.
This adds support for parsing:
TODO:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.