Skip to content

fix: qualify struct field access to avoid binder ambiguity post-join#247

Open
shriram-devrev wants to merge 1 commit intomainfrom
struct-field-aliasing
Open

fix: qualify struct field access to avoid binder ambiguity post-join#247
shriram-devrev wants to merge 1 commit intomainfrom
struct-field-aliasing

Conversation

@shriram-devrev
Copy link
Copy Markdown
Contributor

Summary

When a dimension SQL references a struct field like stage.stage_id, the aliaser previously skipped it because column_names.length !== 1. After joining tables that expose a column with the same name as the struct root (e.g. devusers.stage alongside issue.stage), DuckDB's binder fails with:

Binder Error: Ambiguous reference to column name "stage" (use: "issue.stage" or "devusers.stage")

The query can't even run because the bare `stage` resolves to two different columns in scope.

What changed

Make the aliaser schema-aware:

  • Introspect each table's physical columns via `DESCRIBE (<tableSchema.sql>)` in both the browser and node wrappers before running the batch.
  • Derive `knownTableNames` from the current schema batch.
  • Rewrite a multi-part column ref as `..<rest...>` only when the leading identifier is a known column on the current table.
  • Leave cross-table references (`customers.id`), already-qualified refs, lambda-bound identifiers, and unknown multi-part refs untouched.
  • Fall back to the legacy length-1 behavior when schema information is not supplied, preserving backwards compatibility.

Decision matrix

Case `tableName` Column set Known tables Result
`stage.stage_id` (struct) `issue` has `stage` `issue.stage.stage_id` ✓
`customers.id` (foreign alias) `orders` no `customers` has `customers` unchanged ✓
`issue.id` (already qualified) `issue` unchanged ✓
`foo.bar` (unknown) `issue` no `foo` no `foo` unchanged (fail-safe) ✓
`x -> x.field` (lambda) any unchanged ✓

Files touched

  • `meerkat-core/src/utils/ensure-sql-expression-column-alias.ts` — schema-aware decision logic
  • `meerkat-core/src/utils/ensure-table-schema-alias-sql.ts` — plumbs `tableColumnsByName` and `knownTableNames` into context
  • `meerkat-browser/src/ensure-table-schema-alias/ensure-table-schema-alias.ts` — `DESCRIBE` introspection
  • `meerkat-node/src/ensure-table-schema-alias/ensure-table-schema-alias.ts` — `DESCRIBE` introspection
  • Fixtures + specs updated

Test plan

  • `npx jest meerkat-core/src/utils/ensure-sql-expression-column-alias.spec.ts` — 29 passed (7 new)
  • `npx jest ensure-table-schema-alias` — 9 passed (1 new)
  • `npx nx test meerkat-node` — 887 passed
  • `npx nx run-many -t test -p meerkat-core,meerkat-browser` — 515 passed
  • Verify end-to-end in devrev-web: issue dashboard grouping by `priority_uenum` with `devusers` join no longer hits the binder error

@github-actions
Copy link
Copy Markdown

⚠️ Heads-up: This repository will be blocked from any work other than patching.

File meerkat-browser/src/ensure-table-schema-alias/ensure-table-schema-alias.ts is not allowed to be modified in this patch.
The following vulnerability issues are past SLA:

Note that there is significant latency in updating this list. Please reach out on #antifragile if you are in a hurry or have an emergency.

@shriram-devrev shriram-devrev force-pushed the struct-field-aliasing branch from 05944a2 to f66eab5 Compare April 22, 2026 10:28
@github-actions
Copy link
Copy Markdown

⚠️ Heads-up: This repository will be blocked from any work other than patching.

File meerkat-browser/src/ensure-table-schema-alias/ensure-table-schema-alias.spec.ts is not allowed to be modified in this patch.
The following vulnerability issues are past SLA:

Note that there is significant latency in updating this list. Please reach out on #antifragile if you are in a hurry or have an emergency.

When a dimension SQL references a struct field like `stage.stage_id`, the
aliaser previously skipped it because `column_names.length !== 1`. After
joining tables that have a column with the same name as the struct root
(e.g. `devusers.stage` alongside `issue.stage`), DuckDB's binder raises
an ambiguous column reference error before the query can run.

Make the aliaser schema-aware: introspect each table's physical columns via
`DESCRIBE` in the browser and node wrappers, then rewrite a multi-part
column ref as `<tableName>.<root>.<rest...>` only when the leading
identifier is a known column on the current table. Cross-table references
(`customers.id`), already-qualified refs, lambda-bound identifiers, and
unknown multi-part refs are left untouched.

Falls back to the legacy length-1 behavior when schema information is not
supplied, preserving backwards compatibility.
@shriram-devrev shriram-devrev force-pushed the struct-field-aliasing branch from f66eab5 to 610e8a6 Compare April 22, 2026 11:11
@github-actions
Copy link
Copy Markdown

⚠️ Heads-up: This repository will be blocked from any work other than patching.

File meerkat-browser/src/ensure-table-schema-alias/ensure-table-schema-alias.spec.ts is not allowed to be modified in this patch.
The following vulnerability issues are past SLA:

Note that there is significant latency in updating this list. Please reach out on #antifragile if you are in a hurry or have an emergency.

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.

2 participants