Skip to content

Commit c7e5f4b

Browse files
os-zhuangclaude
andcommitted
feat(analytics): route external objects through driver-correct path (ADR-0062 Phase 3, D6)
NativeSQLStrategy hand-compiles FROM/columns that bypass the driver's physical resolution (remoteName/remoteSchema/columnMap), so it would aggregate against the wrong table for a federated object. It now declines any query whose base or joined object is external (new optional StrategyContext.isExternalObject hook, reported by the analytics plugin from the object's external block), routing it to the ObjectQLStrategy whose engine.aggregate goes through the driver's getBuilder (#2138/#2149). Reuses the driver's resolution rather than re-implementing it; until a native-SQL fast path exists, external analytics is correct (via ObjectQL) instead of silently wrong. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 688a28b commit c7e5f4b

7 files changed

Lines changed: 134 additions & 0 deletions

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/service-analytics": minor
4+
---
5+
6+
feat(analytics): correct analytics over federated objects (ADR-0062 Phase 3, D6)
7+
8+
Analytics over an external (federated) object now aggregates against the
9+
**correct** remote table instead of silently querying the wrong one. The
10+
`NativeSQLStrategy` hand-compiles `FROM "<object>"` and bare column references,
11+
which bypass the driver's physical-table resolution (`external.remoteName` /
12+
`remoteSchema` / `columnMap`). It now **declines** any query whose base or joined
13+
object is federated, routing it to the `ObjectQLStrategy` — whose
14+
`engine.aggregate()` goes through the driver's `getBuilder` and already honours
15+
`remoteName`/`remoteSchema` (#2138/#2149). This "reuses the driver's resolution"
16+
(D6) rather than re-implementing it.
17+
18+
Adds an optional `StrategyContext.isExternalObject(objectName)` hook (reported by
19+
the analytics plugin from the object's `external` block). Purely additive — with
20+
no hook, behavior is unchanged for managed objects.

docs/adr/0062-external-datasource-runtime.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ Code-defined datasources surface in `GET /api/v1/datasources`, `GET /api/v1/meta
7777

7878
The analytics native-SQL strategy compiles its own `FROM "<table>"` / column references outside the driver (ADR-0015 §18 noted this). It must resolve an external object's physical table (`remoteName`/`remoteSchema`) and columns (`columnMap`) the same way `SqlDriver` now does — reusing the driver's resolution (e.g. an exposed `physicalTableFor(object)` / `physicalColumnFor(object, field)`), not a second copy. Until then, analytics over external objects stays disabled rather than silently querying the wrong table.
7979

80+
> **Phase 3 implementation note (#2163) — "reuse the driver's resolution" = route external objects to the ObjectQL path.** Rather than re-implement `remoteName`/`remoteSchema`/`columnMap` resolution inside the native-SQL strategy (a second copy — explicitly rejected), `NativeSQLStrategy.canHandle` now **declines** any query whose base or joined object is federated (new optional `StrategyContext.isExternalObject` hook, reported by the analytics plugin from the object's `external` block). Declining routes the query to the lower-priority `ObjectQLStrategy`, whose `engine.aggregate()` already goes through the driver's `getBuilder` — which honours `remoteName`/`remoteSchema` (#2138/#2149). So external analytics aggregates against the **correct** remote table via the single source of truth (the driver), and native-SQL never queries the wrong table. (A native-SQL fast path for external objects can be added later by exposing `physicalTableFor`/`physicalColumnFor` on the driver; deeper `columnMap`-in-`GROUP BY` support is a separate driver concern.)
81+
8082
### D7 — `columnMap` is the external mechanism; reconcile `field.columnName`
8183

8284
`external.columnMap` ({ remoteColumn → localField }) is the supported way to map external columns (shipped #2149). `field.columnName` (localField → physicalColumn) is its inverse and is **not** applied by the driver's query pipeline for external objects. Decision: for external objects, `columnMap` is authoritative; `field.columnName` on an external object is rejected at validation (no silent dual-source) until a unified column-resolution model is designed. Managed objects' `field.columnName` semantics are untouched.
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// ADR-0062 D6 — the NativeSQLStrategy must DECLINE federated (external-datasource)
4+
// objects: its hand-compiled `FROM "<object>"` / bare column refs bypass the
5+
// driver's physical-table resolution (remoteName/remoteSchema/columnMap) and
6+
// would query the wrong table. Declining routes the query to the lower-priority
7+
// ObjectQL aggregate path, which goes through the driver's getBuilder (correct).
8+
9+
import { describe, it, expect } from 'vitest';
10+
import { NativeSQLStrategy } from '../strategies/native-sql-strategy.js';
11+
import type { StrategyContext } from '../strategies/types.js';
12+
13+
const baseCaps = { nativeSql: true, objectqlAggregate: true, inMemory: false };
14+
15+
function ctxFor(cube: any, isExternalObject?: (o: string) => boolean): StrategyContext {
16+
return {
17+
getCube: () => cube,
18+
queryCapabilities: () => baseCaps,
19+
executeRawSql: async () => [],
20+
...(isExternalObject ? { isExternalObject } : {}),
21+
} as unknown as StrategyContext;
22+
}
23+
24+
const query = { cube: 'c', measures: ['cnt'], dimensions: ['region'] } as any;
25+
26+
describe('NativeSQLStrategy external-object gate (ADR-0062 D6)', () => {
27+
const strategy = new NativeSQLStrategy();
28+
29+
it('DECLINES when the base object is external', () => {
30+
const cube = { name: 'c', sql: 'ext_customer', dimensions: {}, measures: {} };
31+
const ctx = ctxFor(cube, (o) => o === 'ext_customer');
32+
expect(strategy.canHandle(query, ctx)).toBe(false);
33+
});
34+
35+
it('ACCEPTS a managed object (native-SQL still used)', () => {
36+
const cube = { name: 'c', sql: 'account', dimensions: {}, measures: {} };
37+
const ctx = ctxFor(cube, () => false);
38+
expect(strategy.canHandle(query, ctx)).toBe(true);
39+
});
40+
41+
it('DECLINES when a JOINED object is external (the join would hit the wrong table)', () => {
42+
const cube = { name: 'c', sql: 'orders', joins: { customer: { name: 'ext_customer' } }, dimensions: {}, measures: {} };
43+
const ctx = ctxFor(cube, (o) => o === 'ext_customer');
44+
expect(strategy.canHandle(query, ctx)).toBe(false);
45+
});
46+
47+
it('is purely additive — with no isExternalObject hook it behaves as before (accept)', () => {
48+
const cube = { name: 'c', sql: 'ext_customer', dimensions: {}, measures: {} };
49+
const ctx = ctxFor(cube); // no hook
50+
expect(strategy.canHandle(query, ctx)).toBe(true);
51+
});
52+
});

packages/services/service-analytics/src/analytics-service.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,14 @@ export interface AnalyticsServiceConfig {
135135
* `StrategyContext.coerceTemporalFilterValue` for the full rationale.
136136
*/
137137
coerceTemporalFilterValue?: (objectName: string, fieldName: string, value: unknown) => unknown;
138+
/**
139+
* ADR-0062 D6 — report whether an object is federated (external datasource).
140+
* Threaded into the StrategyContext so `NativeSQLStrategy` declines external
141+
* objects (which it would otherwise query against the wrong physical table),
142+
* routing them to the driver-correct ObjectQL aggregate path instead. See
143+
* `StrategyContext.isExternalObject`.
144+
*/
145+
isExternalObject?: (objectName: string) => boolean;
138146
/**
139147
* ADR-0021 — optional object-graph resolver used when compiling datasets:
140148
* `(baseObject, relationshipName) => relatedObjectName | undefined`. When
@@ -260,6 +268,7 @@ export class AnalyticsService implements IAnalyticsService {
260268
this.datasetRegistry.get(cubeName)?.allowedRelationships
261269
?? config.getAllowedRelationships?.(cubeName),
262270
coerceTemporalFilterValue: config.coerceTemporalFilterValue,
271+
isExternalObject: config.isExternalObject,
263272
};
264273

265274
// Build strategy chain (built-in + custom, sorted by priority)

packages/services/service-analytics/src/plugin.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ interface DataEngineLike {
3232
reference?: string;
3333
options?: Array<{ value: unknown; label?: string }>;
3434
}>;
35+
/** Federation marker (ADR-0015): set on objects bound to an external datasource. */
36+
external?: unknown;
37+
/** The datasource this object is bound to (ADR-0062 D6 external detection). */
38+
datasource?: string;
3539
} | undefined;
3640
/**
3741
* Resolve the storage driver backing an object (public ObjectQL accessor).
@@ -443,6 +447,13 @@ export class AnalyticsServicePlugin implements Plugin {
443447
| undefined;
444448
return f ? { type: f.type, defaultCurrency: f.currencyConfig?.defaultCurrency } : undefined;
445449
},
450+
// ADR-0062 D6 — a federated object carries an `external` block (ADR-0015).
451+
// Reported so NativeSQLStrategy declines it (its hand-compiled FROM would
452+
// hit the wrong physical table) and the driver-correct ObjectQL path runs.
453+
isExternalObject: (objectName: string) => {
454+
const obj = dataEngine()?.getObject?.(objectName);
455+
return !!(obj && obj.external != null);
456+
},
446457
draftRowsResolver,
447458
};
448459

packages/services/service-analytics/src/strategies/native-sql-strategy.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,26 @@ export class NativeSQLStrategy implements AnalyticsStrategy {
2828
// query silently grouped by the raw timestamp — one bucket per row — and a
2929
// non-UTC reference timezone was ignored entirely (ADR-0053 Phase 2, #1982).
3030
if (query.timeDimensions?.some((td) => !!td.granularity)) return false;
31+
// ADR-0062 D6 — DECLINE federated (external-datasource) objects. This
32+
// strategy hand-compiles `FROM "<object>"` and bare column references, which
33+
// bypass the driver's physical-table resolution (`external.remoteName` /
34+
// `remoteSchema` / `columnMap`) and would query the WRONG table. Routing the
35+
// query to the lower-priority ObjectQL aggregate path keeps it correct —
36+
// that path goes through the driver's `getBuilder` (#2138/#2149). Applies to
37+
// the base object AND any joined object (a join would also hit the wrong
38+
// table). Until native-SQL learns the driver's resolution, "disabled" beats
39+
// "silently wrong".
40+
if (typeof ctx.isExternalObject === 'function') {
41+
const cube = ctx.getCube(query.cube);
42+
if (cube) {
43+
if (ctx.isExternalObject(this.extractObjectName(cube))) return false;
44+
const joinTargets = cube.joins ? Object.values(cube.joins) : [];
45+
for (const j of joinTargets) {
46+
const joinedObject = (j as { name?: string })?.name;
47+
if (joinedObject && ctx.isExternalObject(joinedObject)) return false;
48+
}
49+
}
50+
}
3151
const caps = ctx.queryCapabilities(query.cube);
3252
return caps.nativeSql && typeof ctx.executeRawSql === 'function';
3353
}

packages/spec/src/contracts/analytics-service.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,26 @@ export interface StrategyContext {
322322
* @param value The stringified comparand from the normalized filter.
323323
*/
324324
coerceTemporalFilterValue?(objectName: string, fieldName: string, value: unknown): unknown;
325+
326+
/**
327+
* ADR-0062 D6 — is `objectName` a federated (external-datasource) object?
328+
*
329+
* The `NativeSQLStrategy` compiles its own `FROM "<object>"` and column
330+
* references, which bypass the driver's physical-table resolution and so
331+
* would query the WRONG table for a federated object whose `external.remoteName`
332+
* / `remoteSchema` / `columnMap` differ from the logical object/field names.
333+
* Until native-SQL learns the driver's physical resolution, the strategy
334+
* DECLINES external objects (see its `canHandle`), so they fall through to the
335+
* ObjectQL aggregate path — which routes through the driver's `getBuilder`
336+
* (honouring `remoteName`/`remoteSchema`, #2138/#2149). This keeps external
337+
* analytics correct ("reuse the driver's resolution") rather than silently
338+
* querying the wrong table.
339+
*
340+
* Returns `true` for a federated object, `false`/`undefined` otherwise. When
341+
* the hook is absent (legacy wiring) the strategy assumes non-external —
342+
* purely additive, no behavior change for managed objects.
343+
*/
344+
isExternalObject?(objectName: string): boolean;
325345
}
326346

327347
/**

0 commit comments

Comments
 (0)