Repository navigation
fix(analytics)!: a query window outside the non-negative integers is refused at the door, and an offset with no limit runs on SQLite - #21399
Conversation
…der an offset-only window per dialect AnalyticsQuerySchema's limit and offset become non-negative integers (the dataset selection reads the same declarations). The native statement takes the dialect's no-limit spelling in front of an OFFSET with no LIMIT. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude <noreply@anthropic.com>
…et-only window per dialect Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude <noreply@anthropic.com>
…ivers and both faces Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude <noreply@anthropic.com>
…window Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude <noreply@anthropic.com>
… semantic entry Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): ⛔ 2 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 138 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 98ab58eb0af7648f63d169e0875e1da81c1b1b75 && git checkout 98ab58eb0af7648f63d169e0875e1da81c1b1b75
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ecb6ca0258176466767588a6805363387c5777a6 b1a8ea073b357d1b05e163e438aa30bd1977c833 && git checkout -B drift-repro ecb6ca0258176466767588a6805363387c5777a6 && git merge --no-ff b1a8ea073b357d1b05e163e438aa30bd1977c833
node scripts/docs-audit/affected-docs.mjs --json ecb6ca0258176466767588a6805363387c5777a6
|
Part of #21365
Clause-②: yes (narrowing)
What this changes
Two halves of one family: an analytics query window that had no single answer across drivers and faces.
1. The contract (
packages/spec).AnalyticsQuerySchema.limitand.offsetwere a barez.number(). They arez.number().int().nonnegative()now (packages/spec/src/data/analytics.zod.ts), each with a.describe().DatasetSelectionSchemareads the same two declarations offAnalyticsQuerySchema.shape, andAnalyticsQueryRequestSchemaextends the query. So all three doors hold one accept set, and there is no second declaration. A spec pin asserts that the dataset selection's two members are the same instances.POST /analytics/queryandPOST /analytics/sql(packages/runtime/src/domains/analytics.ts) andPOST /analytics/dataset/query(packages/rest/src/analytics-selection-door.ts). A negative or fractional value now answers400 VALIDATION_FAILED, withdetails.fields[].fieldnaminglimitoroffset(selection.limit/selection.offsetat the dataset door), before any engine runs. There is no new refusal code and no runtime code change.limit: 0stays legal (LIMIT 0, no rows). An offset with no limit stays legal.number). Only the parse narrows.2. The offset-only window on the native face (
service-analytics).NativeSQLStrategy.assembleStatementwroteOFFSET nwith noLIMITin front of it, which SQLite cannot parse. The window clause is nowwindowClauseSql(limit, offset, dialect)innative-sql-strategy.ts, with the dialect read off the existingsqlDialecthook (sqlDialectFor, filled by the plugin from the executing driver'sdialectName). An offset with no limit takes the dialect's no-limit spelling:sqliteLIMIT -1 OFFSET npostgresOFFSET n(bytes unchanged)unknown(no hook wired)LIMIT 9223372036854775807 OFFSET nmysqlLIMIT 18446744073709551615 OFFSET nThe named-dialect cells are the spelling the driver's own query compiler emits for the same window (knex 3.3.0:
limit -1in the sqlite3 compiler,limit 18446744073709551615in the mysql one, nothing in the base compiler PostgreSQL uses).unknownis everything the hook could not name, SQLite included, so its cell is the largestLIMITevery LIMIT dialect parses. That follows the residue-arm reasoningtext-match-sql.tsalready records for the same hook. The door does not refuse the offset-only window.The echoed
sqlandPOST /analytics/sqlon the native face are the statement that ran, byte for byte (pinned).Measured before and after, at the route
Real
POST /api/v1/analytics/queryand/analytics/sql(createDispatcherPlugin), withAnalyticsServicePluginover a realObjectQLengine andSqlDriver. Two compositions: the default one (native face) and one narrowed to the engine aggregate (ObjectQL face). Four groups bynote(w, x, y, z),order { note: 'asc' }, SQLite (better-sqlite3) and a private PostgreSQL 16.14 server.mainee75aae1a: native SQLitelimit: -1VALIDATION_FAILED(limit)limit: 1.5VALIDATION_FAILED(limit)offset: -1VALIDATION_FAILED(offset)offset: 1, nolimitnear "OFFSET": syntax error)limit: 2, offset: 1On
main,/analytics/sqlanswered 200 for every row, rendering the statement. On this branch it answers the same 400 for rows 1 to 3. The dataset door's selection check (datasetSelectionRefusal) answers400 VALIDATION_FAILEDnamingselection.limit/selection.offsetfor the same four refused values, and passes{ offset: 1 }and{ limit: 2, offset: 1 }. That was measured at the helper, not over HTTP.Pins
packages/spec/src/data/analytics-query-window-integer.test.ts: on each of the three schemas, a negative limit, a fractional limit, a negative offset and a fractional offset are refused at the member's own path, with zod's issue code (never its message). Integer windows,limit: 0,offset: 0, an offset with no limit and no window are each a whole-safeParsesuccess. Plus the identity pin on the dataset selection's two members.packages/services/service-analytics/src/__tests__/native-sql-offset-only-window.test.ts(SQLite always, PostgreSQL whereOS_TEST_POSTGRES_URLis set):generateSqlequal the statement that ran;unknownarm (aNativeSQLStrategywhose context wires nosqlDialect) runs and answers the same rows;windowClauseSqlper dialect, with the MySQL cell as text only.packages/runtime/src/analytics-query-window-validity.test.ts(route level, SQLite always, PostgreSQL whereOS_TEST_POSTGRES_URLis set): rows 1 to 3 plusoffset: 1.5answer 400VALIDATION_FAILEDnaming the member at both routes, on both faces, and the engine sees no raw statement and no aggregate. Row 4 answers x y z on both faces, and the native echo and/analytics/sqlequal the statement that ran. CONTROLs:limit 2, offset 1andlimit 0.No CI step provisions
OS_TEST_POSTGRES_URLfor these two packages, so the PostgreSQL cells are red-capable and un-run in CI. The numbers below include them, run against a private local PostgreSQL 16.14.Ablations
Each leg ran on the committed tree at
b1a8ea073throughscripts/ablation-replace.mjs. The anchor was proven to land (count and blob hash), and the restore was proven by blob hash equal toHEADand an emptygit diff HEAD. The spec and service-analytics pins import fromsrc/, and the runtime suite aliases@objectstack/spectosrc/, so none of these legs goes through adist/..int()dropped (both members)limit: 1.5andoffset: 1.5, both cells. Native PostgreSQL answered 200 withLIMIT 1.5andOFFSET 1.5.nonnegative()droppedlimit: -1andoffset: -1, both cellswindowClauseSqlwrites nothing beforeOFFSET)unknownarm; on PostgreSQL only theunknownarm. 7 green: both CONTROLs and the PostgreSQL cells that run a bareOFFSETEach leg turned red in the direction expected beforehand. The route pin's row 4 reads
service-analyticsfromdist/(KNOWN_UNALIASED_TEST_IMPORTS), so the no-limit leg was run on the service pin, which imports fromsrc/.ADR-0087 disposition
The changeset is declared breaking and carries the disposition
registered analytics-query-window-non-negative-integer. That is a new D3 semantic entry,packages/spec/src/migrations/entries/semantic/18.analytics-query-window-non-negative-integer.ts, andregistry.tswas regenerated bygen:migration-registry. The gate's other categories do not apply: the package is published, nothing was registered before, the changeset carries a FROM → TO prescription (sono-migration-prescriptionis refused), and this is a metadata parse, not a runtime interface or a type surface. No D2 conversion: the window is a query-time request field, not a storedsys_metadatashape, and the refused values meant different windows per backend.spec-changes.jsonand the upgrade guide fold majors up to 17, so a major-18 entry leaves both byte-identical.check:spec-changesandcheck:upgrade-guidepass.Acceptance notes
OFFSET nwith noLIMITon SQLite.objectql-strategy.tsgenerateSql, the two window lines. The face's rows are right. Only the echoed text cannot run on SQLite. Measured on this branch: a month-bucketedorder+offset: 1(which lands on that face on every composition) echoes… ORDER BY "closed_on" ASC OFFSET 1on both drivers. That echo also rendersdate_trunc(…), which SQLite cannot run either. A non-bucketed query on a composition narrowed to the engine aggregate echoes… ORDER BY "note" ASC OFFSET 1. Rendering it withwindowClauseSqlis a two-line change, butobjectql-strategy.tsis outside this claim's file surface and held by another claim (domain:engine#1's #5930 step 4 (domain:engine): the engine-fed faces delete their hand-copied filter meaning (driver-sql, turso remote, memory query, mongodb, formula,having); the memory reference matcher retires (D6) #20822 group 4, prose only), so it is not changed here. It is raised in the report.mysqlcell is text only. Separately, and NOT MEASURED: the native statement quotes identifiers with ANSI double quotes, which MySQL withoutANSI_QUOTESreads as string literals (the CI workflow's own comment says this repo's MySQL runs without it). So the native face may not run on MySQL at all, whatever its window. This is an inference from reading the code, not a measurement, and is noted only.AnalyticsService.querydoes not parse its input, so a host that builds a query in code is not refused by this narrowing. The changeset and the semantic entry say so. The census found no producer of a negative or fractional window: none in examples, fixtures, docs or published skills, and the one stored producer that lowers into a selection, a dashboard widget'slimit, is alreadyz.number().int().positive(). The sibling console repository was not checked out here, so its census is NOT MEASURED.Number.MAX_SAFE_INTEGERare refused too (zod's.int()). That is stated in the changeset.content/docs/references/{api,data}/analytics.mdx) were regenerated bycheck:generated --fix, and it proved onlycheck:docsstale. No hand-written page incontent/docs/**(outsidereleases/) orskills/**says anything this change makes false.Verification (head
b1a8ea073, after mergingmain6d487d209)turbo run build(72 tasks) after the merge. Then:pnpm --filter @objectstack/service-analytics testwith live PostgreSQL: 168 files, 3883 tests passed;pnpm --filter @objectstack/spec test: 599 files, 17557 passed, 1 todo;@objectstack/runtime: the new route pin, 16 passed on both cells.typecheckof@objectstack/spec,@objectstack/service-analyticsand@objectstack/runtime: exit 0. Each new test file is in a compiled program (tsc --listFiles: the spec and runtime test configs, the service-analytics main config), andcheck:test-typecheckholds both debt ledgers unchanged.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandsderived 116 commands atb1a8ea073, and all 116 exited 0.--ranreconciles 116 derived, 116 run, 0 NOT-MEASURED, 0 UNRUN..tsfiles:eslint --no-inline-config --format jsonread 7 files, 0 errors, 0 warnings, none ignored.eslint.config.mjsenables no type-aware linting (noparserOptions.project), so this diff cannot change the verdict on an untouched file. The repo-widepnpm lintis left to CI.packages/cliandpackages/restare untouched.@objectstack/runtimechanges by one test file only.Generated by Claude Code