Skip to content

Commit c1698df

Browse files
os-zhuangclaude
andauthored
fix(analytics): fall back to aggregate strategy when the driver can't run SQL (#1701)
Every dataset query on an in-memory-driver environment returned rows:[] with HTTP 200 and the compiled SQL attached — dashboards read "No rows" forever while looking healthy (live staging incident: even COUNT(*) FROM sys_user and queries against nonexistent tables came back empty). Root cause chain: InMemoryDriver.execute() returns null for raw SQL; the analytics plugin's auto-bridge mapped null to [] silently; and the default queryCapabilities unconditionally advertise nativeSql, so NativeSQLStrategy was always selected even on drivers that cannot execute SQL. Fix at both layers: - the raw-SQL bridge now throws a TYPED error (code RAW_SQL_UNSUPPORTED) when the engine returns null/undefined — an empty result is never fabricated; - AnalyticsService.query treats that error as a runtime capability miss and falls back to the next capable strategy (ObjectQLStrategy over the aggregate bridge — the in-memory driver implements aggregate()), skipping the proven-incapable one. Any other error propagates untouched, and when no fallback exists the failure is loud and names the skipped strategies. service-analytics: 109 tests green (3 new covering fallback, error passthrough, and the no-fallback loud failure). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent d251d95 commit c1698df

3 files changed

Lines changed: 114 additions & 8 deletions

