Skip to content

Fix parsing of the GLOBAL/LOCAL join locality and GLOBAL NOT IN - #293

Open
therealpandey wants to merge 2 commits into
AfterShip:masterfrom
therealpandey:fix-global-join-modifier
Open

Fix parsing of the GLOBAL/LOCAL join locality and GLOBAL NOT IN#293
therealpandey wants to merge 2 commits into
AfterShip:masterfrom
therealpandey:fix-global-join-modifier

Conversation

@therealpandey

@therealpandey therealpandey commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Fixes #292

GLOBAL/LOCAL was consumed in the same switch branch that would otherwise call parseJoinOp, so it acted as a join type of its own rather than a prefix to one. Any real join type after it was rejected, and the two forms that did parse (GLOBAL JOIN, LOCAL JOIN) dropped the keyword from Format(). Separately, GLOBAL claimed IN's precedence unconditionally — so a GLOBAL starting the next join was read as an operator — and parseInfix required IN immediately, leaving GLOBAL NOT IN with no parse path.

SELECT * FROM t1 GLOBAL INNER JOIN t2 ON t1.a = t2.a                                -- was: <EOF> or ';' was expected, but got: "INNER"
SELECT * FROM t1 GLOBAL ANY LEFT JOIN t2 USING (a)                                  -- was: <EOF> or ';' was expected, but got: "ANY"
SELECT * FROM t1 AS x LOCAL FULL JOIN t2 ON x.a = t2.a                              -- was: <EOF> or ';' was expected, but got: "FULL"
SELECT * FROM t1 GLOBAL JOIN t2 ON t1.a = t2.a GLOBAL LEFT JOIN t3 ON t1.a = t3.a   -- was: expected IN after GLOBAL
SELECT * FROM t WHERE a GLOBAL NOT IN (SELECT b FROM t2)                            -- was: expected IN after GLOBAL
SELECT * FROM t1 GLOBAL JOIN t2 ON t1.a = t2.a                                      -- parsed, but Format() lost the GLOBAL

What changed:

  • parseJoinRightExpr now reads the locality via a new parseJoinLocality helper, which consumes GLOBAL/LOCAL and then continues into parseJoinOp, prepending the locality to the join modifiers so writeJoinSQL re-emits it. No AST change — JoinExpr.Modifiers already carries the join keywords.
  • getNextPrecedence gives GLOBAL IN's precedence only when IN or NOT follows, so a GLOBAL that begins the next join is left for the FROM parser.
  • parseInfix accepts an optional NOT after GLOBAL and emits GLOBAL NOT IN, following the existing "NOT " + op handling.
  • GLOBAL/LOCAL before ARRAY JOIN is rejected with an explicit message rather than being accepted by the new path.

Verified against clickhouse local 26.8.1.337 over a 643-case matrix of locality × join type × ON/USING/no-constraint, plus GLOBAL IN/NOT IN in WHERE, PREWHERE, HAVING, ORDER BY, CASE and nested subqueries, and global/local used as identifiers and aliases. 262 statements that master rejected now parse; nothing that master accepted is rejected. LOCAL IN, NOT GLOBAL IN, GLOBAL GLOBAL IN, GLOBAL LIKE and GLOBAL ARRAY JOIN are Code: 62 on the server and stay rejected here — those are in TestParser_InvalidSyntax.

Coverage: a new parser/testdata/query/select_with_global_join_locality.sql fixture (only new golden files; no existing golden changed) and AST-shape assertions in precedence_test.go for the join modifiers, the GLOBAL IN/GLOBAL NOT IN operators, and their precedence.

One thing worth flagging, since it is visible in the matrix: a non-CROSS join without ON/USING parses here although ClickHouse requires a constraint. That predates this change — t1 JOIN t2 already parsed — but because 262 locality forms now parse, it reaches more statements than before. GLOBAL LEFT JOIN t2 with no constraint used to fail for the wrong reason and now parses. Requiring the constraint changes how every join parses and rewrites existing golden files, so it belongs in its own change rather than this one.

…rShip#292)

GLOBAL and LOCAL were consumed as if they were join types, so any real
join type after them was rejected and the two forms that did parse lost
the keyword when formatting. Parse the locality as a prefix and continue
into parseJoinOp, keeping it in the join modifiers.

GLOBAL also claimed IN's precedence unconditionally, so a GLOBAL that
started the next join was read as an operator, and parseInfix demanded
IN immediately, leaving GLOBAL NOT IN with no path.

ARRAY JOIN takes no locality, and neither does LOCAL IN, matching
ClickHouse 26.8.1.337.
Ending the expression whenever GLOBAL was not followed by IN/NOT left the
keyword for the enclosing clause to pick up, and the select item parser
takes any non-reserved keyword as an implicit alias. So
`SELECT a GLOBAL FROM t` began parsing as `SELECT a AS GLOBAL FROM t`,
which ClickHouse rejects as a syntax error. Looking ahead for the join
operator instead narrows the back-off to the locality it was meant for and
leaves GLOBAL binding like IN everywhere else, as it did before.

The ARRAY JOIN guard compared against the uppercase keyword, but
parseJoinOp records each modifier's source spelling, so
`GLOBAL array JOIN arr` slipped past it and read the column list as a
table expression.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] GLOBAL/LOCAL is parsed as a join type instead of a prefix to one, and GLOBAL NOT IN is rejected

1 participant