Repository navigation
feat(grammar): add ANTLR4 grammar for OSSIE_SQL_2026 expressions - #657
Conversation
|
Thank you for your first contribution to Apache Ossie (incubating)! A committer will review your PR. In the meantime, please ensure:
No ICLA is required to contribute. If you are elected as a committer in the future, you will need an Individual Contributor License Agreement (ICLA) on file with the ASF. Questions? Ask in the PR comments or join us on Slack. We appreciate your contribution! |
Add grammar/OssieExpression.g4, an ANTLR 4 grammar for the expression language in core-spec/expression_language.md, with Python tests that parse valid/invalid fixtures, the expressions in examples/, and check operator precedence. Add a grammar CI workflow, a README, a label rule, an .editorconfig rule for .g4 files, and a pointer from the spec. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Move the ANTLR grammar next to the spec (core-spec/grammar/) and the valid/invalid expression fixtures to compliance/expressions/ so any parser can run them. Require at least one argument after DISTINCT in function calls, and add SUM(DISTINCT) and COUNT(DISTINCT) to the invalid fixtures. Drop the stale NOT precedence note from the grammar header, update CI paths, labels and links, and ignore generated ANTLR output. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…spec/expression_language.md
0be85e1 to
6698f97
Compare
jbonofre
left a comment
There was a problem hiding this comment.
Thanks for this, the grammar and the shared compliance fixtures are a useful addition.
I read through the diff, and the operator precedence matches the spec's list.
FYI, I rebased the branch onto main to clean the conflicts in .editorconfig and README.md.
Question: why does .editorconfig get a tab-indent block for Makefile, go.mod and *.go here?
The [{Makefile,go.mod,*.go}] section was added by mistake and is
unrelated to the grammar. Keep only the [*.g4] block.
|
Hi JB,
thanks for the review, and for rebasing the branch onto main.
On your question: the tab-indent block for Makefile, go.mod and *.go in
.editorconfig was unintended. It has nothing to do with the grammar, and this
repository contains no Go or Makefile sources. I have removed it in a follow-up
commit on top of your rebased branch:
chore(editorconfig): remove unintended tab-indent block
The only .editorconfig change that remains in the PR is the [*.g4] section
(indent_style = space).
The branch was fast-forwarded to your rebase; no history was rewritten on my
side.
Thanks again,
Johannes
… Am 02.11.2026 um 18:05 schrieb JB Onofré ***@***.***>:
Thanks for this, the grammar and the shared compliance fixtures are a useful addition.
I read through the diff, and the operator precedence matches the spec's list.
FYI, I rebased the branch onto main to clean the conflicts in .editorconfig and README.md.
Question: why does .editorconfig get a tab-indent block for Makefile, go.mod and *.go here?
|
MrBlack1995
left a comment
There was a problem hiding this comment.
Thanks for this, a reference grammar plus language-neutral fixtures is something every converter can use. I come at it from the Microsoft converter, whose DAX translation emits OSSIE_SQL_2026, so I ran the grammar CI on my fork (9 passed on Python 3.11 and 3.14) and read the grammar against what that engine produces (CASE WHEN, NULLIF denominators, IS NOT DISTINCT FROM, COALESCE, CAST(... AS DOUBLE), LOG(b, x), FLOOR/TRUNC, COUNT(DISTINCT ...)). All of those shapes are covered by the rules as written. Two suggestions and a question, none blocking:
-
||precedence: the validator and the grammar disagree today. You list this as open for the spec, and I agree the spec should decide. Until it does, it may help to note thatvalidation/validate.pyparses with sqlglot, which readsa || b + casa || (b + c), while this grammar reads(a || b) + c. So the repo would hold two parsers that build different trees for the same valid expression. Two options until the spec decides: follow sqlglot/PostgreSQL (one tree across the repo), or reject an unparenthesized mix of||with+/-(nobody gets a silently different tree). The tpcds example (c_first_name || ' ' || c_last_name) parses the same either way. -
Also parse the compliance models.
test_example_models_parsereadsexamples/*.yaml. The portability fixtures (compliance/portability/m0-m3on main, M4-M6 in #631) andcompliance/query/scotthold portable expressions too (FILTER (WHERE ...),CASE,DATEDIFF,DATE_TRUNC). Globbingcompliance/**/*.ossie.yamlas well would keep the grammar in sync with the fixtures converters are graded on, not only with the examples. Happy to add new rungs tovalid.txtfrom the #631 side if that helps. -
Question:
EXISTS_IN(). The "Not Supported in Expressions" table points subquery users toEXISTS_IN(). It parses as a plain function call, so nothing is needed in the grammar, but a line invalid.txtwould pin it if the spec gives it arguments later.
I read the grammar against the spec's precedence list (unary, multiplicative, additive, comparison, NOT, AND, OR) and agree with JB that it matches; NOT a = b parsing as NOT (a = b) and the BETWEEN ... AND operands being value expressions both look right to me.
|
Hello Mr. Black 1995,
Thank you for your tests and comments. As long as the specification does not define the precedence of the `||` operator, the discrepancy between SQLglot and ANTLR is unfortunate. I suggest that we align the ANTLR grammar with the behavior of SQLglot. Since that parser was used first, it makes sense to use it as our reference.
I hope to be able to put forward a proposal by the end of the week.
Furthermore, we should take this as an opportunity to clarify how we can reach a formal decision for the specification.
Personally, I would prefer to reject expressions like `a || b + c` (without parentheses) as invalid, given that different SQL dialects handle this inconsistently.
Johannes
… Am 06.11.2026 um 08:31 schrieb Mr. Black 1995 ***@***.***>:
@MrBlack1995 commented on this pull request.
Thanks for this, a reference grammar plus language-neutral fixtures is something every converter can use. I come at it from the Microsoft converter, whose DAX translation emits OSSIE_SQL_2026, so I ran the grammar CI on my fork (9 passed on Python 3.11 and 3.14) and read the grammar against what that engine produces (CASE WHEN, NULLIF denominators, IS NOT DISTINCT FROM, COALESCE, CAST(... AS DOUBLE), LOG(b, x), FLOOR/TRUNC, COUNT(DISTINCT ...)). All of those shapes are covered by the rules as written. Two suggestions and a question, none blocking:
|| precedence: the validator and the grammar disagree today. You list this as open for the spec, and I agree the spec should decide. Until it does, it may help to note that validation/validate.py parses with sqlglot, which reads a || b + c as a || (b + c), while this grammar reads (a || b) + c. So the repo would hold two parsers that build different trees for the same valid expression. Two options until the spec decides: follow sqlglot/PostgreSQL (one tree across the repo), or reject an unparenthesized mix of || with +/- (nobody gets a silently different tree). The tpcds example (c_first_name || ' ' || c_last_name) parses the same either way.
Also parse the compliance models. test_example_models_parse reads examples/*.yaml. The portability fixtures (compliance/portability/m0-m3 on main, M4-M6 in #631 <#631>) and compliance/query/scott hold portable expressions too (FILTER (WHERE ...), CASE, DATEDIFF, DATE_TRUNC). Globbing compliance/**/*.ossie.yaml as well would keep the grammar in sync with the fixtures converters are graded on, not only with the examples. Happy to add new rungs to valid.txt from the #631 <#631> side if that helps.
Question: EXISTS_IN(). The "Not Supported in Expressions" table points subquery users to EXISTS_IN(). It parses as a plain function call, so nothing is needed in the grammar, but a line in valid.txt would pin it if the spec gives it arguments later.
I read the grammar against the spec's precedence list (unary, multiplicative, additive, comparison, NOT, AND, OR) and agree with JB that it matches; NOT a = b parsing as NOT (a = b) and the BETWEEN ... AND operands being value expressions both look right to me.
—
Reply to this email directly, view it on GitHub <#657?email_source=notifications&email_token=AXD4UBOZRZN5RX72EOZNFV35XV43VA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKNZZG4YDMNJZGAYKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-5797065900>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXD4UBK4SQRSDST3Z2EGZP35XV43VAVCNFSNUABGKJSXA33TNF2G64TZHMYTAOJZGM4DEMJTG45US43TOVSTWNRQHAZTKNZZGE2DFILWAI>.
You are receiving this because you authored the thread.
|
christianeu-db
left a comment
There was a problem hiding this comment.
Thanks @j0hannesz! The dev@ thread reached lazy consensus. Left three small notes; feel free to address them as follow-ups.
cc @kayemkim if you want to take a look too since this seems useful for all the validation work
| nonReservedWord | ||
| : DATE | TIME | TIMESTAMP | TIMESTAMP_NTZ | ||
| | FIRST | LAST | NULLS | ROW | ROWS | RANGE | UNBOUNDED | PRECEDING | FOLLOWING | CURRENT | ||
| | FILTER | OVER | WITHIN | GROUP | PARTITION | ASC | DESC | POSITION |
There was a problem hiding this comment.
The portable example in spec.md (Expression Object) uses a dataset named order, and the grammar rejects it because ORDER is reserved:
SUM(order.sales) / COUNT(DISTINCT order.customer_id)
1:4 no viable alternative at input 'SUM(order'
validate.py accepts the same expression. Could ORDER join this list, or should the spec example be updated to something like orders.name
| - `||` is accepted, at the level of `+` and `-`. The spec's portable form is `CONCAT(a, b)`; | ||
| `a || b` only appears as a dialect form. |
There was a problem hiding this comment.
The String Manipulation table in expression_language.md lists || as a REQUIRED operator, and the portable sql in examples/tpcds_semantic_model.yaml uses it, so it is more than a dialect form. Maybe:
||is accepted, at the level of+and-. The spec lists||as a REQUIRED
concatenation operator but does not say where it sits.
| assert kind(value) == "ValueOnlyExpr" | ||
| add = value.valueExpr() | ||
| assert kind(add) == "AdditiveValue" | ||
| assert kind(add.valueExpr(1)) == "MultiplicativeValue" |
There was a problem hiding this comment.
No test pins unary +/- (level 2 of the spec's list): with # unaryValue moved below # additiveValue, all nine tests still pass and -a + b parses as -(a + b). Could we add a case for it?
kayemkim
left a comment
There was a problem hiding this comment.
validation/validate.py passes 8 of the 15 lines in compliance/expressions/invalid.txt as a field's sql today, with no error or warning (sqlglot 30.31.0): a b, a IS, x IN (), SELECT a FROM t, SUM(DISTINCT), COUNT(DISTINCT), order_date + INTERVAL '7 days' and DATE_ADD(d, INTERVAL 7 DAY). #647 and #698 are closing the SELECT one; the other seven have no PR.
The other way round the two agree. All 66 lines of valid.txt parse in both, and across the 263 distinct portable expressions in the Ossie documents of this repo and ossie-converters there is none that one accepts and the other rejects.
Nothing to change in this PR. I will pick up the seven on the validation side once it is merged.
jbonofre
left a comment
There was a problem hiding this comment.
Thanks @j0hannesz for the .editorconfig cleanup!
LGTM, great one!
Summary
Adds an ANTLR 4 reference grammar for the
OSSIE_SQL_2026expression language.core-spec/grammar/OssieExpression.g4is the grammar.compliance/expressions/valid.txtandinvalid.txtare language-neutral conformance fixtures (one expression per line), so any parser can run them.core-spec/grammar/tests/has a pytest runner. It generates the Python parser, runs both fixture files, parses everyANSI_SQL/OSSIE_SQL_2026expression inexamples/*.yaml, and pins operator precedence on the parse tree..github/workflows/grammar-ci.ymlruns the tests (Python 3.11 and 3.14).core-spec/expression_language.mdgets a short pointer to the grammar at the end. The spec stays normative; where the two differ, the spec wins.Layout and approach follow JB's review.
Open points for the spec (not decided here)
||precedence: the grammar parses it at the level of+and-, soa || b + cis(a || b) + c. sqlglot and PostgreSQL parse it asa || (b + c).$.LIKE ... ESCAPEis not supported, as the spec does not mention it.a = b = cneeds parentheses).The grammar follows whatever the spec decides on these.
Dependencies
No new runtime dependency. The tests use the ANTLR 4.13.2 tool (downloaded from Maven Central into the git-ignored
dev/lib/, verified against a pinned SHA-256) andantlr4-python3-runtime4.13.2, both BSD-3-Clause (ASF Category A), pluspytestandpyyaml.Related Issues
https://lists.apache.org/thread/s5x6lo5gvxhyowb4pcohtplnly5rmgtm
Supported by Jean-Baptiste Onofré.
Checklist
Specification
core-spec/and follow the existing structureOntology
ontology/are consistent with spec changesConverters
Validation
validation/are updated if the spec changedDocumentation
docs/is updated to reflect any user-facing changesCONTRIBUTING.mdis updated if the contribution process changedExamples
examples/are added or updated for any new spec constructsTests
pytest/ CI green)Compliance
NOTICEand/orLICENSEhave been updated per ASF policy