fix(sql): address PR review — path-literal scoping, gzip, dup columns, cache, engine routing

Resolves the 6 P2 review findings:

core (packages/core/src/ops/sql.ts):
- Only rewrite path literals in FROM/JOIN table position, never value-position
  strings — `WHERE x = '/data/sales.csv'` is no longer clobbered.
- Honor a real .gz suffix even when the format is explicitly overridden, so the
  temp file keeps its .gz extension and DuckDB decompresses it.
- Uniquify duplicate result column names (e.g. two projected `id`s) so building
  row objects never drops data by key collision.

live (sql-engine):
- formatForPath strips an optional .gz suffix (and deriveTableName too); the
  document picker adds .gz globs — gzipped text docs are pickable/seedable.
- Browser-engine file registration keys on the current file revision (stat),
  so an edited doc re-registers instead of serving a stale buffer.
- canRunInBrowser is query-aware: a FROM/JOIN path literal that isn't a bound
  doc routes to the server (only it auto-binds bare path literals).

+ tests for path-literal scoping, gzip override, and duplicate columns.

Author: tarasyarema

live/src/components/sql/DocumentPicker.tsx

@@ -12,8 +12,13 @@ import { QUERYABLE_EXTENSIONS, sanitizeTableName, type BoundDoc } from "@/lib/sq
 import type { GlobMatch, GlobResult } from "@/api/types"
 
 /** The glob op treats `{}` as literal characters (no brace expansion), so run
- *  one glob per queryable extension in parallel and merge the matches. */
-const QUERYABLE_GLOBS = Object.keys(QUERYABLE_EXTENSIONS).map((ext) => `**/*.${ext}`)
+ *  one glob per queryable extension in parallel and merge the matches. Gzipped
+ *  text formats (`.csv.gz`, …) get their own globs since the op handles them. */
+const GZIP_EXTENSIONS = ["csv", "tsv", "json", "jsonl", "ndjson"]
+const QUERYABLE_GLOBS = [
+  ...Object.keys(QUERYABLE_EXTENSIONS).map((ext) => `**/*.${ext}`),
+  ...GZIP_EXTENSIONS.map((ext) => `**/*.${ext}.gz`),
+]
 
 function useQueryableDocs() {
   const { client, orgId, driveId } = useAuth()

live/src/lib/sql-engine/duckdb.ts

@@ -74,15 +74,31 @@ function readerFor(doc: BoundDoc, file: string): string {
 }
 
 /** Fetch + register each doc's bytes, skipping files already registered from
- *  the same org/drive/path. */
+ *  the same org/drive/path *and revision*. Keying on the current version means
+ *  an edited/re-uploaded file at the same path re-registers instead of serving
+ *  a stale buffer. */
 async function ensureRegistered(
   db: duckdb.AsyncDuckDB,
   docs: BoundDoc[],
   ctx: SqlEngineContext,
 ): Promise<void> {
   for (const doc of docs) {
     const name = virtualName(doc.path)
-    const key = `${ctx.orgId}/${ctx.driveId}:${name}`
+    // Resolve a revision token. If stat fails, fall back to a unique value so we
+    // never reuse a possibly-stale buffer.
+    let revision: string
+    try {
+      const stat = await ctx.client.callOp<{ currentVersion?: number; modifiedAt?: string }>(
+        ctx.orgId,
+        "stat",
+        { path: doc.path },
+        ctx.driveId,
+      )
+      revision = String(stat.currentVersion ?? stat.modifiedAt ?? performance.now())
+    } catch {
+      revision = String(performance.now())
+    }
+    const key = `${ctx.orgId}/${ctx.driveId}:${name}@${revision}`
     if (registeredFiles.get(name) === key) continue
 
     const blob = await ctx.client.fetchRaw(ctx.orgId, ctx.driveId, doc.path)

live/src/lib/sql-engine/index.ts

@@ -5,13 +5,24 @@ import type { BoundDoc, SqlEngineContext, SqlRunInput, SqlRunResult } from "./ty
  *  and sqlite/duckdb (flaky sqlite_scanner) must run on the server. */
 const WASM_FORMATS = new Set<string>(["csv", "tsv", "parquet", "json", "ndjson"])
 
+/** Drive-path literal in table position, e.g. `FROM '/data/sales.csv'`. */
+const FROM_JOIN_PATH_RE = /\b(?:from|join)\s+(['"])([^'"]+\.[A-Za-z0-9]+(?:\.gz)?)\1/gi
+
 /**
- * True when every bound doc can be read by DuckDB-WASM in the browser. With no
- * bound docs the query can only reference drive paths directly, which only the
- * server resolves — so an empty doc set also routes to the server.
+ * True when the query can run entirely in the browser (DuckDB-WASM). Requires:
+ * at least one bound doc, every bound doc readable by WASM, and no FROM/JOIN
+ * drive-path literal that isn't a bound doc — only the server auto-binds bare
+ * path literals, so a query referencing an unbound one must run there.
  */
-export function canRunInBrowser(docs: BoundDoc[]): boolean {
-  return docs.length > 0 && docs.every((d) => WASM_FORMATS.has(d.format))
+export function canRunInBrowser(docs: BoundDoc[], query?: string): boolean {
+  if (docs.length === 0 || !docs.every((d) => WASM_FORMATS.has(d.format))) return false
+  if (query) {
+    const bound = new Set(docs.map((d) => d.path.replace(/^\/+/, "")))
+    for (const m of query.matchAll(FROM_JOIN_PATH_RE)) {
+      if (!bound.has(m[2].replace(/^\/+/, ""))) return false
+    }
+  }
+  return true
 }
 
 /**
@@ -25,7 +36,7 @@ export async function runSql(
   ctx: SqlEngineContext,
   opts?: { forceServer?: boolean },
 ): Promise<SqlRunResult> {
-  if (!opts?.forceServer && canRunInBrowser(input.docs)) {
+  if (!opts?.forceServer && canRunInBrowser(input.docs, input.query)) {
     const { runInBrowser } = await import("./duckdb")
     return runInBrowser(input, ctx)
   }

live/src/lib/sql-engine/types.ts

@@ -16,9 +16,22 @@ export const QUERYABLE_EXTENSIONS: Record<string, SqlFormat> = {
   duckdb: "duckdb",
 }
 
+/** Formats that can be read from a gzipped file (`.csv.gz`, `.json.gz`, …). */
+const GZIP_FORMATS = new Set<SqlFormat>(["csv", "tsv", "json", "ndjson"])
+
 export function formatForPath(path: string): SqlFormat | null {
-  const ext = path.split(".").pop()?.toLowerCase() ?? ""
-  return QUERYABLE_EXTENSIONS[ext] ?? null
+  let name = path.toLowerCase()
+  let gzipped = false
+  if (name.endsWith(".gz")) {
+    gzipped = true
+    name = name.slice(0, -3)
+  }
+  const ext = name.split(".").pop() ?? ""
+  const format = QUERYABLE_EXTENSIONS[ext] ?? null
+  if (!format) return null
+  // A .gz suffix only makes sense for the text formats DuckDB decompresses.
+  if (gzipped && !GZIP_FORMATS.has(format)) return null
+  return format
 }
 
 export function isQueryablePath(path: string): boolean {
@@ -34,7 +47,9 @@ export function sanitizeTableName(name: string): string {
 
 /** Derive a SQL-safe table name from a file path stem, unique against `taken`. */
 export function deriveTableName(path: string, taken: Iterable<string>): string {
-  const stem = (path.split("/").pop() ?? path).replace(/\.[^.]+$/, "")
+  const stem = (path.split("/").pop() ?? path)
+    .replace(/\.gz$/i, "")
+    .replace(/\.[^.]+$/, "")
   const base = sanitizeTableName(stem) || "doc"
   const set = new Set(taken)
   if (!set.has(base)) return base

live/src/pages/SqlPage.tsx

@@ -185,7 +185,7 @@ export function SqlPage() {
     }
   }, [client, orgId, driveId, query, docs, maxRows, forceServer])
 
-  const wasmEligible = canRunInBrowser(docs)
+  const wasmEligible = canRunInBrowser(docs, query)
   const runsInBrowser = !forceServer && wasmEligible
   const effectiveEngine: "browser" | "server" = runsInBrowser ? "browser" : "server"
   const engineHint = forceServer

packages/core/src/ops/__tests__/sql.test.ts

@@ -324,4 +324,34 @@ describe("sql op", () => {
     });
     expect(result.rows).toEqual([{ n: 3 }]);
   });
+
+  test("a bound path used as a value (not a table) is not rewritten", async () => {
+    const { ctx } = createTestContext();
+    await seedCsv(ctx);
+    // '/data/sales.csv' appears in both FROM (table) and WHERE (value) position.
+    // Only the FROM occurrence must become the table; the predicate stays a string.
+    const result = await sql(ctx, {
+      query:
+        "SELECT count(*) AS n FROM '/data/sales.csv' WHERE name <> '/data/sales.csv'",
+    });
+    expect(result.rows).toEqual([{ n: 3 }]);
+  });
+
+  test("gzip suffix is honored even when the format is overridden", async () => {
+    const { ctx } = createTestContext();
+    const gz = Bun.gzipSync(new TextEncoder().encode("a,b\n1,2\n3,4\n"));
+    await writeRaw(ctx, { path: "/d/log.txt.gz", bytes: new Uint8Array(gz) });
+    const result = await sql(ctx, {
+      query: "SELECT sum(a) AS s FROM t",
+      tables: { t: { path: "/d/log.txt.gz", format: "csv" } },
+    });
+    expect(result.rows).toEqual([{ s: 4 }]);
+  });
+
+  test("duplicate result columns are preserved (uniquified)", async () => {
+    const { ctx } = createTestContext();
+    const result = await sql(ctx, { query: "SELECT 1 AS id, 2 AS id" });
+    expect(result.columns.map((c) => c.name)).toEqual(["id", "id_2"]);
+    expect(result.rows).toEqual([{ id: 1, id_2: 2 }]);
+  });
 });

packages/core/src/ops/sql.ts

@@ -114,15 +114,23 @@ interface Binding {
   path: string;
   format: SqlFormat;
   gzip: boolean;
-  /** Exact quoted literal in the query to substitute with the table identifier. */
-  literal?: string;
+  /** True when bound from a FROM/JOIN path literal (rewritten to the table id). */
+  autoBound?: boolean;
   localFile?: string;
   size: number;
 }
 
 const TABLE_NAME_RE = /^[A-Za-z_][A-Za-z0-9_]*$/;
-// Quoted literal whose content has a queryable file extension, e.g. '/data/sales.csv'
-const PATH_LITERAL_RE = /'([^']+\.[A-Za-z0-9]+(?:\.gz)?)'/g;
+// A quoted path literal in table position (after FROM or JOIN), e.g.
+// `FROM '/data/sales.csv'`. Restricting to table position means an ordinary
+// string value like `WHERE source = '/data/sales.csv'` is never rewritten.
+const FROM_JOIN_PATH_RE =
+  /\b(from|join)\s+(['"])([^'"]+\.[A-Za-z0-9]+(?:\.gz)?)\2/gi;
+
+/** True when `path` is a gzipped file and `format` can read gzip. */
+function isGzip(path: string, format: SqlFormat): boolean {
+  return /\.gz$/i.test(path) && GZIPPABLE.has(format);
+}
 
 function findFile(
   ctx: OpContext,
@@ -181,25 +189,28 @@ function collectBindings(ctx: OpContext, params: SqlParams): Binding[] {
       table: name,
       path,
       format,
-      gzip: explicitFormat ? false : detected?.gzip ?? false,
+      // Honor a real .gz suffix even when the format is overridden, so the temp
+      // file keeps its .gz extension and DuckDB decompresses it.
+      gzip: isGzip(path, format),
       size: file.size,
     });
   }
 
-  // Auto-bind quoted drive-path literals like SELECT * FROM '/data/sales.csv'.
-  // Only literals that resolve to an existing document are bound; anything else
-  // is left untouched so plain string values are never misinterpreted.
-  const seenLiterals = new Set<string>();
+  // Auto-bind drive-path literals that sit in table position, like
+  // `SELECT * FROM '/data/sales.csv'`. Only literals that resolve to an existing
+  // document are bound; a path that merely appears as a string value elsewhere
+  // (e.g. `WHERE source = '/data/sales.csv'`) is never matched here.
+  const seenPaths = new Set<string>();
   let docIdx = 1;
-  for (const match of params.query.matchAll(PATH_LITERAL_RE)) {
-    const raw = match[1];
-    if (seenLiterals.has(raw)) continue;
-    seenLiterals.add(raw);
-
+  for (const match of params.query.matchAll(FROM_JOIN_PATH_RE)) {
+    const raw = match[3];
     const detected = detectSqlFormat(raw);
     if (!detected || !FILE_FORMATS.has(detected.format)) continue;
 
     const path = normalizePath(raw);
+    if (seenPaths.has(path)) continue;
+    seenPaths.add(path);
+
     const file = findFile(ctx, path);
     if (!file) continue;
 
@@ -212,7 +223,7 @@ function collectBindings(ctx: OpContext, params: SqlParams): Binding[] {
       path,
       format: detected.format,
       gzip: detected.gzip,
-      literal: match[0],
+      autoBound: true,
       size: file.size,
     });
   }
@@ -404,12 +415,18 @@ export async function sql(ctx: OpContext, params: SqlParams): Promise<SqlResult>
     }
 
     // --- Query phase ---
-    let query = params.query;
-    for (const b of bindings) {
-      if (b.literal) {
-        query = query.split(b.literal).join(qid(b.table));
+    // Rewrite only FROM/JOIN path literals (never value-position strings) to the
+    // in-memory table they were bound to.
+    const autoBound = new Map(
+      bindings.filter((b) => b.autoBound).map((b) => [b.path, b.table])
+    );
+    const query = params.query.replace(
+      FROM_JOIN_PATH_RE,
+      (full, kw: string, _q: string, raw: string) => {
+        const table = autoBound.get(normalizePath(raw));
+        return table ? `${kw} ${qid(table)}` : full;
       }
-    }
+    );
 
     let timedOut = false;
     timer = setTimeout(() => {
@@ -433,7 +450,14 @@ export async function sql(ctx: OpContext, params: SqlParams): Promise<SqlResult>
       const prepared = await extracted.prepare(extracted.count - 1);
       const reader = await prepared.runAndReadUntil(maxRows + 1);
 
-      const columnNames = reader.columnNames();
+      // Uniquify duplicate column names (e.g. a join projecting two `id`s) so
+      // building row objects never drops data by key collision.
+      const seenCols = new Map<string, number>();
+      const columnNames = reader.columnNames().map((name) => {
+        const n = seenCols.get(name) ?? 0;
+        seenCols.set(name, n + 1);
+        return n === 0 ? name : `${name}_${n + 1}`;
+      });
       const columnTypes = reader.columnTypes();
       const columns: SqlColumn[] = columnNames.map((name, i) => ({
         name,