Repository navigation
fix(driver-sql)!: PG introspectSchema reads each table's columns in its own schema and refuses a same-named pair across schemas - #24920
Conversation
…ema; refuse a same-named pair Claude-Session: https://claude.ai/code/session_013timUNNCthRfQkhy9Hz2fV Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013timUNNCthRfQkhy9Hz2fV Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013timUNNCthRfQkhy9Hz2fV Co-authored-by: Claude <noreply@anthropic.com>
…-multischema-introspect
📓 Docs Drift CheckThis PR changes 1 package(s): 9 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 11 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 52d278f73411c764dacc0708876b42c422b786cd && git checkout 52d278f73411c764dacc0708876b42c422b786cd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin a7199ef9744edf0c3ef0e381bb95a328545a3da9 1eeab8089f223dd08909f385a79ff4c1b0e8f992 && git checkout -B drift-repro a7199ef9744edf0c3ef0e381bb95a328545a3da9 && git merge --no-ff 1eeab8089f223dd08909f385a79ff4c1b0e8f992
node scripts/docs-audit/affected-docs.mjs --json a7199ef9744edf0c3ef0e381bb95a328545a3da9
|
Contract reviewServed-tier: Inputs: card #24707 body and all four comments (triage 6282275557, claim 6292170810, dispatch 6292244225, os-dev-report 6293456848 read as claims); #11201 body and comments; PR #24920 body, file list (3 files, +271/−11) and ① Derived judgmentsThe refusal's reach — right. The only new refusal is in the PostgreSQL arm of Who reaches Platform-owned callers: (a) the boot-time federated validation (
The detection — right. Judged on The column read — right, and the single-schema answer is unchanged by construction. The refusal's text — right. It names the count, the first colliding name with its schemas, the mechanism (one key, bare-name resolution to the first schema), every collision as Precision point, not a defect of this PR: at both HTTP doors a throw from The pins — assert the new truth with teeth; both ablation legs go red by construction. What the lane means for THIS gate: ② Semver level
The optional trailing Nothing else widens. No new export ( ③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
席位核对 ·
|
| Item | Reading |
|---|---|
| Shape | Draft PR → main. The body opens Fixes #24707. 3 files, +271/−11. Not governed: check-governed-merges matched 0 of 3 paths; 282 changed lines. |
| Scope | On PostgreSQL, introspectSchema reads each table's columns in its own schema, and refuses a search_path that reaches one bare table name in two schemas (RESOURCE_CONFLICT / 409, naming each table and its schemas). The single-schema answer is unchanged. SQLite and MySQL are untouched. No spec change, and no IntrospectedTable member. |
| Seat's own read | The remedy the refusal names is real. postgres.zod.ts's schema reaches knex searchPath in default-datasource-driver-factory.ts. So only a saved external datasource that omits schema, on a server whose default search_path names two schemas, can meet the refusal. content/docs/data-modeling/external-datasources.mdx says nothing about search_path or multi-schema listing, so the docs-drift list holds no page this change makes false. git merge-tree against today's origin/main (which carries PR #24906's driver-sql hunks) is clean. |
| Review | CONTRACT_REVIEW_TIER, narrowing face. Record 6294518392 is PASS on this head. |
| Declaration | One changeset, @objectstack/driver-sql minor, BREAKING (FROM / TO / Fix), Clause-②: no (narrowing) at a line start, as on the claim and the PR body; adr-0087: not-required (no-migration-prescription). The new optional schema parameter on two protected methods is a subclass-visible signature change, not a new member; the record judges it the sounder shape (TursoDriver overrides introspectColumns). |
| Cross-lane | None: driver-sql is this lane's. service-datasource, the consumer, was read only. No objection on the card or the seat posts. |
| Gates | Dev at 1eeab8089: 70 derived, 70 run, all exit 0. driver-sql full suite 3687 passed, 21 live-PG introspection files 170 passed, service-datasource 862 passed, core test:repo 82 passed, full build 77 tasks. Ablation: two legs, each red, restored to HEAD. |
| CI | At this read on 1eeab8089f: ci-failure GREEN, all 41 check-runs completed, none failed. The four live-PG pins are a named skip in this PR's Test Core lane and run in main's Temporal Conformance battery, the package's standing convention (record ①). That battery's first run on main after the merge is the pins' CI measurement, and the seat reads it. |
Disposition:
open_questions: A, the 409 refusal (record ③): a log line would be the same silent drop for the HTTP and AI callers.RESOURCE_CONFLICTis an existing code.deviations: all accepted (record ③).out_of_scope_findings:introspectPrimaryKeys'?::regclasscan resolve a view in an earlier schema. That is an inference, not measured, so it meets no filing gate; it is noted in the PR.- Precision point (record ①), not a defect: the HTTP doors (
admin-routes.ts,rest's external-datasource routes) answer the refusal as400 EXTERNAL_DATASOURCE_ERRORcarrying its message; the 409 pair is the driver's in-process contract. The landing comment says so.
Landing: ordinary queue (pr_ready + automerge_enable through the relay). Fixes #24707 then closes the card.
Generated by Claude Code
… into 7b-0 Conflicts in seven fixture files take #24893's resolution (4dadd7e): where 7a-2 now writes the grant with an organization the pre-18 relax goes; where main still lands an organization-less row the relax stays. sharing-rule-criteria-required takes main's version verbatim (both sides had added the organization key). plugin-webhooks' new real-SqlDriver case gets the lean engine's Default Organization. Claude-Session: https://claude.ai/code/session_018EszgtA3V2KXBX5tf59Hi4 Co-authored-by: Claude <noreply@anthropic.com>
Fixes #24707
Clause-②: no (narrowing)
Dev session
session_013timUNNCthRfQkhy9Hz2fV, dispatched by thedomain:engineseat 1 (session_01JtSQQ3EWsbNPzyYMug1tXy) under claim 6292170810. The review of record is owed atCONTRACT_REVIEW_TIER, on the narrowing face.What changes
SqlDriver.introspectSchema, PostgreSQL branch only (packages/drivers/driver-sql/src/sql-driver.ts):table_schema(and its position incurrent_schemas(false)). Each table's columns are read with knexwithSchema(schema).columnInfo(), and the declared-order readintrospectColumnOrderusestable_schema = ?for that schema. Before, both read by bare name, which knex scopes tocurrent_schema(), the first schema on the path only.introspectColumns/introspectColumnOrdergain an optional trailingschemaparameter. Both areprotected, so no new public member. Every other caller passes none, and its SQL is byte-identical to before.search_pathreaches one bare table name in more than one schema,introspectSchemathrowsRESOURCE_CONFLICT/status: 409, naming each colliding table with its schemas insearch_pathorder. The message gives the remedy: one schema per connection (the datasource'sschemasetting sets itssearch_path), or a rename.publicdeployment), and the SQLite and MySQL branches. The listing query gains noORDER BY, so a single-schema answer keeps its key order.Not touched, per the claim: no
schemamember onIntrospectedTable, nopackages/specchange, no edit toservice-datasource(read only, as the consumer).Measured before and after (throwaway PostgreSQL 16.15 cluster, local
postgresbinary)The card's database:
public.customers,sales.orders,sales.customers, andALTER DATABASE … SET search_path = public, sales. It was read throughSqlDriver.introspectSchemaand throughExternalDatasourceService.listRemoteTables, the service class constructed with the driver as itsintrospect, with no server.fcb40ad673)1eeab8089)public, sales,customersin both)customers(public's 3 columns),orderswithcolumns: []andprimaryKeys: ['id'];sales.customersabsent;listRemoteTables→customerscolumnCount 3,orderscolumnCount 0RESOURCE_CONFLICT/ 409:Colliding: 'customers' ('public', 'sales'), from bothintrospectSchemaandlistRemoteTablessales.customersorders)ordersreads its 4 columns (id,customer_id,total,placed_at);listRemoteTables→ columnCount 3 and 4publiconly,customers+orderswith an FK)introspectedAtmasked)a273d463…f8a273d463…f8: byte-identicalThe collision answer: refusal (four axes)
Callers of
introspectSchema:ExternalDatasourceService(listRemoteTables,generateObjectDraft,refreshCatalog,validateObject*, andtestConnectionfor a non-primary datasource), throughObjectQL.introspectDatasourceor the driver directly (service-datasource/src/plugin.ts). Also the datasource admin plugin's connection test, only as a last fallback afterping/checkHealth, whichSqlDriverhas.driver-tursooverrides it (SQLite). No migration differ oroscommand calls it (packages/cli/src/utils/unmanaged-tables.tsmirrors its listing statement and does not call it). Every caller already surfaces a throw: the listing, draft and catalog doors return the error, andtestConnectionanswers{ ok: false, error }.search_pathnames two or more schemas holding a same-named table, which is the finder's measured shape. A single-schema connection can never collide, so no internal caller that sees one schema is refused.IntrospectedSchemacarries no warnings, and aschemamember onIntrospectedTableis the spec decision the triage withheld). That leaves a server log line, a workaround in all but name.Rejected third shape, recorded: keying the shadowed twin as a qualified
sales.customers. The consumers'parseQualifiedwould read it, but a key whose spelling depends on context is exactly what this file'sIntrospectedForeignKey.referencedTablenote rejects (#11377), and it is outside the two options the claim names.Pins
New
packages/drivers/driver-sql/src/sql-driver-24707-introspect-pg-multischema.test.ts, a live-PG cell declared throughdeclareDialectCell(a named skip withoutOS_TEST_POSTGRES_URL). In CI it runs inTemporal Conformance (live PG + MySQL)'s driver-sql step (ci.yml, "Run driver-sql suite against both live servers"), which is inmain's full battery. In PR runs the file is a named skip.columnInfo()reads zero columns for the later schema's table;{ code: 'RESOURCE_CONFLICT', status: 409 }, and the message names the table and both schemas;Ablation (committed fix,
node scripts/ablation-replace.mjs, one leg per mutation, each restored blob-equal toHEAD74cd151dd2bewithgit diff HEADempty; the test imports../src, so no dist leg):expected undefined to be defined), 1 failed / 3 passed.Pin sweep (
introspectSchema,introspectColumns,columnInfo,current_schemas,columnCountacrosspackages/,qa/,docs/qa/platform-checklist/,content/docs/): no existing pin moves.unmanaged-tables.test.tsassertscurrent_schemas(false)in the CLI's own mirror statement, which is unchanged. The checklist's external-datasource items run on a SQLite fixture, and their knownGaps text about bare names fromcurrent_schemas(false)stays true. Nodocs/qa/platform-checklist/**edit.Verification record (head
1eeab8089, after mergingorigin/maina8b44d407)pnpm --filter @objectstack/driver-sql typecheck: exit 0 (its tsconfig includessrc/**/*, so the new test is type-checked).pnpm --filter @objectstack/driver-sql test: exit 0, 224 files passed / 12 skipped, 3687 tests passed / 209 skipped (no live URL, as inTest Core). It ran onb61cf5026; the merge brought onlyplatform-objectsfiles.OS_TEST_POSTGRES_URLon the local cluster: exit 0, 170 passed / 10 skipped (MySQL cells, no MySQL URL).pnpm --filter @objectstack/service-datasource test(the consumer), after building its dependency closure: exit 0, 48 files / 862 tests passed.pnpm exec turbo run build --concurrency=2on the merged head: exit 0, 77/77 tasks.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack: 70 families, each run with its exit code captured before any pipe, all 0.check:dual-build-cjs-loadsfirst answered 3 (PREREQUISITE NOT MET, no dist) and was re-run after the full build: exit 0.check:type-check-debtfirst hit my per-command 280 s cap (124) while the box was busy and was re-run: exit 0.--ranreconciliation: 70 derived, 70 run, 0 NOT-MEASURED, all 70 carrying an exit code.pnpm --filter @objectstack/core test:repo: exit 0, 6 files / 82 tests passed.Acceptance notes
introspectSchemamakes after the columns (introspectPrimaryKeysby?::regclass,introspectForeignKeysandintrospectUniqueConstraintsscoped tocurrent_schemas(false)) are unchanged. With base-table collisions refused, each now answers for the one base table of that name on the path. One residual shape is a read-only inference, not measured: a VIEW (or other relation) named like a later schema's table but sitting in an earlier schema would still capture theregclassresolution of the primary-key read. Noted, not filed.unmanaged-tablessweep lists names across the same schemas and is untouched.Generated by Claude Code