Skip to content

fix: traverse missing query and DML children in TablesNamesFinder - #2748

Closed
fudianchn wants to merge 1 commit into
JSQLParser:masterfrom
fudianchn:fudianchn
Closed

fudianchn wants to merge 1 commit into
JSQLParser:masterfrom
fudianchn:fudianchn

Conversation

@fudianchn

Copy link
Copy Markdown
Contributor

AI disclosure: this change was prepared with AI coding agents, reviewed and revised line by line by me.

What

TablesNamesFinder now visits omitted array subscripts, named function arguments, analytic partitions, initial piped-query joins and lateral views, UPSERT duplicate actions, IF conditions and OUTPUT destinations. MERGE uses the existing OUTPUT traversal shared by INSERT, UPDATE and DELETE.

Why

SELECT a[(SELECT max(x) FROM t2)] FROM t1 returns only t1 on the base commit instead of t1 and t2. Array slices such as a[OFFSET(1):2] can throw a NullPointerException during table extraction.

How

  1. Reuse the existing expression-list, join and insert-action helpers to visit the omitted child fields.
  2. Check the array index before visiting it, independently of either slice bound.
  3. Keep column-processing rules, table-variable handling and existing window traversal. Add the missing MERGE call to the shared OUTPUT visitor.
  4. Add regression tests for all public entry paths, adjacent DML consumers, array bounds, finder reuse and visit counts.

Root cause

Several visitor overrides omit already-parsed child fields. ArrayExpression checks the start bound before dereferencing the single index, which skips ordinary subscripts and dereferences a null index for slices.

Testing

  • TablesNamesFinderTraversalTest adds 65 cases. On base bb55bb9d, 39 fail and 26 normal controls pass; the fix passes all 65. Static, instance, expression and other-source entry points are covered. Four isolated mutations are rejected by the tests.
  • JDK 17: both Finder test classes pass, 171 cases total. Full ./gradlew check passes with 9467 cases, zero failures/errors and 25 skipped; mvn clean verify passes with 9449 cases, zero failures/errors and 25 skipped. Formatting, Checkstyle, PMD, SpotBugs, grammar ambiguity, applicable coverage and changed-file license checks pass.
  • Project JMH JSQLParserBenchmark.parseSQLStatements, unchanged 54-statement corpus, version=latest: interleaved baseline/fixed, 3 forks per state, 2 one-second warmups and 5 one-second measurements per fork. Baseline 27.875 ms/op, 99.9% CI [21.312,34.438]; fixed 27.890 ms/op, CI [21.270,34.510], 15 samples per state. The intervals overlap. This parsing benchmark does not exercise TablesNamesFinder.
  • No database-server validation, Finder-specific performance measurement or Windows/macOS local test matrix was run.

Behavior notes

  • Returned table sets include names previously omitted. User variables and inserted/deleted pseudo-tables keep their existing statement-entry handling. No grammar or public API changes are required.
  • Slice tests assert index=null and both bound expressions present. Bare f()[1:2] retains its JsonExpression index reading as a normal control; this change does not fix JSON-path grammar or traversal.
  • Existing WITH visitation and expression-entry column-qualification rules remain outside this change. Node-count assertions guard the newly visited fields without promising identity deduplication for arbitrary manually constructed AST graphs.

Verification of the original issue

No existing issue is linked. The failures were reproduced on upstream master bb55bb9de8377de9880b14e4fe3b3cc94bf63498; the fixed commit is 8f7de28c5a72316ac72d41acf74211f1d34e8dba. This builds on #2479, #2517 and #2538, covering child fields that those changes did not address.

SELECT a[(SELECT max(x) FROM t2)] FROM t1;
SELECT f()[(SELECT max(x) FROM t2)] FROM t1;
SELECT a[OFFSET(1):2] FROM t1;

The first two return t1 and t2 with the fix. The slice returns t1 without throwing. The same families are tested through statement and expression entry points.

Signed-off-by: 付典 <fudianchn@gmail.com>
@fudianchn fudianchn closed this Oct 2, 2026
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.

1 participant