File tree

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,3 +641,66 @@ describe('AnalyticsService', () => {
641641
expect(JSON.stringify(filterObj)).toContain('open');
642642
});
643643
});
644+
645+
// ─────────────────────────────────────────────────────────────────
646+
// Runtime strategy fallback — RAW_SQL_UNSUPPORTED (in-memory driver)
647+
// ─────────────────────────────────────────────────────────────────
648+
//
649+
// A driver that cannot run SQL (the in-memory driver) returns null from
650+
// execute(); the plugin's raw-SQL bridge surfaces that as a typed
651+
// RAW_SQL_UNSUPPORTED error. The orchestrator must fall back to the next
652+
// capable strategy (aggregate bridge) instead of failing — and must NEVER
653+
// fabricate an empty result (the "every dashboard reads No rows" bug).
654+
655+
describe('AnalyticsService — RAW_SQL_UNSUPPORTED fallback', () => {
656+
const rawUnsupported = () =>
657+
Object.assign(new Error('driver returned null for raw SQL'), { code: 'RAW_SQL_UNSUPPORTED' });
658+
659+
it('falls back from NativeSQL to the aggregate strategy and returns its rows', async () => {
660+
const executeRawSql = vi.fn().mockRejectedValue(rawUnsupported());
661+
const executeAggregate = vi.fn().mockResolvedValue([
662+
{ status: 'completed', count: 5 },
663+
]);
664+
const service = new AnalyticsService({
665+
cubes: [ordersCube],
666+
logger: silentLogger,
667+
executeRawSql,
668+
executeAggregate,
669+
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: true, inMemory: false }),
670+
});
671+
672+
const result = await service.query({ cube: 'orders', measures: ['count'], dimensions: ['status'] });
673+
674+
expect(executeRawSql).toHaveBeenCalledTimes(1);
675+
expect(executeAggregate).toHaveBeenCalledTimes(1);
676+
expect(Array.isArray(result.rows)).toBe(true);
677+
expect(result.rows.length).toBeGreaterThan(0);
678+
});
679+
680+
it('propagates any OTHER raw-SQL error untouched (no fallback masking)', async () => {
681+
const executeRawSql = vi.fn().mockRejectedValue(new Error('no such column: bogus'));
682+
const executeAggregate = vi.fn();
683+
const service = new AnalyticsService({
684+
cubes: [ordersCube],
685+
logger: silentLogger,
686+
executeRawSql,
687+
executeAggregate,
688+
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: true, inMemory: false }),
689+
});
690+
691+
await expect(service.query({ cube: 'orders', measures: ['count'] })).rejects.toThrow('no such column');
692+
expect(executeAggregate).not.toHaveBeenCalled();
693+
});
694+
695+
it('reports a clear error (never silent empty rows) when no fallback strategy exists', async () => {
696+
const executeRawSql = vi.fn().mockRejectedValue(rawUnsupported());
697+
const service = new AnalyticsService({
698+
cubes: [ordersCube],
699+
logger: silentLogger,
700+
executeRawSql,
701+
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }),
702+
});
703+
704+
await expect(service.query({ cube: 'orders', measures: ['count'] })).rejects.toThrow('No strategy can handle');
705+
});
706+
});

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

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,14 @@ export class AnalyticsService implements IAnalyticsService {
284284

285285
/**
286286
* Execute an analytical query by delegating to the first capable strategy.
287+
*
288+
* A strategy can discover only AT EXECUTION TIME that the underlying driver
289+
* cannot serve it — the canonical case is NativeSQLStrategy on an in-memory
290+
* driver, whose `execute()` returns null for raw SQL (the auto-bridge throws
291+
* `RAW_SQL_UNSUPPORTED`). That is a capability miss, not a query error: fall
292+
* back to the next capable strategy (e.g. ObjectQLStrategy over the
293+
* aggregate bridge) instead of failing — or worse, fabricating empty rows.
294+
* Any other error propagates untouched.
287295
*/
288296
async query(query: AnalyticsQuery, context?: ExecutionContext): Promise<AnalyticsResult> {
289297
if (!query.cube) {
@@ -292,10 +300,23 @@ export class AnalyticsService implements IAnalyticsService {
292300

293301
this.ensureCube(query);
294302
const ctx = await this.callCtx(query, context);
295-
const strategy = this.resolveStrategy(query, ctx);
296-
this.logger.debug(`[Analytics] Query on cube "${query.cube}" → ${strategy.name}`);
297-
298-
return strategy.execute(query, ctx);
303+
let skip: Set<AnalyticsStrategy> | undefined;
304+
for (;;) {
305+
const strategy = this.resolveStrategy(query, ctx, skip);
306+
this.logger.debug(`[Analytics] Query on cube "${query.cube}" → ${strategy.name}`);
307+
try {
308+
return await strategy.execute(query, ctx);
309+
} catch (e) {
310+
if ((e as { code?: string })?.code === 'RAW_SQL_UNSUPPORTED') {
311+
this.logger.warn(
312+
`[Analytics] ${strategy.name} cannot run on this driver (raw SQL unsupported) — falling back to the next strategy.`,
313+
);
314+
(skip ??= new Set()).add(strategy);
315+
continue;
316+
}
317+
throw e;
318+
}
319+
}
299320
}
300321

301322
/**
@@ -508,17 +529,24 @@ export class AnalyticsService implements IAnalyticsService {
508529
}
509530

510531
/**
511-
* Walk the strategy chain and return the first strategy that can handle the query.
532+
* Walk the strategy chain and return the first strategy that can handle the
533+
* query. `skip` excludes strategies that already proved incapable at
534+
* execution time (see {@link query}'s RAW_SQL_UNSUPPORTED fallback).
512535
*/
513-
private resolveStrategy(query: AnalyticsQuery, ctx: StrategyContext): AnalyticsStrategy {
536+
private resolveStrategy(
537+
query: AnalyticsQuery,
538+
ctx: StrategyContext,
539+
skip?: Set<AnalyticsStrategy>,
540+
): AnalyticsStrategy {
514541
for (const strategy of this.strategies) {
542+
if (skip?.has(strategy)) continue;
515543
if (strategy.canHandle(query, ctx)) {
516544
return strategy;
517545
}
518546
}
519547
throw new Error(
520548
`[Analytics] No strategy can handle query for cube "${query.cube}". ` +
521-
`Checked: ${this.strategies.map(s => s.name).join(', ')}. ` +
549+
`Checked: ${this.strategies.map(s => s.name).join(', ')}${skip?.size ? ` (skipped at runtime: ${[...skip].map((s) => s.name).join(', ')})` : ''}. ` +
522550
'Ensure a compatible driver is configured or a fallback service is registered.',
523551
);
524552
}

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,23 @@ export class AnalyticsServicePlugin implements Plugin {
211211
// driver-sql) speaks `?` placeholders, so translate.
212212
const knexSql = sql.replace(/\$(\d+)/g, '?');
213213
const result = await engine.execute(knexSql, { args: params });
214+
// A driver that cannot run SQL (e.g. the in-memory driver) returns
215+
// null from execute(). Silently mapping that to [] made EVERY dataset
216+
// query on such environments report "No rows" while looking healthy
217+
// (HTTP 200, compiled SQL attached). Throw a TYPED error instead so
218+
// the orchestrator can fall back to an aggregate-based strategy —
219+
// never fabricate an empty result.
220+
if (result === null || result === undefined) {
221+
const err = new Error(
222+
'[Analytics] The "data" engine\'s driver returned null for raw SQL — ' +
223+
'this driver does not support SQL execution. The query will fall back ' +
224+
'to an aggregate-based strategy when one is available.',
225+
) as Error & { code: string };
226+
err.code = 'RAW_SQL_UNSUPPORTED';
227+
throw err;
228+
}
214229
if (Array.isArray(result)) return result as Record<string, unknown>[];
215-
if (result && typeof result === 'object' && 'rows' in (result as Record<string, unknown>)) {
230+
if (typeof result === 'object' && 'rows' in (result as Record<string, unknown>)) {
216231
return (result as { rows: Record<string, unknown>[] }).rows;
217232
}
218233
return [];

0 commit comments

Comments
 (0)