Parse data-dict definition expressions - #269
Draft
jat255 wants to merge 1 commit into
Draft
Conversation
First of four PRs porting the definition compiler (kata f6hz). Text in, AST out; no types yet, so nothing here produces an export record. The AST is asserted directly, not just checked for the absence of a parse error. A parser that accepts everything and builds the wrong tree would pass a "the corpus parses" check and then silently misinform the type checker built on top of it. The constructs with dedicated parse functions (intervals, COLUMNS, CASE, quoted names, field paths) get their own cases, along with the precedence and associativity rules. The two corpus tests were confirmed to bite by perturbing the parser: dropping COLUMNS support fails the valid corpus, and accepting a missing operand fails parse.yaml. The first perturbation attempted, removing the trailing-text check, did not fail either test, because `amount +` is refused for its missing right operand instead.
jat255
marked this pull request as draft
September 4, 2026 05:49
Collaborator
Author
|
Manual review completed 2026-09-04 21:29 MDT. Final pass over the parser and its tests: clean (two INFO-level notes only: >4300-digit float literals bypass the positioned error message, and the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First of four PRs porting the data-dict definition compiler to Python (kata f6hz, stage 1). Text in, AST out. No type checking, so nothing here produces an export record yet.
_expression.pyfollowspkg-r/R/definition-expression.Rclosely enough to diff against it, which is the only practical way to review a hand-port this size. The later PRs are_export.py,_emit_duckdb.py, then_compile.pyand the wiring.The tests assert the AST rather than only the absence of a parse error. A parser that accepts everything and builds the wrong tree passes a "the corpus parses" check and then silently misinforms the type checker built on top of it, so the constructs with dedicated parse functions get their own cases alongside the precedence and associativity rules.
Both shared-corpus tests passed on first run, so I confirmed they can fail: dropping
COLUMNSsupport fails the valid corpus (42 expressions), and accepting a missing operand failsparse.yaml. A third perturbation, removing the trailing-text check, failed neither, becauseamount +is refused for its missing right operand instead. That is recorded in the commit message so the next reader does not repeat it.One deliberate departure from R: integer literals are stored as
intrather than as the normalized string R keeps to protect 64-bit precision. Python needs no such protection, and the range check against9223372036854775807is still there because data-dict's is a Rusti64.Verification: 406 tests pass, ruff and pyrefly clean on
srcandtests.