feat: add explicit emit output grammar and parsing support#173
feat: add explicit emit output grammar and parsing support#173eliot1480 wants to merge 13 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e486d22ab1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
wackywendell
left a comment
wackywendell
left a comment
There was a problem hiding this comment.
The Grammar update makes sense, and scoping it to just Read is reasonable, good work there!
That said, even if we're just scoping it to Read for this PR, it should be architected as something more general that we can adopt for other relations and maintain clarity and orthogonality around the relevant structures, and I have a few suggestions along those lines.
Co-authored-by: Wendell Smith <wendell.smith@datadoghq.com>
Co-authored-by: Wendell Smith <wendell.smith@datadoghq.com>
Co-authored-by: Wendell Smith <wendell.smith@datadoghq.com>
Co-authored-by: Wendell Smith <wendell.smith@datadoghq.com>
…sing' of github.com:DataDog/substrait-explain into eseol/QSEM-585/add-explicit-emit-output-grammar-and-parsing
…ake output syntax more friendly for other relations
…plicit-emit-output-grammar-and-parsing
2 participants
Description
Added explicit output grammar and parsing support. When the parser encounters an emit, it will parse that emit and determine if it is a direct emit or explicit emit using
parse_output_emit. I will be implementing textifying from substrait in another PR.Type of Change
Testing
Checklist
Related Issues
Closes #(34)
#34