Repository navigation
Read columns inside TRIM-like forms and long operator chains - #74
Closed
IL-William wants to merge 3 commits into
Closed
IL-William wants to merge 3 commits into
IL-William wants to merge 3 commits into
Conversation
The parser builds a chain of binary operators left-deep: `a || b || c`
is `(a || b) || c`. The expression walkers recursed into both operands
and counted every operator as one level of nesting, so a flat chain of
more than about 100 operators passed MAX_RECURSION_DEPTH. The walkers
then gave up at the guard, which sits at the leftmost end of the chain,
and the statement was reported as APPROXIMATE_LINEAGE.
Generated surrogate keys reach that length quickly, because every field
is wrapped and joined with a separator:
SELECT MD5(
COALESCE(CAST(field_000 AS VARCHAR), '') || '-' ||
COALESCE(CAST(field_001 AS VARCHAR), '') || '-' ||
...
COALESCE(CAST(field_119 AS VARCHAR), '')
) AS row_key
FROM events
Over 120 fields, row_key was derived from the last 49 only: field_000
to field_070 were missing from its lineage. A key over 50 fields
already loses its first operands.
Walk the left spine of a BinaryOp chain in a loop and visit its operands
left to right, each one level below the chain. The length of a chain no
longer counts as depth, while nesting in right operands, function
arguments and parentheses still does, so the guard keeps protecting the
stack. The four walkers that share MAX_RECURSION_DEPTH use it:
visit_expression_for_subqueries, collect_column_refs,
find_aggregate_function and collect_simple_identifiers. Operands are
still visited in source order.
Tests: the 120-field key above keeps every field and raises no
APPROXIMATE_LINEAGE; unit tests pin the operand order and check that
nesting on the right still trips the guard.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
sqlparser gives TRIM, SUBSTRING, POSITION, CEIL and FLOOR, AT TIME ZONE,
IS [NOT] DISTINCT FROM, the `col:path` and `col[i]` accessors, COLLATE,
OVERLAY, SIMILAR TO, RLIKE, ANY and ALL, and a few more, their own `Expr`
variants rather than function calls. collect_column_refs matched none of
them and ended in a catch-all arm, so a column inside one of these forms
was never a source, and no issue said so:
SELECT o.id, TRIM(c.name) AS customer_name
FROM orders o
JOIN customers c ON o.customer_id = c.id
customer_name came out with no source column. In
`SUBSTR(code, 1, 2) || suffix` only suffix was read, and
`WHERE TRIM(c.status) = 'active'` was not attached to customers.
collect_simple_identifiers, which drives lateral column alias
resolution, already walked SUBSTRING, CEIL and POSITION but not TRIM, so
a key hashed from earlier aliases of the same SELECT list lost every
source:
SELECT
MD5(UPPER(TRIM(CAST(order_id AS VARCHAR)))) AS order_key,
MD5(UPPER(TRIM(CAST(customer_id AS VARCHAR)))) AS customer_key,
MD5(CONCAT(UPPER(TRIM(CAST(order_key AS VARCHAR))), '||',
UPPER(TRIM(CAST(customer_key AS VARCHAR))))) AS link_key
FROM orders
Make the match in collect_column_refs exhaustive. Every form descends
into the operands it evaluates in the enclosing scope, including named
arguments written `name => value`, the tested value of
`x IN (SELECT ...)`, subscripts and bracket keys. Field names in a path
step and lambda parameters are not read, and subqueries keep their own
scope. The parser keeps `o.items[1]` as the root `o` followed by the
steps `.items` and `[1]`, so the leading dot steps are folded back into
the name: the column read is `o.items`, as without the subscript, and
not a column `o` of orders. With no catch-all arm left, a variant added
to sqlparser is a compile error rather than a column dropped from
lineage.
collect_simple_identifiers walks the same forms, so an alias hidden in
one of them is replaced by its sources instead of being resolved as a
column of a table in scope.
The postgres_array_slicing snapshot changes accordingly: `a[:]`,
`b[:1]`, `c[2:]` and `d[2:3]` now derive from the columns a, b, c and d
rather than from the table node.
Tests: the queries above, SUBSTR, CEIL, POSITION and `:` operands in
Snowflake, the left operand of IN (subquery), a DuckDB lambda and a
subscript of a qualified column in Postgres at the analysis level; a
table of dedicated forms pins what collect_column_refs reads, and checks
that extract_simple_identifiers sees the same bare names.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Two gaps in the expression walkers made column lineage drop sources without
raising an issue.
a || b || cleft-deep, and thewalkers counted every operator as a level of nesting, so a flat chain of about
100 operators reached
MAX_RECURSION_DEPTH. Generated surrogate keys getthere quickly,
COALESCE(CAST(f AS VARCHAR), '') || '-' || ...: over 120fields, the key came out derived from the last 49 only, and the statement was
reported as
APPROXIMATE_LINEAGE.CEIL and FLOOR, AT TIME ZONE, IS [NOT] DISTINCT FROM, the
col:pathandcol[i]accessors, COLLATE, OVERLAY, SIMILAR TO, RLIKE, ANY and ALL their ownExprvariants.collect_column_refsended in a catch-all arm, so a columninside one of them was never a source:
SELECT TRIM(c.name) AS customer_name FROM orders o JOIN customers c ON o.customer_id = c.idgave
customer_nameno source column.Changes
fix(core): stop long operator chains from tripping the depth guardwalksthe left spine of a
BinaryOpchain in a loop, in the four walkers that shareMAX_RECURSION_DEPTH. Nesting in right operands, function arguments andparentheses still counts, so the guard keeps protecting the stack.
fix(core): read columns inside TRIM, SUBSTRING and other dedicated formsmakes the match in
collect_column_refsexhaustive, so a variant added tosqlparser becomes a compile error rather than a column silently dropped.
Lambda parameters and path field names are not read, and subqueries keep
their own scope.
collect_simple_identifiers, which resolves lateral columnaliases, walks the same forms. The
postgres_array_slicingsnapshot changesaccordingly:
a[:]and its neighbours now derive from their column ratherthan from the table node.
Each commit message has the details and examples. There is no API or schema
change.
Validation
cargo test --workspace --locked,cargo clippy --workspace --locked -- -D warnings,cargo fmt --all -- --checkand the schema guard pass on thisbranch.
APPROXIMATE_LINEAGE, and unit tests pin the operand order and check thatnesting on the right still trips the guard. For the dedicated forms,
analysis level tests in Snowflake, DuckDB and Postgres, and a table of forms
pinning what
collect_column_refsreads.on a large Snowflake dbt project, these restore several hundred column edges.
The committed browser WASM is not rebuilt here, since CI builds its own.
🤖 Generated with Claude Code