diff --git a/backend/parsers/windmill-parser/src/sql_materialize.rs b/backend/parsers/windmill-parser/src/sql_materialize.rs index dccf8ec313..1b24a5bccd 100644 --- a/backend/parsers/windmill-parser/src/sql_materialize.rs +++ b/backend/parsers/windmill-parser/src/sql_materialize.rs @@ -1248,6 +1248,28 @@ fn sample_star(ctx: &DataTestCtx, qualifier: Option<&str>) -> String { } } +// Self-teaching tail appended to every malformed-custom-test error. It states +// the two rules that aren't documented or scaffolded anywhere else — the body +// is a single SELECT, and it reads the freshly-materialized target through the +// internal `_wm_target.` alias — and doubles that alias into a copyable +// one-line example. `target_qualified` is already `_wm_target.
`. +fn custom_test_hint(target_qualified: &str) -> String { + format!( + "Write a single SELECT against `{target_qualified}` returning the offending rows, e.g. \ + `SELECT * FROM {target_qualified} WHERE ` — an empty result means the test \ + passes." + ) +} + +// Whether a custom-test statement reads the materialized target through the +// reserved `_wm_target` alias (the only handle the runtime attaches it under). +// SQL identifiers are case-insensitive, so match case-insensitively; +// `split_statements` has already stripped comments, so a match here is a real +// reference, not one buried in a comment. `TARGET_ALIAS` is lowercase. +fn references_target(stmt: &str) -> bool { + stmt.to_lowercase().contains(TARGET_ALIAS) +} + /// Compile resolved data tests into ATTACH statements + per-test checks for /// `ctx`'s target. Pure: returns SQL text, executes nothing. Errors carry an /// actionable message (e.g. a relationships target that isn't an attachable @@ -1379,24 +1401,44 @@ pub fn build_data_test_checks( } DataTestResolved::Custom { path, body } => { // dbt singular-test convention: the body is a *single* SELECT - // (or CTE) returning the violating rows. It is embedded as a - // subquery (`FROM ()`), so a multi-statement body would - // produce invalid SQL — validate up front with an actionable - // error. It runs in the target's connection (can read - // `_wm_target` + the user's attaches); partition substitution is - // already applied by the worker. + // (or CTE) returning the violating rows, reading the + // freshly-materialized target through the internal `_wm_target` + // schema. It is embedded as a subquery (`FROM ()`), so a + // multi-statement or non-SELECT body would produce invalid SQL. + // Neither rule is documented or scaffolded elsewhere, so the + // errors are self-teaching: they name the exact violation and + // append a correct one-line example. It runs in the target's + // connection (can read `_wm_target` + the user's attaches); + // partition substitution is already applied by the worker. + let hint = custom_test_hint(t); let stmts = split_statements(body); if stmts.is_empty() { - return Err(format!("data_test custom `{path}`: empty test body")); + return Err(format!( + "data_test custom `{path}`: empty test body. {hint}" + )); } if stmts.len() > 1 { return Err(format!( - "data_test custom `{path}`: must be a single SELECT returning the \ - violating rows (found {} statements)", + "data_test custom `{path}`: a custom data test must be a single SELECT, \ + but found {} statements. {hint}", stmts.len() )); } - push_check(&mut out, format!("custom({path})"), stmts[0].to_string()); + let stmt = &stmts[0]; + if classify_block(stmt) != BlockClass::Output { + return Err(format!( + "data_test custom `{path}`: a custom data test must be a single SELECT, \ + not a write or DDL statement. {hint}" + )); + } + if !references_target(stmt) { + return Err(format!( + "data_test custom `{path}`: the test never reads the freshly-materialized \ + target — reference it through the internal `{TARGET_ALIAS}` schema (as \ + `{t}`), not the table name on its own. {hint}" + )); + } + push_check(&mut out, format!("custom({path})"), stmt.to_string()); } } } @@ -2343,13 +2385,85 @@ mod tests { #[test] fn data_test_custom_rejects_multi_statement_body() { // The body is embedded as a subquery, so a setup-then-SELECT body would - // produce invalid SQL — reject it up front with an actionable error. + // produce invalid SQL — reject it up front with a self-teaching error + // that names the violation and shows the correct single-SELECT shape. let tests = vec![DataTestResolved::Custom { path: "f/tests/amount".into(), body: "SET threads = 1; SELECT * FROM _wm_target.orders WHERE amount < 0".into(), }]; let err = build_data_test_checks(&tests, &ctx_unpartitioned()).unwrap_err(); assert!(err.contains("single SELECT"), "unexpected error: {err}"); + assert!( + err.contains("found 2 statements"), + "unexpected error: {err}" + ); + // the copyable example points at the internal target alias. + assert!( + err.contains("SELECT * FROM _wm_target.orders WHERE "), + "unexpected error: {err}" + ); + } + + #[test] + fn data_test_custom_rejects_non_select_body() { + // A write/DDL body can't be embedded as `FROM ()`; the error must + // say so and teach the single-SELECT convention. + let tests = vec![DataTestResolved::Custom { + path: "f/tests/amount".into(), + body: "DELETE FROM _wm_target.orders WHERE amount < 0".into(), + }]; + let err = build_data_test_checks(&tests, &ctx_unpartitioned()).unwrap_err(); + assert!(err.contains("single SELECT"), "unexpected error: {err}"); + assert!( + err.contains("SELECT * FROM _wm_target.orders WHERE "), + "unexpected error: {err}" + ); + } + + #[test] + fn data_test_custom_rejects_wrong_target_alias() { + // Referencing the target by its bare table name (not `_wm_target.
`) + // is the most common custom-test mistake — the runtime only attaches the + // freshly-materialized target under `_wm_target`, so the query would fail + // at runtime. Catch it at codegen with a self-teaching error. + let tests = vec![DataTestResolved::Custom { + path: "f/tests/amount".into(), + body: "SELECT * FROM orders WHERE amount < 0".into(), + }]; + let err = build_data_test_checks(&tests, &ctx_unpartitioned()).unwrap_err(); + assert!(err.contains("_wm_target"), "unexpected error: {err}"); + assert!( + err.contains("SELECT * FROM _wm_target.orders WHERE "), + "unexpected error: {err}" + ); + } + + #[test] + fn data_test_custom_accepts_from_first_and_uppercased_alias() { + // DuckDB's FROM-first syntax is a valid Output, and the alias match is + // case-insensitive (SQL identifiers are), so this passes. + let tests = vec![DataTestResolved::Custom { + path: "f/tests/amount".into(), + body: "FROM _WM_TARGET.orders WHERE amount < 0".into(), + }]; + let sql = build_data_test_checks(&tests, &ctx_unpartitioned()).unwrap(); + assert!(sql.checks[0] + .probe + .contains("FROM (FROM _WM_TARGET.orders WHERE amount < 0) _wm_v")); + } + + #[test] + fn data_test_custom_empty_body_teaches_shape() { + let tests = vec![DataTestResolved::Custom { + path: "f/tests/amount".into(), + body: " \n-- just a comment\n".into(), + }]; + let err = build_data_test_checks(&tests, &ctx_unpartitioned()).unwrap_err(); + assert!(err.contains("empty test body"), "unexpected error: {err}"); + assert!( + err.contains("SELECT * FROM _wm_target.orders WHERE "), + "unexpected error: {err}" + ); } #[test] diff --git a/frontend/src/lib/components/assets/AssetGraph/pipelineTemplates.dataTest.test.ts b/frontend/src/lib/components/assets/AssetGraph/pipelineTemplates.dataTest.test.ts new file mode 100644 index 0000000000..2acdf2abfb --- /dev/null +++ b/frontend/src/lib/components/assets/AssetGraph/pipelineTemplates.dataTest.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from 'vitest' +import { + autoOutputAsset, + compatibleOutputKinds, + generatePipelineDraft, + PIPELINE_OUTPUT_KINDS +} from './pipelineTemplates' + +// The `data_test` output kind scaffolds a *custom* (singular) data test: a +// standalone DuckDB script referenced from a materialize script's +// `-- data_test ` line. It must be a single SELECT that reads the +// freshly-materialized target through the internal `_wm_target` schema — the +// two rules the backend's self-teaching errors enforce. +describe('data_test scaffold', () => { + it('is a DuckDB-only output kind exposed in the picker', () => { + expect(compatibleOutputKinds('duckdb')).toContain('data_test') + expect(compatibleOutputKinds('python3')).not.toContain('data_test') + expect(PIPELINE_OUTPUT_KINDS.map((k) => k.id)).toContain('data_test') + }) + + it('produces no output asset (it asserts against an existing target)', () => { + expect(autoOutputAsset('data_test', 'folder', 'duckdb')).toBeUndefined() + }) + + it('scaffolds a single SELECT against `_wm_target.
`', () => { + const src = generatePipelineDraft({ + language: 'duckdb', + outputKind: 'data_test', + triggers: [] + }) + // starter body is a single SELECT against the internal target alias. + expect(src).toContain('SELECT * FROM _wm_target.your_table WHERE your_condition;') + // exactly one SQL statement (single SELECT) — count statement lines, not + // the word "SELECT" that also appears in the guidance comment. + const stmtLines = src.split('\n').filter((l) => /^\s*SELECT\b/i.test(l)) + expect(stmtLines).toHaveLength(1) + // no `-- materialize ` output annotation — a data test declares no + // asset (the word still appears in the guidance comment, which is fine). + expect(src).not.toMatch(/^--\s*materialize\s/m) + // teaches how to wire it up + the offending-rows convention. + expect(src).toContain('-- data_test ') + expect(src).toContain('offending rows') + }) + + it('seeds the table name from an upstream ducklake asset when present', () => { + const src = generatePipelineDraft({ + language: 'duckdb', + outputKind: 'data_test', + input: { kind: 'ducklake', path: 'analytics/orders' }, + triggers: [] + }) + expect(src).toContain('SELECT * FROM _wm_target.orders WHERE your_condition;') + // no ATTACH of the input — the runtime attaches the target as `_wm_target`. + expect(src).not.toContain('ATTACH') + }) +}) diff --git a/frontend/src/lib/components/assets/AssetGraph/pipelineTemplates.ts b/frontend/src/lib/components/assets/AssetGraph/pipelineTemplates.ts index 11077d3609..f81d1a330d 100644 --- a/frontend/src/lib/components/assets/AssetGraph/pipelineTemplates.ts +++ b/frontend/src/lib/components/assets/AssetGraph/pipelineTemplates.ts @@ -17,6 +17,7 @@ export type PipelineOutputKind = | 'datatable' | 'ducklake' | 'materialize' + | 'data_test' | 's3_parquet' | 's3_object' | 'macros' @@ -37,6 +38,11 @@ export const PIPELINE_OUTPUT_KINDS: PipelineOutputKindMeta[] = [ label: 'Materialized table', description: 'Managed DuckLake table — idempotent, versioned, tracked' }, + { + id: 'data_test', + label: 'Data test', + description: 'Custom assertion — a single SELECT returning offending rows (empty = pass)' + }, { id: 'datatable', label: 'Data table', @@ -83,7 +89,16 @@ const LANG_COMPATIBILITY: Record = { // single SELECT. The Python/TS `wmll.ducklake` helper currently takes a SQL // SELECT (not in-memory rows), so a polyglot managed materialize is a // separate follow-up — those langs keep the `ducklake` raw-write kind. - duckdb: ['materialize', 'datatable', 'ducklake', 's3_parquet', 's3_object', 'macros', 'none'], + duckdb: [ + 'materialize', + 'data_test', + 'datatable', + 'ducklake', + 's3_parquet', + 's3_object', + 'macros', + 'none' + ], postgresql: ['datatable', 'none'], mysql: ['none'], mssql: ['none'], @@ -183,8 +198,10 @@ export function autoOutputAsset( } } // A macro library produces no asset — its "output" is the registry - // entries the deploy records. + // entries the deploy records. A custom data test produces no asset + // either — it asserts against an existing materialized target. case 'macros': + case 'data_test': case 'none': return undefined } @@ -316,6 +333,17 @@ export type TemplateContext = { function header(ctx: TemplateContext): string { const { language, triggers, output, outputKind } = ctx const p = commentPrefix(language) + // A custom data test is a standalone script, not a graph node that produces + // an asset — so it gets no `// pipeline` / output annotation. Instead, tell + // the author how to wire it up (the `data_test` reference) and the two rules + // that aren't obvious: single SELECT, returning the offending rows. + if (outputKind === 'data_test') { + return [ + `${p} Custom data test — reference it from a materialize script with \`${p} data_test \`.`, + `${p} It must be a single SELECT returning the offending rows; the run fails if any row comes back.`, + '' + ].join('\n') + } const lines = triggers.map((t) => { switch (t.kind) { case 'asset': @@ -544,6 +572,22 @@ function bodyPython(ctx: TemplateContext): string { function bodyDuckdb(ctx: TemplateContext): string { const { input, output, outputKind } = ctx const dataUpload = isDataUpload(ctx.triggers) + if (outputKind === 'data_test') { + // Standalone custom data test: it reads ONLY the freshly-materialized + // target, which the runtime attaches under the internal `_wm_target` + // schema — so it emits no ATTACH / input load of its own. When the test + // was created off a ducklake asset, seed its table name; otherwise a + // clear placeholder. This exact shape (single SELECT vs `_wm_target`) is + // what the backend's self-teaching errors ask for. + const testTable = input?.kind === 'ducklake' ? catalogTableRef(input.path) : 'your_table' + return [ + '', + `-- Return the rows that VIOLATE your assertion; an empty result means the test passes.`, + `-- \`_wm_target\` is the freshly-materialized target, attached by the runtime.`, + `SELECT * FROM _wm_target.${testTable} WHERE your_condition;`, + '' + ].join('\n') + } const lines: string[] = [] if (dataUpload) { // `(s3object)` param declaration → the run form renders the S3 picker