Skip to content

Commit 5a3f97b

Browse files
paddymulclaude
andcommitted
fix(duckdb-node): derive active search SQL/length on demand
The row window read `activeSql` and `filteredRows`, both refreshed only inside `initialState()`, so a `setSearch` not followed by `initialState` served stale rows and a stale `length`. - Drop the `activeSql` field; derive the active (search-filtered) SQL on demand via `searchEffectiveSql(plan)` in both `initialState` and `handleInfiniteRequest`. It's pure SQL building, so it can't drift. - Replace the `filteredRows` field with a `cachedFiltered` cache that `setSearch` invalidates. `handleInfiniteRequest` recomputes via `ensureFiltered` when stale; `initialState` seeds the cache from its own SUMMARIZE, so the common state_change -> initial_state -> infinite_request flow still runs exactly one SUMMARIZE. Also corrects the `isActiveSearch` comment: the term is not trimmed, matching pandas `Search` (no-ops only on ""). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a0825aa commit 5a3f97b

2 files changed

Lines changed: 54 additions & 23 deletions

File tree

packages/buckaroo-duckdb-node/src/backend.ts

Lines changed: 50 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -77,20 +77,22 @@ export class DuckBackend {
7777
private plan?: RenamePlan;
7878
/** Unfiltered row count — `df_meta.total_rows`. Stable across searches. */
7979
private totalRows = 0;
80-
/** Rows after search — `df_meta.filtered_rows` and the infinite_resp length. */
81-
private filteredRows = 0;
80+
/** Unfiltered count, cached so repeated searches don't re-summarize the base. */
8281
private cachedTotal?: number;
82+
/**
83+
* Rows after search — `df_meta.filtered_rows` and the infinite_resp `length`.
84+
* Cached against the current `searchTerm`; `setSearch` invalidates it so a row
85+
* window can never report a stale count. `undefined` means "not yet computed".
86+
*/
87+
private cachedFiltered?: number;
8388
private searchTerm: string;
84-
/** The effective SQL the row/stats queries run against (base + search). */
85-
private activeSql: string;
8689

8790
constructor(source: DuckSource, baseStmt: string, opts: DuckBackendOptions = {}) {
8891
this.source = source;
8992
this.baseStmt = baseStmt;
9093
this.summaryStatsKey = opts.summaryStatsKey ?? 'all_stats';
9194
this.transforms = opts.transforms ?? [];
9295
this.searchTerm = opts.search ?? '';
93-
this.activeSql = effectiveQuery(this.baseStmt, this.transforms);
9496
}
9597

9698
/** The base effective SQL, search excluded. */
@@ -99,11 +101,17 @@ export class DuckBackend {
99101
}
100102

101103
/**
102-
* Set (or clear) the search term. The next `initialState` re-runs stats over
103-
* the filtered set and re-points the row window; an empty term clears it.
104+
* Set (or clear) the search term. Invalidates the cached filtered count so the
105+
* next `initialState`/`infinite_request` recomputes against the new term; an
106+
* empty term clears the filter. The active SQL is derived on demand (no cached
107+
* copy to go stale), so a row window is consistent regardless of call order.
104108
*/
105109
setSearch(term: string): void {
106-
this.searchTerm = term ?? '';
110+
const next = term ?? '';
111+
if (next !== this.searchTerm) {
112+
this.searchTerm = next;
113+
this.cachedFiltered = undefined;
114+
}
107115
}
108116

109117
/** The original (pre-rename) text columns search targets. */
@@ -130,28 +138,29 @@ export class DuckBackend {
130138
async initialState(): Promise<InitialStateMessage> {
131139
const plan = await this.ensurePlan();
132140
const active = isActiveSearch(this.searchTerm) && this.searchColumns(plan).length > 0;
133-
this.activeSql = this.searchEffectiveSql(plan);
134141

135142
// Stats run over the (possibly search-filtered) renamed relation — pandas
136143
// re-runs summary stats on the filtered df, so we do too. The relation
137144
// includes the synthesized, non-null `index` column, so its SUMMARIZE count
138145
// is the row count of that set (filtered under search), no extra count
139-
// query needed.
140-
const summarizeRows = await this.source.summarize(plan.renamedRelation(this.activeSql));
146+
// query needed. Seed the filtered-count cache from this same SUMMARIZE so a
147+
// following `infinite_request` doesn't re-run it.
148+
const summarizeRows = await this.source.summarize(plan.renamedRelation(this.searchEffectiveSql(plan)));
141149
const indexRow = summarizeRows.find((r) => r.column_name === INDEX_COL);
142-
this.filteredRows = indexRow ? Number(indexRow.count) : 0;
150+
const filteredRows = indexRow ? Number(indexRow.count) : 0;
151+
this.cachedFiltered = filteredRows;
143152

144153
// total_rows is the unfiltered count and never changes with search. Cache it
145154
// so repeated searches don't re-summarize the base relation; when search is
146155
// inactive the filtered count IS the total.
147156
if (!active) {
148-
this.cachedTotal = this.filteredRows;
157+
this.cachedTotal = filteredRows;
149158
} else if (this.cachedTotal === undefined) {
150159
const baseRows = await this.source.summarize(plan.renamedRelation(this.effectiveSql));
151160
const baseIndex = baseRows.find((r) => r.column_name === INDEX_COL);
152-
this.cachedTotal = baseIndex ? Number(baseIndex.count) : this.filteredRows;
161+
this.cachedTotal = baseIndex ? Number(baseIndex.count) : filteredRows;
153162
}
154-
this.totalRows = this.cachedTotal ?? this.filteredRows;
163+
this.totalRows = this.cachedTotal ?? filteredRows;
155164

156165
const sd = summarizeToSDType(summarizeRows);
157166
const statRows = sdTypeToStatRows(sd);
@@ -164,8 +173,8 @@ export class DuckBackend {
164173
df_meta: {
165174
total_rows: this.totalRows,
166175
columns: plan.columns.length,
167-
filtered_rows: this.filteredRows,
168-
rows_shown: this.filteredRows,
176+
filtered_rows: filteredRows,
177+
rows_shown: filteredRows,
169178
},
170179
df_data_dict: {
171180
// main rows arrive on demand via infinite_request
@@ -198,15 +207,34 @@ export class DuckBackend {
198207
}
199208
}
200209

210+
/**
211+
* The filtered row count for the current search. Cached and invalidated by
212+
* `setSearch`, recomputed with one SUMMARIZE-count only when stale.
213+
* `initialState` seeds the cache from its own SUMMARIZE, so the common
214+
* state_change → initial_state → infinite_request flow never double-counts;
215+
* a standalone `setSearch` followed directly by `infinite_request` recomputes
216+
* here rather than reporting a stale length.
217+
*/
218+
private async ensureFiltered(plan: RenamePlan): Promise<number> {
219+
if (this.cachedFiltered === undefined) {
220+
const rows = await this.source.summarize(plan.renamedRelation(this.searchEffectiveSql(plan)));
221+
const indexRow = rows.find((r) => r.column_name === INDEX_COL);
222+
this.cachedFiltered = indexRow ? Number(indexRow.count) : 0;
223+
}
224+
return this.cachedFiltered;
225+
}
226+
201227
/**
202228
* Answer one `infinite_request`. The window runs against the active
203-
* (search-filtered) SQL, is serialized through the COPY→parquet no-coercion
204-
* path and returned inline as a `parquet_b64` envelope. `length` is the
205-
* filtered row count so the grid scrolls only the matching rows.
229+
* (search-filtered) SQL — derived on demand from the current term, never a
230+
* cached copy that could go stale — serialized through the COPY→parquet
231+
* no-coercion path and returned inline as a `parquet_b64` envelope. `length`
232+
* is the filtered row count so the grid scrolls only the matching rows.
206233
*/
207234
async handleInfiniteRequest(args: PayloadArgs): Promise<PayloadResponse> {
208235
const plan = await this.ensurePlan();
209-
const sql = windowedQuery(this.activeSql, plan, {
236+
const length = await this.ensureFiltered(plan);
237+
const sql = windowedQuery(this.searchEffectiveSql(plan), plan, {
210238
start: args.start,
211239
end: args.end,
212240
sort: args.sort,
@@ -217,7 +245,7 @@ export class DuckBackend {
217245
return {
218246
type: 'infinite_resp',
219247
key: args,
220-
length: this.filteredRows,
248+
length,
221249
payload,
222250
};
223251
}

packages/buckaroo-duckdb-node/src/search.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@ export function isSearchableType(duckType: string): boolean {
2323
return base !== 'BOOLEAN' && base !== 'BOOL';
2424
}
2525

26-
/** A no-op term — empty or whitespace — means "no filter". */
26+
/**
27+
* An empty/absent term means "no filter". Whitespace is a real term — pandas
28+
* `Search` no-ops only on `""` (`val == ""`), so we must not trim here.
29+
*/
2730
export function isActiveSearch(term: string | null | undefined): boolean {
2831
return typeof term === 'string' && term.length > 0;
2932
}

0 commit comments

Comments
 (0)