Repository navigation
Conversation
`c -> '$.a'` is defined as `JSON_EXTRACT(c, '$.a')`, so the extraction has to
happen before an operator can use its result. Both arrows carried
`Precedence::PgOther`, which sits below `*`, `+` and the bitwise operators, so
`c -> '$.a' + 1` parsed as `c -> ('$.a' + 1)` — adding a number to a JSON path
— and `1 + c -> '$.a'` parsed as `(1 + c) -> '$.a'`.
Following the narrowest of the two approaches in the issue. The arrows move to
their own `Precedence::JsonExtraction`, which takes the same value as
`PgOther` in the shared table, so no dialect changes by default and the rest of
that operator family stays where it is. `MySqlDialect` then prices those two
tokens above `MulDivModOp` through `get_next_precedence`, which leaves the
table it shares with everyone else alone.
`GenericDialect` follows MySQL rather than PostgreSQL. The issue calls this a
judgment call, and the contributing guide settles it: a feature specific to one
dialect should parse under that dialect and under Generic.
Checked which dialects this moves by parsing `SELECT c -> 'p' + 1` under each:
MySQL and Generic group it the new way; PostgreSQL, SQLite, Hive, BigQuery,
Redshift and MsSQL are untouched; ClickHouse, Snowflake, Databricks and DuckDB
read `->` as a lambda, where precedence never applies.
The issue's table stops at arithmetic while its title does not: `&` and `^`
misgrouped for the same reason, at 23 and 22. `|` was already correct, but only
because `Pipe` shares 21 with `PgOther` and left associativity gives the same
answer, so the test pins it against a future change to either value.
Closes apache#2462
Signed-off-by: Laszlo Hadhazy <laszlo@cordata.tech>
Contributor
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.
Fixes #2462. Thanks @zvonimir-dd for the analysis in the issue — this follows the approach you set out there.
->and->>carryPrecedence::PgOther(21), which sits below*(40),+(30) and the bitwise operators, so the arrows under-bind on both sides:c -> '$.a' + 1c -> ('$.a' + 1)(c -> '$.a') + 1c ->> '$.a' * 2c ->> ('$.a' * 2)(c ->> '$.a') * 21 + c -> '$.a'(1 + c) -> '$.a'1 + (c -> '$.a')c -> '$.a' & 1c -> ('$.a' & 1)(c -> '$.a') & 1c -> pathis defined asJSON_EXTRACT(c, path)(docs), so the extraction has to happen before any operator can use its result.c -> ('$.a' + 1)adds a number to a JSON path.This takes the first of the two approaches in the issue, since
PgOther= 21 is correct for PostgreSQL and a shared row cannot be right for both engines. The arrows move to their ownPrecedence::JsonExtraction, which takes the same value asPgOtherin the shared table — so no dialect changes by default, and the rest of that operator family stays put — andMySqlDialectprices those two tokens aboveMulDivModOpthroughget_next_precedence, without duplicating the table.On the issue's two open questions:
GenericDialectfollows MySQL. The contributing guide asks that a dialect-specific feature parse under both the relevant dialect and Generic, and Generic already gives MySQL's grouping for every other operator in these expressions.Height: 42, just above
MulDivModOp(40) andAtTz(41), belowDoubleColon(50). Anything aboveMulDivModOpis correct for valid input, since the right operand is lexically a path string. Happy to move it nearerDoubleColonif you prefer the more literal reading of the grammar.I checked which dialects this actually moves, by parsing
SELECT c -> 'p' + 1under each: MySQL and Generic group it the new way; PostgreSQL, SQLite, Hive, BigQuery, Redshift and MsSQL are untouched; ClickHouse, Snowflake, Databricks and DuckDB read->as a lambda, where precedence never applies.The issue's table stops at arithmetic while its title does not —
&and^misgrouped for the same reason, at 23 and 22.|was already correct, but only becausePipeshares 21 withPgOtherand left associativity gives the same answer, so the test pins it against a future change to either value.Tests in
parse_json_arrow_arithmetic_precedence, besideparse_div_precedenceandparse_is_distinct_from_json_arrow_precedence, running under both dialects.cargo test --locked --all-features,cargo fmt --checkandcargo clippy --locked --all-targets --all-features -- -D warningsall pass.