Skip to content

fix(service-analytics): qualify a native-SQL base column whenever the statement joins a relationship path, read from the hop resolver's joins, not cube.joins - #21266

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-21249-native-sql-base-column-qualify
Oct 2, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-21249-native-sql-base-column-qualify

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #21249
Clause-②: no

What changed

On the native-SQL strategy, a bare base-table column is now qualified with the base table ("deal"."note") exactly when the statement joins something. What the statement joins is read from the joins the one hop resolver (hop-object.ts, consumed unchanged) registered for the query. It is no longer read from whether the cube declares joins. ⛔ No new resolver.

  • qualifyAndRegisterJoin reads joins.qualifyBaseColumns, a flag the statement's StatementJoins now carries, instead of canJoin = !!cube?.joins && Object.keys(cube.joins).length > 0.
  • generateSql compiles the query once with bare base columns. When that compile registered any join, it compiles the query once more with every base column qualified. The flag cannot be read at call time, because joins are registered lazily: whichever member walks a relationship path first registers the join, and an absorbed $or takes back the joins its branches registered. In the card's own pair, note is resolved before owner.email registers the join. Ablation A2 below measures that a call-time read (joins.size > 0) leaves the card's pair broken. The method is split into compileClauses (the SELECT / GROUP BY / WHERE half, unchanged in place) and assembleStatement (the allowlist, read-scope and assembly half, unchanged in place). Both compiles walk the same members through the same resolver, so the second registers the same joins. A statement that joins nothing is compiled once.
  • resolveFieldSql's fallback for a filter member the cube does not declare (where: { id }) used to return the bare name. It now goes through qualifyAndRegisterJoin like a declared member. Measured below: the fallback was the same 500 beside a join.
  • Every base-column reference now takes the one rule: the select list, GROUP BY, WHERE (the where, the dataset scope, measure filters), measures and time-dimension windows. ORDER BY emits the member's output alias (ORDER BY "note"), never a table column, so it is not a qualification site. See the Acceptance notes for ordering by a member the query does not select.

Measured: POST /api/v1/analytics/query and /sql on the real dispatcher route

Setup: AnalyticsServicePlugin over a real ObjectQL engine and SqlDriver, a signed-in caller, the real dispatcher-plugin mount, SQLite in memory and a private PostgreSQL 16.14. A configured cube over deal declares no join. owner is a lookup whose reference is person, and person also declares note, amount, closed_on and id. A second cube declares the join (the control). Before: origin/main at 4727fcb2. After: this branch's build, re-taken at the merge with origin/main (1b176fbf). The scratch probe was deleted.

