Skip to content

Commit c3616bc

Browse files
os-zhuangclaude
andcommitted
fix(driver-sql): an unknown $select column must not zero the result set
find() swallowed any "no such column" error into an empty array. A projected $select naming a column the table lacks (e.g. a generic list view auto- requesting status/due_date/image on an object without them) then made the WHOLE query return zero rows — reading to the UI as "no records exist" while the data was actually there: a silent data-loss footgun. When the failure came from the projection, retry once with SELECT * so the real rows still come back (the phantom field is simply absent from each row). Non-projection errors (unknown table, etc.) still surface as before. The engine's unknown-field filter is the first line of defense, but it only fires when the object's schema is populated in the registry; this driver backstop holds even when it isn't (notably the cloud multi-tenant runtime). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 05717ed commit c3616bc

2 files changed

Lines changed: 75 additions & 25 deletions

File tree

packages/plugins/driver-sql/src/sql-driver.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,27 @@ describe('SqlDriver (SQLite Integration)', () => {
4949
expect(results.map((r: any) => r.name)).toEqual(['Alice', 'Charlie']);
5050
});
5151

52+
it('returns rows when $select names an unknown column (retries with SELECT *)', async () => {
53+
// A generic list view auto-requests view-binding fields (status, due_date,
54+
// image, ...) the object may not have. The unknown column must NOT zero
55+
// the whole result — the real rows still come back, minus the phantom field.
56+
const results = await driver.find('users', {
57+
fields: ['id', 'name', 'status', 'due_date', 'image'],
58+
orderBy: [{ field: 'name', order: 'asc' }],
59+
});
60+
61+
expect(results.length).toBe(4);
62+
expect(results.map((r: any) => r.name)).toEqual(['Alice', 'Bob', 'Charlie', 'Dave']);
63+
// The phantom columns are simply absent (the table never had them).
64+
expect((results[0] as any).status).toBeUndefined();
65+
});
66+
67+
it('still surfaces non-column errors (e.g. unknown table) instead of empty', async () => {
68+
await expect(
69+
driver.find('no_such_table', { fields: ['id'] }),
70+
).rejects.toThrow();
71+
});
72+
5273
it('should apply simple AND/OR logic', async () => {
5374
const results = await driver.find('users', {
5475
where: {

packages/plugins/driver-sql/src/sql-driver.ts

Lines changed: 54 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -362,8 +362,34 @@ export class SqlDriver implements IDataDriver {
362362
// ===================================
363363

364364
async find(object: string, query: QueryAST, options?: DriverOptions): Promise<any[]> {
365-
const builder = this.getBuilder(object, options);
366-
this.applyTenantScope(builder, object, options);
365+
// Build everything EXCEPT the SELECT list, so the unknown-column retry
366+
// below can rebuild without re-deriving where/order/pagination.
367+
const buildBase = () => {
368+
const b = this.getBuilder(object, options);
369+
this.applyTenantScope(b, object, options);
370+
371+
// WHERE
372+
if (query.where) {
373+
this.applyFilters(b, query.where);
374+
}
375+
376+
// ORDER BY
377+
if (query.orderBy && Array.isArray(query.orderBy)) {
378+
for (const item of query.orderBy) {
379+
if (item.field) {
380+
b.orderBy(this.mapSortField(item.field), item.order || 'asc');
381+
}
382+
}
383+
}
384+
385+
// PAGINATION
386+
if (query.offset !== undefined) b.offset(query.offset);
387+
if (query.limit !== undefined) b.limit(query.limit);
388+
389+
return b;
390+
};
391+
392+
const builder = buildBase();
367393

368394
// SELECT
369395
if (query.fields) {
@@ -372,36 +398,39 @@ export class SqlDriver implements IDataDriver {
372398
builder.select('*');
373399
}
374400

375-
// WHERE
376-
if (query.where) {
377-
this.applyFilters(builder, query.where);
378-
}
379-
380-
// ORDER BY
381-
if (query.orderBy && Array.isArray(query.orderBy)) {
382-
for (const item of query.orderBy) {
383-
if (item.field) {
384-
builder.orderBy(this.mapSortField(item.field), item.order || 'asc');
385-
}
386-
}
387-
}
388-
389-
// PAGINATION
390-
if (query.offset !== undefined) builder.offset(query.offset);
391-
if (query.limit !== undefined) builder.limit(query.limit);
392-
393401
let results: any[];
394402
try {
395403
results = await builder;
396404
} catch (error: any) {
397-
if (
405+
const isUnknownColumn =
398406
error.message &&
399407
(error.message.includes('no such column') ||
400-
(error.message.includes('column') && error.message.includes('does not exist')))
401-
) {
402-
return [];
408+
(error.message.includes('column') && error.message.includes('does not exist')));
409+
if (isUnknownColumn) {
410+
// A `$select` projection naming a column the table lacks (e.g. a
411+
// generic list view auto-requesting `status`/`due_date`/`image` on an
412+
// object without them) makes the WHOLE query fail. Swallowing that
413+
// into an empty result — the old behavior — reads to the UI as "no
414+
// records exist" even though rows are there: a silent data-loss
415+
// footgun. When the failure came from the projection, retry once
416+
// selecting all columns so the real rows still come back; the unknown
417+
// field is simply absent from each row (it never existed). The
418+
// engine's unknown-field filter is the first line of defense, but it
419+
// only fires when the object's schema is populated in the registry —
420+
// this driver backstop holds even when it isn't (notably the cloud
421+
// multi-tenant runtime, where the projection otherwise zeroes the list).
422+
if (query.fields) {
423+
try {
424+
results = await buildBase().select('*');
425+
} catch {
426+
return [];
427+
}
428+
} else {
429+
return [];
430+
}
431+
} else {
432+
throw error;
403433
}
404-
throw error;
405434
}
406435

407436
if (!Array.isArray(results)) {

0 commit comments

Comments
 (0)