Fix parsing of the GLOBAL/LOCAL join locality and GLOBAL NOT IN - #293
Open
therealpandey wants to merge 2 commits into
Open
Fix parsing of the GLOBAL/LOCAL join locality and GLOBAL NOT IN#293therealpandey wants to merge 2 commits into
therealpandey wants to merge 2 commits into
Conversation
…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.
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 #292
GLOBAL/LOCALwas consumed in the same switch branch that would otherwise callparseJoinOp, 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 fromFormat(). Separately,GLOBALclaimedIN's precedence unconditionally — so aGLOBALstarting the next join was read as an operator — andparseInfixrequiredINimmediately, leavingGLOBAL NOT INwith no parse path.What changed:
parseJoinRightExprnow reads the locality via a newparseJoinLocalityhelper, which consumesGLOBAL/LOCALand then continues intoparseJoinOp, prepending the locality to the join modifiers sowriteJoinSQLre-emits it. No AST change —JoinExpr.Modifiersalready carries the join keywords.getNextPrecedencegivesGLOBALIN's precedence only whenINorNOTfollows, so aGLOBALthat begins the next join is left for theFROMparser.parseInfixaccepts an optionalNOTafterGLOBALand emitsGLOBAL NOT IN, following the existing"NOT " + ophandling.GLOBAL/LOCALbeforeARRAY JOINis rejected with an explicit message rather than being accepted by the new path.Verified against
clickhouse local26.8.1.337 over a 643-case matrix of locality × join type ×ON/USING/no-constraint, plusGLOBAL IN/NOT INinWHERE,PREWHERE,HAVING,ORDER BY,CASEand nested subqueries, andglobal/localused 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 LIKEandGLOBAL ARRAY JOINareCode: 62on the server and stay rejected here — those are inTestParser_InvalidSyntax.Coverage: a new
parser/testdata/query/select_with_global_join_locality.sqlfixture (only new golden files; no existing golden changed) and AST-shape assertions inprecedence_test.gofor the join modifiers, theGLOBAL IN/GLOBAL NOT INoperators, and their precedence.One thing worth flagging, since it is visible in the matrix: a non-
CROSSjoin withoutON/USINGparses here although ClickHouse requires a constraint. That predates this change —t1 JOIN t2already parsed — but because 262 locality forms now parse, it reaches more statements than before.GLOBAL LEFT JOIN t2with 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.