query (no-join cube unless named) native, SQLite before → after native, PostgreSQL 16.14 before → after ObjectQL face
dimensions note + owner.email (the card's pair) 500 → 200, 3 groups by deal note 500 (42702 note) → 200 200 → 200, same groups
the same pair, the dimension declared on the cube (owner_email) 500 → 200 500 → 200 200, unchanged
the pair with where: { note: 'x' } and order: { note: 'asc' } 500 → 200 500 → 200 200, unchanged
sum(amount) by owner.email 500 → 200 (15 / 8) 500 (42702 amount) → 200 200, unchanged
where: { note } by owner.email 500 → 200 500 → 200 200, unchanged
a closed_on time-dimension window by owner.email 500 → 200 500 (42702 closed_on) → 200 200, unchanged
where: { id: 'd1' } (undeclared member) by owner.email 500 → 200 500 (42702 id) → 200 200, unchanged
ad-hoc query over deal (inferred cube), the pair 500 → 200 500 → 200 200, unchanged
control: the declared-join cube, the pair 200 → 200, statement byte-identical same 200, unchanged
control: note alone, either cube, or an absorbed $or over owner.email 200 → 200 200 → 200 200, unchanged

The compiled statement for the card's pair, before: SELECT note AS "note", "owner"."email" AS "owner.email", COUNT(*) AS "count" FROM "deal" LEFT JOIN "person" "owner" ON "deal"."owner" = "owner"."id" GROUP BY note, "owner"."email". After, it is the statement the declared-join cube always compiled: SELECT "deal"."note" AS "note", … GROUP BY "deal"."note", "owner"."email".

The no-path control, stated (dispatch Zone 2, item 3). A statement that joins nothing stays unqualified, as before, on a cube that declares no join. On a cube that declares a join, a query that uses none of it now also compiles bare columns. The /sql dry run shows SELECT note AS "note", COUNT(*) AS "count" FROM "deal" GROUP BY note, where before it showed "deal"."note". The answer is the same on both drivers: the statement reads one table, so both spellings name one column. This follows from triage's predicate (what the query joins, not cube.joins). The other way to keep that statement byte-identical is to keep reading cube.joins beside the predicate, which is what triage rules out. A third reading, qualifying every base column always, was measured and rejected: it reddens 64 tests in 21 files of this package, among them native-sql-rls.test.ts's deliberate pin that a single-object cube keeps bare columns.

Pins

New file: packages/services/service-analytics/src/__tests__/native-sql-base-column-qualify.test.ts. It uses the plugin's own composition over a real engine, a SQLite cell and a PostgreSQL cell (a named skip without OS_TEST_POSTGRES_URL), and both faces. It has 12 tests, 6 per cell. Each serve pin checks the rows on both faces, that the native face answered with one raw statement and no engine aggregate, and the compiled statement.

  1. The card's pair, grouped by the deal's note: (x, a, 2) · (x, b, 1) · (y, b, 1). The person rows carry different note / amount / closed_on values, so a statement that read the person's column would answer differently.
  2. The pair with a where on note and an order by it.
  3. The other emitters beside the join: a measure (sum(amount)), a time-dimension window (closed_on), and a member the cube does not declare (where: { id }).
  4. A join registered by a LATER member: dimensions: ['note'] with where: { 'owner.email': … }. The SELECT list is compiled before the filter joins owner. Native face only, because the ObjectQL face refuses a cross-object filter of its own accord. The reversed pair is pinned too.
  5. CONTROL: the declared-join cube compiles the pair to the exact statement it compiled on 4727fcb2, and the no-join cube now compiles the same statement.
  6. CONTROL: a statement that joins nothing keeps bare columns, on either cube, and when an absorbed $or takes its join back.

Ablations

The tests import the subject by relative path (../plugin.js), so each run reads src and there is no dist leg. Every mutation went through scripts/ablation-replace.mjs in WRAP mode, with an outer trap restore on EXIT INT TERM against the absolute path. They ran from committed 2a575561, with the PostgreSQL cell live. Predictions were written before each run.

ablation mutation predicted observed
A1 canJoin put back on cube.joins (joins.qualifyBaseColumns → the removed expression) all 12 red: pins 1–4 on the 500 / bare statement, pin 5 on "the no-join cube compiles the same statement", pin 6 on the declared cube's bare statement 12 failed / 0 passed
A2 the flag read at call time (joins.qualifyBaseColumns → joins.size > 0) 8 red (pins 1, 2, 4, 5 on each cell). Pin 3 stays green because there the join is registered before the base column. Pin 6 stays green because nothing joins 8 failed / 4 passed
A3 resolveFieldSql's fallback back to return fieldName; 2 red (pin 3 on each cell) 2 failed / 10 passed

Each mutation landed: anchor 1 → 0, and the blob changed from d2c652842c9f to A1 06e880595f77, A2 79a30a8137c0 and A3 09066c0079e7. Each was restored and proven: the blob equals the HEAD blob d2c652842c9f, and git diff HEAD is empty.

Fixture triage

The full service-analytics suite turned up exactly three expectations that pinned a bare base column inside a statement that joins. That is the shape this PR corrects. Each was replaced, and its comment, which described the old rule, now describes the new one. The tests' subjects (member resolution, spelling parity, traversal unification) are unchanged.

  • analytics-service.test.ts, "should resolve client-style lookup.field member references": SUM(amount) → SUM("opportunity"."amount").
  • infer-cube-relation-traversal.test.ts, "unifies a dotted key riding alongside a bare one": WHERE (industry = $1 AND …) → WHERE ("crm_account"."industry" = $1 AND …).
  • infer-cube-where-spelling-parity.test.ts, "bare and dotted keys reach parity together": WHERE (stage = $1 AND …) → WHERE ("deal"."stage" = $1 AND …). The file header and block 2's comment named the old rule, and now name the new one. Block 2's assertion (a statement that joins nothing stays bare) is unchanged and green.

Consumer radius, run against this branch's build at 560ab361 with OS_TEST_POSTGRES_URL set: every test file that drives the analytics service in packages/rest (23 files), packages/runtime (19), packages/drivers/driver-memory (27) and packages/drivers/driver-sql (7). The results were 359 / 359, 551 / 551, 863 / 863 and 133 passed / 1 skipped, with no failures. The 9 packages/qa/dogfood files that drive analytics are NOT MEASURED: their build closure (the example apps, the CLI) was not built here. They pin no statement text (git grep for GROUP BY / LEFT JOIN / SELECT over them has zero hits). They are left to the required Dogfood Regression Gate.

Docs

git grep -niE for ambiguous, qualif…, declares (no) join, bare column, single-object cube, cube can/has join and declared join over content/docs/**, excluding releases/ and references/, gave no sentence about analytics column qualification. The native-SQL join sentences are protocol/objectql/query-syntax.mdx:517 (the positive control: "on the native-SQL path those joins serve dimensions, measures and filter members", about a dataset's include) and data-modeling/analytics.mdx:189-193 (dataset joins come from include). Both stay true. No page edited.

Verification at 560ab361

560ab361 is this branch's head. It carries origin/main at 434c6c7c merged in (no overlap with this diff's files), with install and the build refreshed after the merge.

  • pnpm --filter @objectstack/service-analytics exec vitest run, with the PostgreSQL cells live: 165 files, 3827 passed, 0 skipped, 0 failed. typecheck (tsc --noEmit): exit 0, and its program lists all 5 touched .ts files (--listFiles).
  • Base reading on 4727fcb2, cells skipped: 164 files, 3758 passed / 56 skipped.
  • Gates: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derives 61 commands, and all 61 exit 0 at 560ab361. --ran reconciliation: "61 derived, 61 run, 0 NOT-MEASURED, 0 UNRUN", with every exit code recorded. check:dual-build-cjs-loads first answered PREREQUISITE NOT MET (exit 3), because some packages had no dist. After pnpm build (72 tasks, 71 cached) it exits 0, and that is the run recorded.
  • Lint, a declared narrowing: eslint --no-inline-config --format json over the 5 touched .ts files at 560ab361 reports 5 files, 0 errors and 0 warnings. ① All 5 are inside the population eslint.config.mjs lints (**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}, minus the global node_modules / dist / build / .next / .turbo ignores), and the changeset .md is in no files glob. ② The count of 5 is read from the JSON output. ③ The config enables no type-aware linting (--print-config gives parserOptions {"ecmaVersion":"latest","sourceType":"module"}, with no project), so this diff cannot move a verdict on an untouched file. The repo-wide pnpm lint is left to CI.

Acceptance notes

  • Ordering by a member the query does not select is a separate defect, reported to the seat and not handled here. The ORDER BY emitter writes the request key as an identifier (ORDER BY "note"), which names an output column only when that member is selected. Measured after this PR, all through POST /api/v1/analytics/query, native face, with the ObjectQL face answering 200 in every row:

    • { dimensions: ['owner.email'], order: { note: 'asc' } } answers 500 on both drivers. On PostgreSQL that is 42702, note ambiguous at the ORDER BY.
    • { dimensions: ['note'], order: { amount: 'asc' } } with no join at all answers 500 on PostgreSQL (42803, must appear in GROUP BY). On SQLite it answers 200, ordered by an arbitrary row's value.
    • order: { 'owner.email': 'asc' }, unselected, answers 500 on both drivers (42703, column "owner.email" does not exist).

    Qualifying the column would not mend it (PostgreSQL would still answer 42803). The fix is a decision about what an unselected order key means.

  • The other cube.joins readers in this file (canHandle's federated decline, and the three readScopedObjects fallbacks) were not seen failing at a door in these measurements. They stay with PR fix(service-analytics)!: refuse a JSON-stored dimension or count_distinct over a relationship path the cube declares no join for, located through the one hop resolver #21247's acceptance notes.

  • The /sql dry run's text changes for every statement that joins through an undeclared lookup (now qualified), and for a declared-join cube's statement that joins nothing (now bare). The consumer radius above found no test outside this package that pins that text.

Deviation from the declared file surface

The claim names native-sql-strategy.ts and its tests, plus a changeset. The diff stays inside that. The three fixture files above are this strategy's tests in service-analytics. The resolveFieldSql fallback is in the same file and is the same 500 beside a join, measured at the door.


Generated by Claude Code

claude added 5 commits October 1, 2026 22:48
…ement joins anything, read from the hop resolver's joins

The native strategy qualified a bare base-table column only when the cube
declared a join. A cube that declares none still joins a lookup's declared
reference through the one hop resolver, so a base column sharing its name with
a column of the joined target was emitted bare: "ambiguous column" on SQLite
and PostgreSQL (500). The statement is now compiled once with bare base
columns and, when that compile registered any join, once more with every base
column qualified.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…he same qualification; fixtures that pinned a bare base column beside a join

Three fixtures asserted a bare base column in a statement that joins: the shape
the qualification rule now corrects. Their expectations name the qualified
column, and the comments that described the old rule describe the new one.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…ionship path the cube declares no join for, on SQLite and PostgreSQL

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…on beside a joined relationship path

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-analytics, touching 12 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/concepts/north-star.mdx (via tableName (symbol, a field of interface StatementClauses))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v14.mdx (via generateSql (symbol, a method of class NativeSQLStrategy))
  • content/docs/releases/v17/17-5.mdx (via generateSql (symbol, a method of class NativeSQLStrategy))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 10 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 30c530e5c0e13ca283cde133f4e63a9503edf936 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from ee80aa23e6af37f03b2d2d18e0d3b2cd74bca5a0 — the merge of head 560ab3613e432858dc72a617a7e2c0d2be8b4d83 into base 30c530e5c0e13ca283cde133f4e63a9503edf936, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin ee80aa23e6af37f03b2d2d18e0d3b2cd74bca5a0 && git checkout ee80aa23e6af37f03b2d2d18e0d3b2cd74bca5a0
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 30c530e5c0e13ca283cde133f4e63a9503edf936 560ab3613e432858dc72a617a7e2c0d2be8b4d83 && git checkout -B drift-repro 30c530e5c0e13ca283cde133f4e63a9503edf936 && git merge --no-ff 560ab3613e432858dc72a617a7e2c0d2be8b4d83

node scripts/docs-audit/affected-docs.mjs --json 30c530e5c0e13ca283cde133f4e63a9503edf936

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 30c530e5c0e13ca283cde133f4e63a9503edf936 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants