From 67c2ce39369c6776b4a7fe6fbbba9541c3c1bd1b Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 5 May 2026 07:27:48 +0000 Subject: [PATCH] all --- .../AssetGraph/AssetGraphDetailsPane.svelte | 5 +- .../assets/AssetGraph/DataTablePreview.svelte | 211 ++++++++++++++---- .../assets/AssetGraph/pipelineTemplates.ts | 51 ++++- .../(logged)/pipeline/[folder]/+page.svelte | 15 +- 4 files changed, 228 insertions(+), 54 deletions(-) diff --git a/frontend/src/lib/components/assets/AssetGraph/AssetGraphDetailsPane.svelte b/frontend/src/lib/components/assets/AssetGraph/AssetGraphDetailsPane.svelte index aa9a8dde89..119e3051ec 100644 --- a/frontend/src/lib/components/assets/AssetGraph/AssetGraphDetailsPane.svelte +++ b/frontend/src/lib/components/assets/AssetGraph/AssetGraphDetailsPane.svelte @@ -292,6 +292,7 @@ >
{#if isDraft && script} + {@const draftScriptPath = script.path} {#if onDraftPathChange} + {:else if datatables.loading && !datatables.current} +
+ +
+ {:else if !datatableExists} + +
+ +
+ Datatable not configured + + The datatable + {parsed.datatable} + isn't set up in this workspace yet. + +
+ +
+ {:else if colDefs.loading && !colDefs.current} +
+ +
+ {:else if !tableColDefs} + +
+ + Table doesn't exist yet + + Nothing has been created at + {parsed.datatable}/{parsed.schema ? `${parsed.schema}.` : ''}{parsed.table} + yet. It'll appear here once an upstream pipeline script writes to it. + +
+ {:else if dbTableOps} + {#key refreshKey} - + {/key} {/if}
diff --git a/frontend/src/lib/components/assets/AssetGraph/pipelineTemplates.ts b/frontend/src/lib/components/assets/AssetGraph/pipelineTemplates.ts index 9fcdef094b..ae2d1634f0 100644 --- a/frontend/src/lib/components/assets/AssetGraph/pipelineTemplates.ts +++ b/frontend/src/lib/components/assets/AssetGraph/pipelineTemplates.ts @@ -139,6 +139,34 @@ export function assetUri(asset: { kind: AssetKind; path: string }): string { return `${ASSET_URI_PREFIX[asset.kind]}${asset.path}` } +// Splits a datatable asset path (`/` or `/.
`) +// into its constituent parts. Mirrors `parseDbInputFromAssetSyntax` in +// $lib/utils.ts: schema is omitted when the table lives in the default +// (`public`) schema, prefixed with `.` only when explicit. +function parseDatatablePath(p: string): { + db: string + schema: string | undefined + table: string +} { + const slash = p.indexOf('/') + if (slash < 0) return { db: p, schema: undefined, table: '' } + const db = p.slice(0, slash) + const tail = p.slice(slash + 1) + const dot = tail.indexOf('.') + if (dot < 0) return { db, schema: undefined, table: tail } + return { db, schema: tail.slice(0, dot), table: tail.slice(dot + 1) } +} + +// Schema-qualified table reference for SQL emission. We always emit the +// schema (defaulting to `public`) so the generated statement doesn't rely +// on whatever search-path / default-schema the engine picks at attach +// time — important for DuckDB attaching to Postgres as `pg`, where bare +// `pg.
` is fragile. +function qualifiedDatatableRef(p: string, defaultSchema = 'public'): string { + const parsed = parseDatatablePath(p) + return `${parsed.schema ?? defaultSchema}.${parsed.table || 'table_name'}` +} + // Comment prefix per language; mirrors what the parser accepts. function commentPrefix(lang: ScriptLang): string { switch (lang) { @@ -378,6 +406,11 @@ function bodyDuckdb(ctx: TemplateContext): string { switch (input.kind) { case 's3object': return `read_parquet('s3://${input.path}')` + case 'datatable': + // `pg` is the attached Postgres catalog; we schema-qualify + // the table reference so the SELECT doesn't rely on the + // catalog's default schema setting. + return `pg.${qualifiedDatatableRef(input.path)}` case 'ducklake': { return null // attach + read shown below } @@ -428,10 +461,14 @@ function bodyDuckdb(ctx: TemplateContext): string { break case 'datatable': if (output) { - const tableName = output.path.split('/').slice(1).join('_') || 'table_name' + // Schema-qualified `pg..
` (rather than + // `pg.
`) so the CREATE TABLE doesn't silently + // land in whatever default schema the attached catalog + // resolves to. + const ref = `pg.${qualifiedDatatableRef(output.path)}` lines.push( `-- DataTable is exposed as the 'pg' attached database in duckdb scripts.`, - `CREATE TABLE IF NOT EXISTS pg.${tableName} AS`, + `CREATE TABLE IF NOT EXISTS ${ref} AS`, `SELECT * FROM ${inSql ?? '(SELECT 1 AS placeholder)'};` ) } @@ -452,18 +489,22 @@ function bodyPostgres(ctx: TemplateContext): string { lines.push('') if (outputKind === 'datatable' && output) { - const tableName = output.path.split('/').slice(1).join('_') || 'table_name' + // Schema-qualified — Postgres respects search_path so a bare + // `
` would land in `public` by default, but emitting + // `.
` makes the target explicit and survives any + // non-default search_path the connection might carry. + const ref = qualifiedDatatableRef(output.path) lines.push( `-- The pipeline-script harness runs this query against the`, `-- datatable backing the configured workspace storage.`, - `CREATE TABLE IF NOT EXISTS ${tableName} (`, + `CREATE TABLE IF NOT EXISTS ${ref} (`, ` id serial primary key,`, ` payload jsonb,`, ` computed_at timestamptz default now()`, `);`, ``, `-- Replace with your real INSERT.`, - `INSERT INTO ${tableName} (payload) VALUES ('{}'::jsonb);` + `INSERT INTO ${ref} (payload) VALUES ('{}'::jsonb);` ) } else { lines.push(`SELECT 1;`) diff --git a/frontend/src/routes/(root)/(logged)/pipeline/[folder]/+page.svelte b/frontend/src/routes/(root)/(logged)/pipeline/[folder]/+page.svelte index 79d3c985a1..ce108d2e46 100644 --- a/frontend/src/routes/(root)/(logged)/pipeline/[folder]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/pipeline/[folder]/+page.svelte @@ -260,15 +260,20 @@ }) // Native browser tab close / reload — we can't show a custom modal - // here, only the browser's generic confirm. Setting returnValue to a - // non-empty string is the cross-browser idiom that triggers the - // "Leave site?" prompt. The text shown is browser-controlled. + // here, only the browser's generic confirm. Modern browsers require + // BOTH preventDefault() *and* a non-empty `returnValue` to actually + // pop the prompt; the assigned text is then ignored in favour of a + // browser-controlled string ("Leave site? Changes you made may not be + // saved."). Empty string used to work but Chromium 119+ silently + // suppresses the dialog in that case. The handler also requires that + // the user has interacted with the page (Chrome's user-activation + // requirement) — a fresh tab with no clicks/keystrokes won't get the + // prompt. This is enforced by the browser, not us. $effect(() => { - if (typeof window === 'undefined') return function onBeforeUnload(e: BeforeUnloadEvent) { if (drafts.size === 0) return e.preventDefault() - e.returnValue = '' + e.returnValue = 'You have unsaved drafts.' } window.addEventListener('beforeunload', onBeforeUnload) return () => window.removeEventListener('beforeunload', onBeforeUnload)