Skip to content

Commit e799bda

Browse files
committed
fix: local-executor explain() used stale projections-only column set + missing filterGroups
explain() computed projectedColumns using only query.projections, not including filter/sort/groupBy/aggregate/window/distinct/join columns. This produced inaccurate estimatedBytes and estimatedR2Reads. Now uses the same neededNames logic as the execute path. Also adds filterGroups to the explain filters output (query-do already had this) and fixes format.test.ts fixture that incorrectly showed IN filter as non-pushable.
1 parent 43bc6df commit e799bda

File tree

2 files changed

+22
-10
lines changed

2 files changed

+22
-10
lines changed

src/format.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ describe("formatExplain", () => {
126126
fragments: 1,
127127
filters: [
128128
{ column: "amount", op: "gt", pushable: true },
129-
{ column: "region", op: "in", pushable: false },
129+
{ column: "region", op: "in", pushable: true },
130130
],
131131
metaCached: true,
132132
};
@@ -139,7 +139,7 @@ describe("formatExplain", () => {
139139
expect(output).toContain("name");
140140
expect(output).toContain("utf8");
141141
expect(output).toContain("amount gt [pushable]");
142-
expect(output).toContain("region in [not pushable]");
142+
expect(output).toContain("region in [pushable]");
143143
expect(output).toContain("20 skipped (62.5%)");
144144
expect(output).toContain("128KB across 4 reads");
145145
expect(output).toContain("Meta: cached");

src/local-executor.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -209,9 +209,22 @@ export class LocalExecutor implements QueryExecutor {
209209
async explain(query: QueryDescriptor): Promise<ExplainResult> {
210210
const meta = await this.getOrLoadMeta(query.table);
211211
const { columns } = meta;
212-
const projectedColumns = query.projections.length > 0
213-
? columns.filter(c => query.projections.includes(c.name))
214-
: columns;
212+
const neededCols = new Set(query.projections.length > 0 ? query.projections : columns.map(c => c.name));
213+
for (const f of query.filters) neededCols.add(f.column);
214+
if (query.filterGroups) for (const g of query.filterGroups) for (const f of g) neededCols.add(f.column);
215+
if (query.sortColumn) neededCols.add(query.sortColumn);
216+
if (query.groupBy) for (const g of query.groupBy) neededCols.add(g);
217+
if (query.aggregates) for (const a of query.aggregates) if (a.column !== "*") neededCols.add(a.column);
218+
if (query.distinct) for (const d of query.distinct) neededCols.add(d);
219+
if (query.windows) for (const w of query.windows) {
220+
if (w.column) neededCols.add(w.column);
221+
for (const p of w.partitionBy) neededCols.add(p);
222+
for (const o of w.orderBy) neededCols.add(o.column);
223+
}
224+
if (query.join) neededCols.add(query.join.leftKey);
225+
if (query.subqueryIn) for (const sq of query.subqueryIn) neededCols.add(sq.column);
226+
if (query.vectorSearch) neededCols.add(query.vectorSearch.column);
227+
const projectedColumns = columns.filter(c => neededCols.has(c.name));
215228

216229
let pagesTotal = 0;
217230
let pagesSkipped = 0;
@@ -267,11 +280,10 @@ export class LocalExecutor implements QueryExecutor {
267280
fragments: this.datasetCache.get(query.table)?.fragmentMetas.size ?? 1,
268281
fragmentsScanned: this.datasetCache.get(query.table)?.fragmentMetas.size ?? 1,
269282
fanOut: false,
270-
filters: query.filters.map(f => ({
271-
column: f.column,
272-
op: f.op,
273-
pushable: true,
274-
})),
283+
filters: [
284+
...query.filters.map(f => ({ column: f.column, op: f.op, pushable: true })),
285+
...(query.filterGroups ?? []).flatMap(g => g.map(f => ({ column: f.column, op: f.op, pushable: true }))),
286+
],
275287
metaCached: this.metaCache.has(query.table),
276288
};
277289
}

0 commit comments

Comments
 (0)