Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/adr-0015-federation-read-path.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
---
"@objectstack/driver-sql": patch
"@objectstack/objectql": patch
"@objectstack/spec": patch
---

fix(ADR-0015): honor `external.remoteName` / `external.remoteSchema` on the federation read path.

The query path previously resolved an external object's physical table from the
object name, ignoring its `external` binding — so a federated object bound to a
differently-named remote table failed with `no such table`, and ADR-0015's own
`wh_order` → `mart.fact_orders` example was unqueryable. The SQL driver now
resolves the remote table (`remoteName`, plus `remoteSchema` via `.withSchema()`
on pg/mysql) and registers external objects' read-coercion metadata without DDL
(`SqlDriver.registerExternalObject`, routed from the engine/plugin schema-sync).
The managed path is unchanged. See ADR-0015 §18.
84 changes: 84 additions & 0 deletions docs/adr/0015-external-datasource-federation.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -865,3 +865,87 @@ The ADR is considered "delivered" when:
- `packages/plugins/driver-sql/src/sql-driver.ts` (introspectSchema, createTable, alterTable)
- `packages/services/service-ai/src/tools/query-data.tool.ts`
- `packages/services/service-ai/src/schema-retriever.ts`

---

## 18. Addendum (2026-06): Federation read path honours `remoteName` / `remoteSchema`

**Status:** accepted — implements §3.1 / §8 of this ADR that shipped only partially.

### Problem

The spec, introspection (`os datasource introspect`), boot validation (§5.2), and
the write gate (§5.3) all honoured an object's `external.remoteName` /
`external.remoteSchema`. **The query-execution path did not.** Reads resolved the
physical table from the *object name* alone, ignoring the binding — so the
canonical example in §8 (object `wh_order` → `external: { remoteSchema: 'mart',
remoteName: 'fact_orders' }`) could not actually be queried.

Reproduced with the real engine + a real better-sqlite3 driver: an object
`ext_customer` bound to remote table `remote_customers` via `remoteName` failed
with `no such table: ext_customer`; only naming the object `remote_customers`
returned rows. This is a correctness gap, not a feature request — the declared
contract validated green and then queried the wrong (non-existent) table.

### Root cause

- `SqlDriver.getBuilder(object)` did `this.knex(object)` — the object name *was*
the physical table.
- Per-object read-coercion metadata (`json`/`boolean`/`numeric`/`date`/`datetime`)
is populated only inside `initObjects()`, whose first statement is
`assertSchemaMutable()` (the DDL gate, §5.1) → it throws for any
`schemaMode !== 'managed'` driver. The boot schema-sync swallowed that throw
per-object, leaving external objects with **zero** coercion metadata and no
table-name mapping.

### Fix

