Skip to content

fix(driver-sql): refuse scalar-comparison operators on JSON/multi-value columns instead of answering silently wrong - #7415

Draft
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-7398-multiple-column-operator-refusal
Draft

fix(driver-sql): refuse scalar-comparison operators on JSON/multi-value columns instead of answering silently wrong#7415
claude[bot] wants to merge 2 commits into
mainfrom
claude/issue-7398-multiple-column-operator-refusal

Conversation

@claude

@claude claude Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Closes #7398.

The defect

driver-sql stores a multiple: true field — and every other JSON_COLUMN_TYPES field — as a JSON TEXT column. The equality family lowered straight to SQL against that text with no column-type consultation, so a filter naming such a column compiled, ran, and returned a wrong answer with a 200.

Reproduced on main before the change, on the issue's own fixture (one row whose members holds ["usr_1111","usr_2222"]):

filter before after
{members:{$in:[U1]}} 200, 0 rows 400 INVALID_FILTER
{members:{$in:[U1,U2]}} 200, 0 rows 400 INVALID_FILTER
{members:{$eq:U1}} 200, 0 rows 400 INVALID_FILTER
{members: U1} (bare equality) 200, 0 rows 400 INVALID_FILTER
{members:{$nin:[U1]}} 200, 1 row — the row it was asked to EXCLUDE ⚠️ 400 INVALID_FILTER
{members:{$ne:U1}} 200, 1 row — same inversion ⚠️ 400 INVALID_FILTER
{members:{$gt:U1}} 200, 0 rows 400 INVALID_FILTER
{members:{$lte:U1}} 200, 1 row (lexicographic, on the leading [) 400 INVALID_FILTER
{members:{$between:[U1,U2]}} 200, 0 rows 400 INVALID_FILTER
{members:{$contains:U1}} 200, 1 row unchanged, 200, 1 row
{$or:[{members:{$contains:U1}},{members:{$contains:U2}}]} 200, 1 row unchanged
{owner:{$in:[U1]}} (scalar control) 200, 1 row unchanged
{members:{$overlaps:[U1]}} 400 unknown operator unchanged

members not in ('U1') is TRUE — the stored text genuinely is not equal to that id — so "exclude these" compiled to "return everything". $in fails closed; $nin and $ne fail OPEN, and an exclusion that silently stops excluding widens a result set. A 200 with [] is byte-identical to a query that legitimately matched nothing, so nothing existed for a caller to key on.

$lte is worth its own row: the answers were never merely empty. The ordering comparisons return a lexicographic verdict over a serialization.

The change (the issue's ask 1 only)

Gate the three filter-lowering entries on the column type, ahead of every rewrite and both comparison emitters:

# site spelling it compiles
1 applyFilterCondition — operator-object branch $eq/$ne/ordering/$in/$nin/$between
2 applyFilterCondition — bare-value branch { field: value } alongside an operator sibling
3 applyFilters — plain-map loop { field: value } when no key carries an operator

Site 1 sits before applyNormalizedComparison, which matters: a JSON column reaches two different emitters, and the one the issue named is not the one the measured fixture uses. A managed multiple: true lookup goes through the plain whereIn / where(f, op, v) arms; a multiple: true datetime column on an external object (ADR-0015) goes through applyNormalizedComparison's normalised whereRaw arms instead, because registerExternalObject never runs backfillCanonicalDatetimes, so needsLegacyDatetimeRepair stays true. Both were measured wrong the same way ($in → 0 rows, $nin → the excluded row) before this gate.

Predicate: the existing jsonFields registry — JSON_COLUMN_TYPES.has(type) || !!field.multiple — not a new list, and not scoped to multiple alone. A structured-JSON column shows the identical defect for the identical reason ({address:{$nin:['Beijing']}} returned the row it was asked to exclude), because the mechanism is the JSON-text storage rather than the array-ness. A table this driver was never told about answers false and is unaffected.

Envelope: ADR-0112 class 1 — INVALID_FILTER / 400, the same unsupportedFilterError envelope as the unknown-operator refusal. The message names the operator, the field, why the column cannot answer it, states the filter was not applied, and prescribes $contains (or an $or of $contains for any-of).

Faces (#6203): the gate sits in the shared lowering funnel, and every face that lowers a filter is swept by test — find, findOne, count, aggregate, distinct, and the where-clauses of updateMany / deleteMany.

What deliberately does not change

$contains, $notContains, $startsWith, $endsWith, $icontains — the LIKE family matches the serialization as text, and $contains is the only working membership spelling. $null / $exists also keep working: the column's presence is a well-formed question whatever it holds. Scalar columns are untouched.

The issue's ask 2 ($overlaps / $containsAny) is not implemented — it would open the closed FILTER_OPERATORS set and needs its own ruling.

Tests

packages/drivers/driver-sql/src/sql-driver-json-column-operator-refusal.test.ts — 129 cases. Every refusal asserts code AND status AND message content, never a bare toThrow (#6144). It carries a closed-world sweep over FILTER_OPERATORS: every declared operator must be either refused or on the keep-working list, and the partition is asserted whole — so a newly declared operator or a newly added lowering site cannot join the silent set without turning the file red.

Reverse verification. Moving the gate one line later — after applyNormalizedComparison instead of before it — leaves exactly the 6 external-column cells red and the 123 managed-column cells green, which is what makes the external cell a positive control on the site enumeration rather than a second example. Removing all three gate calls reds 95 of 129.

No existing test was changed, deleted, weakened, skipped or retried; none asserted the old silent behaviour.

Gates

  • pnpm --filter @objectstack/driver-sql test82 files / 1285 passed, 48 skipped (was 81 / 1156 before this PR: +1 file, +129 tests)
  • pnpm --filter @objectstack/driver-sql typecheck — clean
  • eslint on both changed files — clean
  • driver-sqlite-wasm (308) and driver-turso (931), which subclass this driver — green
  • objectql (3066), rest (1278), runtime (1906), metadata (593) — green
  • check-empty-changeset.mjs, check-adr-0087-registration.mjs — green (patch, not declared-breaking, so no ADR-0087 marker is owed)

Generated by Claude Code

…ue columns

A `multiple: true` field — and every other `JSON_COLUMN_TYPES` field — is
stored as a JSON TEXT column, and the equality family lowered straight to SQL
against that text with no column-type consultation. The result was a wrong
answer with a 200:

  {members:{$in:[U1]}}   -> 0 rows            (fail-closed)
  {members: U1}          -> 0 rows            (fail-closed)
  {members:{$nin:[U1]}}  -> the excluded row  (fail-OPEN)
  {members:{$lte:U1}}    -> 1 row, lexicographic on the leading '['

`members not in ('U1')` is TRUE — the stored text genuinely is not equal to
that id — so "exclude these" compiled to "return everything". An exclusion
that silently stops excluding widens a result set, and a 200 with [] is
byte-identical to a query that legitimately matched nothing, so nothing
existed for a caller to key on.

Gate the three lowering entries on the column type, ahead of every rewrite and
both comparison emitters: the operator-object branch and the bare-value branch
of applyFilterCondition, and the plain-map loop of applyFilters. Placing it
before applyNormalizedComparison matters — a `multiple: true` datetime column
on an external object is served by the normalised whereRaw arms rather than
the plain whereIn arms, and showed the identical defect.

The refusal names the operator, the field, why the column cannot answer it,
states the filter was not applied, and prescribes $contains (or an $or of
$contains for any-of). ADR-0112 class 1 — INVALID_FILTER / 400, the same
envelope as the unknown-operator refusal, on every face that lowers a filter.

$contains / $notContains / $startsWith / $endsWith / $icontains and the null
predicates are untouched: the LIKE family matches the serialization as text
and is the only working membership spelling, and column presence is a
well-formed question whatever the column holds.

Fixes #7398

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011GCuQuqxKvWLYUGss7CXdc
@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 10, 2026 10:57am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-sql.

8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx (via @objectstack/driver-sql)
  • content/docs/getting-started/glossary.mdx (via @objectstack/driver-sql)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/driver-sql)
  • content/docs/plugins/anatomy.mdx (via @objectstack/driver-sql)
  • content/docs/plugins/packages.mdx (via @objectstack/driver-sql)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/driver-sql)
  • content/docs/protocol/kernel/lifecycle.mdx (via @objectstack/driver-sql)
  • content/docs/protocol/objectql/query-syntax.mdx (via @objectstack/driver-sql)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx (via @objectstack/driver-sql)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 10, 2026
…ing it

`check:query-options-erasure` counts an `as any` at the options position of
find/findOne/count/aggregate, and the new refusal sweep raised the test surface
249 -> 250. The cast was gratuitous: `aggregations` is on `DriverQuery`, so the
call types as written once the entry carries its `field`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011GCuQuqxKvWLYUGss7CXdc
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/l tests tooling

Projects

None yet

1 participant