Repository navigation
fix: traverse skipped expression children in TablesNamesFinder - #2826
Open
AndreasLanz01 wants to merge 1 commit into
Open
AndreasLanz01 wants to merge 1 commit into
AndreasLanz01 wants to merge 1 commit into
Conversation
- Visit aggregate ORDER BY, KEEP, HAVING, LIMIT, keyword arguments and attribute functions of Function and AnalyticExpression - Visit LIKE ESCAPE, AT TIME ZONE zones, JSON operator paths, IS UNKNOWN and MATCH ... AGAINST operands - Visit GROUP_CONCAT and JSON aggregate key, value, ORDER BY and OVER - Dispatch table functions dynamically so XMLTABLE PASSING is visited Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
What
TablesNamesFinderskipped some child expressions. A table read by a subquery in one of these positions was missing fromgetTables(..),getTablesOrOtherSources(..)andfindTablesInExpression(..):FunctionORDER BY:string_agg(a, ',' ORDER BY ..),array_agg(a ORDER BY ..),group_concat(a ORDER BY ..);KEEP (DENSE_RANK FIRST ORDER BY ..);HAVING MAX ..;LIMIT ..; keyword arguments such asSEPARATOR ..; an attribute function:f(a).g(..)AnalyticExpressionHAVING MAX ..,LIMIT .., keyword argumentsKeepExpressionORDER BY(the visit was empty)MySQLGroupConcatLikeExpressionESCAPEexpressionTimezoneExpressiond AT TIME ZONE (..)JsonExpression->,->>,#>,#>>IsUnknownExpressionFullTextSearchMATCH (..) AGAINST (..)columns and value (the visit was empty)JsonAggregateFunctionJSON_OBJECTAGG, itsORDER BY,OVER (PARTITION BY .. ORDER BY .. ROWS ..)TableFunctionXMLTABLE(.. PASSING ..)inFROMTwo private helpers keep this in one place:
visitAggregateModifiers(KEEP, HAVING, keyword arguments and LIMIT ofFunctionandAnalyticExpression) andvisitWindowElement(window frame, moved out ofvisit(AnalyticExpression)and reused forJsonAggregateFunction).visit(TableFunction)calledvisit(function, null), which binds at compile time tovisit(Function).XmlTableFunctionandJsonTableFunctionextendFunction, so their own visit methods were never reached. It now callsfunction.accept(this, context), which also passes the caller's context instead ofnull.JSON_TABLEis now traversed byvisit(JsonTableFunction); the result is the same and is covered by a test.Two outputs change beyond the missing tables:
MATCH (..) AGAINST (..)columns are now visited like other qualified columns:findTablesInExpression("MATCH (t.a) AGAINST ('x')")returns[t](before:[]).findTablesInExpression("f(a).b.c")returns no table.Why
A caller that checks a statement against a table allow-list with
TablesNamesFindernever sawt2in any of these; each returned onlyt1, or nothing:Testing
TablesNamesFinderTraversalTest:nestedTables: 15 new cases, one per skipped child (KEEP, HAVING, LIMIT, SEPARATOR,GROUP_CONCAT ... ORDER BY, attribute function,IS UNKNOWN,MATCH ... AGAINST, the JSON aggregates,XMLTABLE), and 2 controls forJSON_TABLEand a plain table function;findsTablesInPostgresExpressionChildren, parsed withDialect.POSTGRESQL, checkinggetTablesandgetTablesOrOtherSources:ORDER BY,ESCAPE,AT TIME ZONEand the four JSON operators;FILTER,WITHIN GROUP, windowPARTITION BY/ORDER BY/ frame,LIMIT/OFFSET/FETCH,ANY/ALL/SOME,ARRAY(..),VALUES,substring/trim/position/overlay,CASE, casts,GROUP BY/GROUPING SETS,DISTINCT ON,LATERAL;expressionTables: 4 new cases throughfindTablesInExpression, and thef(a).b.ccontrol;visitsAggregateChildrenOnce: no table is visited twice.On base
bdf8ac31the 28 cases for skipped children fail; the 3 array cases (fixed by fix: traverse missing query and DML children in TablesNamesFinder #2749) and all controls pass../gradlew checkon Windows 11 with the JDK 17 toolchain: 10,411 test cases, 0 failures, 0 errors, 25 skipped. Grammar ambiguity, Spotless, Checkstyle, PMD, SpotBugs and JaCoCo verification pass.Not run:
mvn verify, JMH (no grammar or parser change), Linux/macOS.Not changed
DECLARE @x int = (SELECT ..)andPRAGMA x = (SELECT ..): their visits are empty on purpose (PRAGMA: "no tables involved").JSON_OBJECTAGG(KEY (SELECT ..) VALUE b)does not parse, so only theVALUEposition is tested.visitBinaryExpression(BinaryExpression)still passes anullcontext to its children; it is public, so I left it alone.Origin
First observed with
com.manticore-projects.jsqlformatter:jsqlparser:5.4.127andDialect.POSTGRESQL. Reproduced on upstream masterbdf8ac31bb17b2ae76400a8ddd22a39a990dcc89. The array slice upper bound, also missed in 5.4.127, is already fixed on master by #2749; this PR only adds regression tests for it. No existing issue is linked.AI disclosure: this change was prepared with an AI coding agent (Claude Code) and reviewed by me.