mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
feat(pipelines): self-teaching custom data_test errors + scaffold (#9937)
Custom `// data_test <path>` scripts must be a single SELECT reading the freshly-materialized target via the internal `_wm_target.<table>` alias — neither was documented or scaffolded. Make the codegen errors name the exact violation (multi-statement, non-SELECT, wrong alias, empty) and append a copyable `SELECT * FROM _wm_target.<table> WHERE <condition>` example. Add a DuckDB-only 'Data test' pipeline output kind that scaffolds that starter body. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
GitHub
@@ -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.<table>` alias — and doubles that alias into a copyable
|
||||
// one-line example. `target_qualified` is already `_wm_target.<table>`.
|
||||
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 <condition>` — 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 (<body>)`), 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 (<body>)`), 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 <condition>"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn data_test_custom_rejects_non_select_body() {
|
||||
// A write/DDL body can't be embedded as `FROM (<body>)`; 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 <condition>"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn data_test_custom_rejects_wrong_target_alias() {
|
||||
// Referencing the target by its bare table name (not `_wm_target.<table>`)
|
||||
// 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 <condition>"),
|
||||
"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 <condition>"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -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 <path>` 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.<table>`', () => {
|
||||
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 <uri>` 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 <this-script-path>')
|
||||
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')
|
||||
})
|
||||
})
|
||||
@@ -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<ScriptLang, PipelineOutputKind[]> = {
|
||||
// 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 <this-script-path>\`.`,
|
||||
`${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
|
||||
|
||||
Reference in New Issue
Block a user