From 2d09399e474635538eca8756a94068ab9cd5fbf0 Mon Sep 17 00:00:00 2001 From: Matthew Rathbone Date: Tue, 13 Oct 2026 17:03:58 +0000 Subject: [PATCH] Reject SELECT columns that no document has MongoDB returns an empty document for each match when a projected field doesn't exist, so SELECT invalid_lmao FROM users returned one blank row per document instead of an error (#7). - The compiler records the fields a SELECT list reads on FIND and AGGREGATE commands (selectedFields). Table qualifiers are dropped, paths stop at the first array index, and JOINs are skipped. - The executor throws 'Column "x" does not exist in collection "y"' when no document in the collection has a selected field. execute() only looks up fields that none of the returned rows have a value for; executeCursor() checks before returning the cursor. Fields that only some documents have, and empty collections, are still allowed. - DummyCollection gets a findOne, which the executor already needed for ObjectId detection. Claude-Session: https://claude.ai/code/session_013yzyAbpCE94cknaLBFAtZ7 --- packages/lib/src/compiler.ts | 28 +++++++ packages/lib/src/executor/dummy-client.ts | 17 ++++ packages/lib/src/executor/index.ts | 83 ++++++++++++++++++- packages/lib/src/interfaces.ts | 10 +++ .../regressions.integration.test.ts | 63 ++++++++++++++ packages/lib/tests/unit/regressions.test.ts | 41 +++++++++ 6 files changed, 241 insertions(+), 1 deletion(-) diff --git a/packages/lib/src/compiler.ts b/packages/lib/src/compiler.ts index e6b34d5..33c26d5 100644 --- a/packages/lib/src/compiler.ts +++ b/packages/lib/src/compiler.ts @@ -303,6 +303,7 @@ export class SqlCompilerImpl implements SqlCompiler { } const collection = this.extractTableName(ast.from[0]); + const selectedFields = this.extractSelectedFields(ast); // Check if we have nested field projections const hasNestedFieldProjections = @@ -338,6 +339,7 @@ export class SqlCompilerImpl implements SqlCompiler { collection, pipeline: [], }; + if (selectedFields) aggregateCommand.selectedFields = selectedFields; // Start with $match if we have a filter if (ast.where) { @@ -929,6 +931,7 @@ export class SqlCompilerImpl implements SqlCompiler { collection, filter: ast.where ? this.convertWhere(ast.where) : undefined, }; + if (selectedFields) findCommand.selectedFields = selectedFields; // Set up projection if (ast.columns) { @@ -958,6 +961,31 @@ export class SqlCompilerImpl implements SqlCompiler { } } + /** + * Collect the document fields read by the SELECT list, so the executor can reject columns + * that no document has. Table qualifiers are dropped and paths stop at the first array index. + * JOINs are skipped, since their columns come from more than one collection. + */ + private extractSelectedFields(ast: any): string[] | undefined { + if (!Array.isArray(ast.columns) || ast.from.length > 1) return undefined; + + const qualifiers = new Set([ast.from[0].table, ast.from[0].as].filter(Boolean)); + const fields = new Set(); + + for (const column of ast.columns) { + if (column?.expr?.type !== 'column_ref' || column.expr.column === '*') continue; + + let parts = this.processFieldName(this.extractFieldPath(column)).split('.'); + if (parts.length > 1 && qualifiers.has(parts[0])) parts = parts.slice(1); + const arrayIndex = parts.findIndex((part) => /^\d+$/.test(part)); + const field = (arrayIndex === -1 ? parts : parts.slice(0, arrayIndex)).join('.'); + + if (field && field !== '_id') fields.add(field); + } + + return fields.size > 0 ? Array.from(fields) : undefined; + } + /** * Compile an INSERT statement into a MongoDB INSERT command */ diff --git a/packages/lib/src/executor/dummy-client.ts b/packages/lib/src/executor/dummy-client.ts index 0f80367..5052846 100644 --- a/packages/lib/src/executor/dummy-client.ts +++ b/packages/lib/src/executor/dummy-client.ts @@ -49,6 +49,23 @@ class DummyCollection { return new DummyCursor(this.name, 'find', filter); } + /** + * Log a findOne operation + * @param filter Query filter + * @returns A dummy document + */ + async findOne(filter: any = {}) { + console.log( + `[DUMMY MongoDB] FIND ONE in ${this.dbName}.${this.name} with filter:`, + JSON.stringify(filter, null, 2) + ); + return { + _id: 'dummy-id', + operation: 'findOne', + message: 'This is a dummy result from the DummyClient', + }; + } + /** * Log an insertMany operation * @param documents Documents to insert diff --git a/packages/lib/src/executor/index.ts b/packages/lib/src/executor/index.ts index 2a76447..24158fb 100644 --- a/packages/lib/src/executor/index.ts +++ b/packages/lib/src/executor/index.ts @@ -4,8 +4,10 @@ import { ExecutionResult, CursorResult, CursorOptions, + FindCommand, + AggregateCommand, } from '../interfaces'; -import { Db, Document, MongoClient, ObjectId } from 'mongodb'; +import { Collection, Db, Document, MongoClient, ObjectId } from 'mongodb'; /** * MongoDB command executor implementation for Node.js @@ -84,6 +86,7 @@ export class MongoExecutor implements CommandExecutor { // Always return array for the regular execute result = await findCursor.toArray(); + await this.assertSelectedFieldsExist(database, command, result); break; case 'INSERT': @@ -128,6 +131,7 @@ export class MongoExecutor implements CommandExecutor { // Always return array for the regular execute result = await aggregateCursor.toArray(); + await this.assertSelectedFieldsExist(database, command, result); break; default: @@ -185,6 +189,8 @@ export class MongoExecutor implements CommandExecutor { findOptions.batchSize = options.batchSize; } + await this.assertSelectedFieldsExist(database, command); + // Create the cursor with all options at once const cursorFilter = await this.resolveFilterObjectIds( database, @@ -211,6 +217,8 @@ export class MongoExecutor implements CommandExecutor { aggregateOptions.batchSize = options.batchSize; } + await this.assertSelectedFieldsExist(database, command); + // Create the cursor with options result = database.collection(command.collection).aggregate(pipeline, aggregateOptions); break; @@ -228,6 +236,79 @@ export class MongoExecutor implements CommandExecutor { return result as CursorResult; } + /** + * MongoDB returns an empty row per document when a projected field doesn't exist, which hides + * misspelled columns. Throw instead when no document in the collection has a selected field. + * Empty collections are allowed, since they have no fields to check against. + * @param rows Rows the query returned. Fields with a value in one of them aren't looked up. + */ + private async assertSelectedFieldsExist( + db: Db, + command: FindCommand | AggregateCommand, + rows: Document[] = [] + ): Promise { + const unseen = (command.selectedFields || []).filter( + (field) => + !rows.some((row) => this.valuesAtPath(row, field.split('.')).some((v) => v != null)) + ); + if (unseen.length === 0) return; + + const collection = db.collection(command.collection); + const missing = await this.findMissingFields(collection, unseen); + if (missing.length === 0) return; + if (rows.length === 0 && !(await collection.findOne({}, { projection: { _id: 1 } }))) return; + + const names = missing.map((field) => `"${field}"`).join(', '); + throw new Error( + missing.length === 1 + ? `Column ${names} does not exist in collection "${command.collection}"` + : `Columns ${names} do not exist in collection "${command.collection}"` + ); + } + + /** + * Return the fields that no document in the collection has. Each lookup finds a document with + * at least one of the remaining fields, so this usually takes a single query. + */ + private async findMissingFields(collection: Collection, fields: string[]): Promise { + let missing = fields; + while (missing.length > 0) { + // MongoDB rejects overlapping projection paths, so only project the outermost ones + const projection = Object.fromEntries( + missing + .filter((field) => !missing.some((other) => field.startsWith(`${other}.`))) + .map((field) => [field, 1]) + ); + const doc = await collection.findOne( + { $or: missing.map((field) => ({ [field]: { $exists: true } })) }, + { projection } + ); + if (!doc) return missing; + + const remaining = missing.filter( + (field) => this.valuesAtPath(doc, field.split('.')).length === 0 + ); + // Can't tell which field the document matched on, so don't report any as missing + if (remaining.length === missing.length) return []; + missing = remaining; + } + return missing; + } + + /** + * Values at a field path, looking inside arrays the way MongoDB queries do. + */ + private valuesAtPath(value: any, path: string[]): unknown[] { + if (path.length === 0) return [value]; + if (Array.isArray(value)) return value.flatMap((item) => this.valuesAtPath(item, path)); + if (!value || typeof value !== 'object') return []; + // Don't look inside BSON values like ObjectId and Date + const proto = Object.getPrototypeOf(value); + if (proto !== Object.prototype && proto !== null) return []; + if (!Object.prototype.hasOwnProperty.call(value, path[0])) return []; + return this.valuesAtPath(value[path[0]], path.slice(1)); + } + /** * Sample one document from the collection to discover which fields contain ObjectIds, * then apply conversions to the filter accordingly. diff --git a/packages/lib/src/interfaces.ts b/packages/lib/src/interfaces.ts index f41d723..e2d0964 100644 --- a/packages/lib/src/interfaces.ts +++ b/packages/lib/src/interfaces.ts @@ -46,6 +46,11 @@ export interface FindCommand extends BaseCommand { foreignField: string; as: string; }[]; + /** + * Document fields read by the SELECT list. The executor rejects the query if no document in + * the collection has one of them, since MongoDB would return empty rows instead. + */ + selectedFields?: string[]; } /** @@ -80,6 +85,11 @@ export interface DeleteCommand extends BaseCommand { export interface AggregateCommand extends BaseCommand { type: 'AGGREGATE'; pipeline: Record[]; + /** + * Document fields read by the SELECT list. The executor rejects the query if no document in + * the collection has one of them, since MongoDB would return empty rows instead. + */ + selectedFields?: string[]; } /** diff --git a/packages/lib/tests/integration/regressions.integration.test.ts b/packages/lib/tests/integration/regressions.integration.test.ts index 96a57e5..0a1bb87 100644 --- a/packages/lib/tests/integration/regressions.integration.test.ts +++ b/packages/lib/tests/integration/regressions.integration.test.ts @@ -156,4 +156,67 @@ describe('Regression Integration Tests', () => { expect(totals).toEqual({ admin: 1, user: 1 }); }); }); + + // https://github.com/beekeeper-studio/queryleaf/issues/7 + describe('selecting a column that no document has', () => { + beforeEach(async () => { + await testSetup + .getDb() + .collection('regression_test') + .updateOne({ name: 'Bob' }, { $set: { nickname: 'Bobby', address: { city: 'Paris' } } }); + }); + + // Each of these used to return one empty row per document instead of an error + test.each([ + 'SELECT invalid_lmao FROM regression_test', + 'SELECT name, invalid_lmao FROM regression_test', + 'SELECT invalid_lmao AS x FROM regression_test', + 'SELECT r.invalid_lmao FROM regression_test r', + 'SELECT invalid_lmao[0] FROM regression_test', + "SELECT invalid_lmao FROM regression_test WHERE name = 'Nobody'", + 'SELECT invalid_lmao, COUNT(*) AS total FROM regression_test GROUP BY invalid_lmao', + ])('rejects %s', async (sql) => { + const queryLeaf = testSetup.getQueryLeaf(); + await expect(queryLeaf.execute(sql)).rejects.toThrow( + 'Column "invalid_lmao" does not exist in collection "regression_test"' + ); + }); + + test('rejects a nested field that no document has', async () => { + const queryLeaf = testSetup.getQueryLeaf(); + await expect(queryLeaf.execute('SELECT address.zip FROM regression_test')).rejects.toThrow( + 'Column "address.zip" does not exist in collection "regression_test"' + ); + }); + + test('rejects it when returning a cursor', async () => { + const queryLeaf = testSetup.getQueryLeaf(); + await expect( + queryLeaf.executeCursor('SELECT invalid_lmao FROM regression_test') + ).rejects.toThrow('Column "invalid_lmao" does not exist in collection "regression_test"'); + }); + + test('allows fields that only some documents have', async () => { + const queryLeaf = testSetup.getQueryLeaf(); + const results = ensureArray( + await queryLeaf.execute("SELECT name, nickname FROM regression_test WHERE name = 'Alice'") + ); + expect(results).toEqual([{ name: 'Alice' }]); + + const cities = ensureArray( + await queryLeaf.execute('SELECT address.city AS city FROM regression_test') + ); + expect(cities.map((r) => r.city).filter(Boolean)).toEqual(['Paris']); + + const cursor = await queryLeaf.executeCursor('SELECT nickname FROM regression_test'); + const nicknames = await cursor!.toArray(); + await cursor!.close(); + expect(nicknames.map((r) => r.nickname).filter(Boolean)).toEqual(['Bobby']); + }); + + test('allows any column on an empty collection', async () => { + const queryLeaf = testSetup.getQueryLeaf(); + expect(await queryLeaf.execute('SELECT invalid_lmao FROM regression_missing')).toEqual([]); + }); + }); }); diff --git a/packages/lib/tests/unit/regressions.test.ts b/packages/lib/tests/unit/regressions.test.ts index 23db49f..a9e2769 100644 --- a/packages/lib/tests/unit/regressions.test.ts +++ b/packages/lib/tests/unit/regressions.test.ts @@ -108,6 +108,7 @@ describe('Regressions', () => { pipeline = stages; return { toArray: async () => [] }; }, + findOne: async () => null, }), }), } as unknown as MongoClient; @@ -122,4 +123,44 @@ describe('Regressions', () => { expect(regex.source).toBe('^Al.*$'); }); }); + + // https://github.com/beekeeper-studio/queryleaf/issues/7 + describe('selecting a column that no document has', () => { + test.each<[string, string[] | undefined]>([ + ['SELECT invalid_lmao FROM t', ['invalid_lmao']], + ['SELECT name, invalid_lmao AS x FROM t', ['name', 'invalid_lmao']], + ['SELECT u.name FROM t u', ['name']], + ['SELECT address.city FROM t', ['address.city']], + ['SELECT tags[0], items[1].name FROM t', ['tags', 'items']], + ['SELECT category, COUNT(*) AS total FROM t GROUP BY category', ['category']], + ['SELECT _id FROM t', undefined], + ['SELECT * FROM t', undefined], + ['SELECT u.name, o.total FROM users u JOIN orders o ON u._id = o.userId', undefined], + ])('records the fields read by %s', (sql, expected) => { + expect((compile(sql) as any).selectedFields).toEqual(expected); + }); + + const fakeClient = (rows: any[]) => { + // No document in the collection matches a $exists check + const findOne = jest.fn(async (_filter: any) => null); + const collection = { find: () => ({ toArray: async () => rows }), findOne }; + const client = { db: () => ({ collection: () => collection }) } as unknown as MongoClient; + return { client, findOne }; + }; + + test('the executor rejects a field that no document has', async () => { + const { client } = fakeClient([{}, {}]); + await expect( + new MongoExecutor(client, 'db').execute([compile('SELECT invalid_lmao FROM t')]) + ).rejects.toThrow('Column "invalid_lmao" does not exist in collection "t"'); + }); + + test('the executor does not look up fields the returned rows already have', async () => { + const { client, findOne } = fakeClient([{ name: 'Alice' }]); + await expect( + new MongoExecutor(client, 'db').execute([compile('SELECT name FROM t')]) + ).resolves.toEqual([{ name: 'Alice' }]); + expect(findOne.mock.calls.filter(([filter]) => filter.$or)).toHaveLength(0); + }); + }); });