- **`SqlDriver.registerExternalObject(schema)`** — a DDL-free counterpart to
`initObjects()`. It records the physical remote table
(`physicalTableByObject` / `physicalSchemaByObject`) and populates the same
coercion maps, keyed by **object name** (matching `formatInput`/`formatOutput`/
`coerceFilterValue`), without running any DDL.
- **`getBuilder()`** now resolves `physicalTableByObject[object] ?? object`, and
applies `.withSchema()` when a remote schema is recorded. Managed objects miss
both maps, so the path is unchanged (one `undefined` lookup).
- **Coercion re-keying** — `applyFilters`/`applyFilterCondition` map the builder's
physical table back to the object name (`coercionKey`) so date/datetime filter
coercion still resolves after the table switch.
- **Engine/plugin routing** — `ObjectQLPlugin.syncRegisteredSchemas` (boot) and
`ObjectQL.syncObjectSchema` (on-demand) route objects with `external != null`
to `driver.registerExternalObject()` instead of the DDL `syncSchema`. The
on-demand path lets an app register a *late* external driver (one added via an
`onEnable` hook) and then make its objects queryable.
- **Driver contract** — `IDataDriver.registerExternalObject?()` is declared
optional, so non-SQL drivers degrade gracefully (the engine skips external
objects they can't serve).

### Scope

- `remoteName` is honoured on **all** dialects.
- `remoteSchema` is applied via `knex.withSchema()` on Postgres / MySQL; on
**SQLite it is a no-op** (no schema namespace) and logs a one-time warning.
- External reads use **best-effort coercion**: with no DDL/`columnInfo`, coercion
is driven purely by the declared field types. Keep external object fields to
well-understood scalar types.

### Explicitly out of scope (separate follow-ups)

1. **`external.columnMap`** (remote column name ≠ local field key). The driver's
`select` / `where` / `orderBy` do not currently apply column-name translation,
and `columnMap` is the inverse of per-field `field.columnName` — reconciling
the two into one source of truth is its own change.
2. **Native-analytics SQL over external objects** — the analytics service compiles
its own `FROM "<table>"` outside the driver and needs the same remote-table
awareness.
3. **Auto-connecting declared datasources** as queryable ObjectQL drivers in the
standalone runtime. Today a declared non-default datasource appears in the
metadata registry (Setup → Datasources) but is only made queryable by
registering a live driver under its name (e.g. via an app `onEnable` hook).

### Tests

`packages/plugins/driver-sql/src/sql-driver-external-remote-name.test.ts` (read /
filter / coercion against a differently-named remote table, no DDL leakage) and an
added case in `sql-driver-ddl-gate.test.ts` (`registerExternalObject` is DDL-free).
File-based better-sqlite3 tests require Node ≥ 25 (ABI 141) in this repo.
13 changes: 12 additions & 1 deletion packages/objectql/src/engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2815,7 +2815,18 @@ export class ObjectQL implements IDataEngine {
const obj = this._registry.getObject(objectName) as any;
if (!obj) return;
const driver = this.getDriverForObject(objectName);
if (!driver || typeof (driver as any).syncSchema !== 'function') return;
if (!driver) return;
// Federated (external) object (ADR-0015): register read metadata WITHOUT DDL
// (its remote schema is owned externally). This is what an app's onEnable
// calls after registering a late external driver so coercion maps + the
// physical-table mapping exist for queries. See SqlDriver.registerExternalObject.
if (obj.external != null) {
if (typeof (driver as any).registerExternalObject === 'function') {
await (driver as any).registerExternalObject(obj);
}
return;
}
if (typeof (driver as any).syncSchema !== 'function') return;
const tableName = StorageNameMapping.resolveTableName(obj);
await (driver as any).syncSchema(tableName, obj);
}
Expand Down
27 changes: 27 additions & 0 deletions packages/objectql/src/plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -698,6 +698,33 @@ export class ObjectQLPlugin implements Plugin {
continue;
}

// Federated (external) objects (ADR-0015): their schema is owned by the
// remote database, so DDL (syncSchema/initObjects) is forbidden and would
// throw. Register read metadata (physical remote table + coercion maps)
// without DDL so the query path resolves to the remote table, then skip
// the DDL grouping below.
if (obj.external != null) {
if (typeof driver.registerExternalObject === 'function') {
try {
await driver.registerExternalObject(obj);
synced++;
} catch (e: unknown) {
ctx.logger.warn('Failed to register external object metadata', {
object: obj.name,
driver: driver.name,
error: e instanceof Error ? e.message : String(e),
});
}
} else {
ctx.logger.debug('Driver does not support registerExternalObject, skipping external object', {
object: obj.name,
driver: driver.name,
});
skipped++;
}
continue;
}

if (typeof driver.syncSchema !== 'function') {
ctx.logger.debug('Driver does not support syncSchema, skipping', {
object: obj.name,
Expand Down
15 changes: 15 additions & 0 deletions packages/plugins/driver-sql/src/sql-driver-ddl-gate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -68,6 +68,21 @@ describe('SqlDriver DDL gate (ADR-0015)', () => {
);
});

it('registerExternalObject is DDL-free: does not throw on external mode and creates no table', async () => {
driver = makeDriver('external');
expect(() =>
driver.registerExternalObject!({
name: 'ext_widget',
external: { remoteName: 'widgets' },
fields: { sku: { type: 'text' } },
}),
).not.toThrow();
const k = (driver as any).knex;
// No DDL ran — neither the object name nor the remote name was created.
expect(await k.schema.hasTable('ext_widget')).toBe(false);
expect(await k.schema.hasTable('widgets')).toBe(false);
});

it('also blocks DDL in validate-only mode', async () => {
driver = makeDriver('validate-only');
await expect(
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

/**
* Federation read-path tests (ADR-0015 addendum).
*
* Before this fix the query path resolved an external object to a table named
* after the OBJECT, ignoring `external.remoteName` — so `find('ext_customer')`
* threw `no such table: ext_customer` even though the object bound to a real
* remote table `remote_customers`. ADR-0015's own canonical example
* (`wh_order` → `mart.fact_orders`) was therefore broken.
*
* These tests stand up one sqlite file as the "remote" database (populated with
* a managed driver), then open a second `schemaMode: 'external'` driver over the
* same file and assert that an object whose name differs from the remote table
* is fully queryable — with read coercion (boolean/json/date) working even
* though no DDL ran for the external object.
*/

import { describe, it, expect, afterAll } from 'vitest';
import { rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { SqlDriver } from '../src/index.js';
import { ExternalSchemaModeViolationError } from '@objectstack/spec/shared';

const FIELDS = {
name: { type: 'text' },
flag: { type: 'boolean' },
meta: { type: 'json' },
when: { type: 'date' },
amount: { type: 'number' },
seen_at: { type: 'datetime' },
} as const;

let file: string;

afterAll(() => {
if (file) {
try { rmSync(file, { force: true }); } catch { /* ignore */ }
}
});

async function seedRemote(path: string) {
const fixture = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: path },
useNullAsDefault: true,
});
(fixture as any).name = 'fixture';
await fixture.connect?.();
// Physical table name deliberately differs from the object name used later.
await (fixture as any).initObjects([{ name: 'remote_customers', fields: FIELDS }]);
await (fixture as any).create('remote_customers', {
id: 'c1', name: 'Acme', flag: true, meta: { tier: 'gold' },
when: '2026-01-01', amount: 100, seen_at: new Date('2026-01-02T10:00:00.000Z'),
});
await (fixture as any).create('remote_customers', {
id: 'c2', name: 'Globex', flag: false, meta: { tier: 'silver' },
when: '2026-02-15', amount: 250, seen_at: new Date('2026-02-16T08:00:00.000Z'),
});
await fixture.disconnect?.();
}

function externalDriver(path: string): SqlDriver {
const ext = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: path },
useNullAsDefault: true,
schemaMode: 'external',
} as any);
(ext as any).name = 'extds';
return ext;
}

describe('SqlDriver external read path — remoteName resolution (ADR-0015)', () => {
it('queries a remote table whose name differs from the object name, with coercion', async () => {
file = join(tmpdir(), `os-ext-read-${process.pid}-${Date.now()}.db`);
await seedRemote(file);

const ext = externalDriver(file);
await ext.connect?.();
try {
// DDL-free registration — must NOT throw (unlike initObjects/syncSchema).
expect(() =>
ext.registerExternalObject!({ name: 'ext_customer', external: { remoteName: 'remote_customers' }, fields: FIELDS as any }),
).not.toThrow();

// The bug: this used to throw `no such table: ext_customer`.
const rows = await ext.find('ext_customer', {} as any);
expect(rows).toHaveLength(2);

const acme = rows.find((r: any) => r.name === 'Acme');
expect(acme).toBeTruthy();
// Coercion populated despite no DDL having run for the external object:
expect(acme.flag).toBe(true); // boolean (stored 1/0)
expect(acme.meta).toEqual({ tier: 'gold' }); // json (stored as text)
expect(acme.when).toBe('2026-01-01'); // date → YYYY-MM-DD
expect(typeof acme.amount).toBe('number'); // numeric scalar

// count + findOne route to the remote table too.
expect(await ext.count('ext_customer', {} as any)).toBe(2);
const one = await ext.findOne('ext_customer', { where: { name: 'Globex' } } as any);
expect(one?.name).toBe('Globex');

// Filtered reads hit the remote table.
const filtered = await ext.find('ext_customer', { where: { name: 'Acme' } } as any);
expect(filtered).toHaveLength(1);
expect(filtered[0].name).toBe('Acme');

// Date filter — guards the coercion re-keying (§3): coercion maps are keyed
// by the OBJECT name even though the builder now targets the remote table.
const byDate = await ext.find('ext_customer', { where: { when: '2026-02-15' } } as any);
expect(byDate).toHaveLength(1);
expect(byDate[0].name).toBe('Globex');

// Datetime filter — the SQLite epoch-affinity case the §3 trap would break
// if coercion were keyed by the physical (remote) name instead of object.
const byDatetime = await ext.find('ext_customer', { where: { seen_at: '2026-01-02T10:00:00.000Z' } } as any);
expect(byDatetime.map((r: any) => r.name)).toContain('Acme');

// No object-named table was ever created in the remote db (no DDL leaked).
const k = (ext as any).knex;
expect(await k.schema.hasTable('ext_customer')).toBe(false);
expect(await k.schema.hasTable('remote_customers')).toBe(true);
} finally {
await ext.disconnect?.();
}
});

it('still throws on initObjects/syncSchema (DDL) for external objects', async () => {
const ext = externalDriver(file);
await ext.connect?.();
try {
await expect(
ext.initObjects([{ name: 'ext_customer', fields: FIELDS as any }]),
).rejects.toBeInstanceOf(ExternalSchemaModeViolationError);
} finally {
await ext.disconnect?.();
}
});

it('an object whose name equals the remote table also works (no remap)', async () => {
const ext = externalDriver(file);
await ext.connect?.();
try {
ext.registerExternalObject!({ name: 'remote_customers', external: {}, fields: FIELDS as any });
const rows = await ext.find('remote_customers', {} as any);
expect(rows.length).toBe(2);
} finally {
await ext.disconnect?.();
}
});

it('treats remoteSchema as a no-op on sqlite (bare table)', async () => {
const ext = externalDriver(file);
await ext.connect?.();
try {
ext.registerExternalObject!({ name: 'ext_cust2', external: { remoteName: 'remote_customers', remoteSchema: 'mart' }, fields: FIELDS as any });
const rows = await ext.find('ext_cust2', {} as any);
expect(rows.length).toBe(2);
} finally {
await ext.disconnect?.();
}
});
});
Loading
Loading