mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
fix(duckdb): cast list columns in quicksearch so tables containing them can be previewed (#10614)
* fix(duckdb): cast columns in quicksearch so nested types can be previewed
DuckDB's `CONCAT` implicitly casts scalars but rejects nested types:
D SELECT CONCAT(' ', ['a','b']);
Binder Error: Cannot concatenate types VARCHAR and VARCHAR[] - an explicit
cast is required
Quicksearch concatenates every visible column, so one LIST, STRUCT or MAP column
makes a table impossible to preview — both the grid and its row count fail:
Binder Error: Cannot concatenate types VARCHAR, VARCHAR, BIGINT, ...,
VARCHAR[], ... and TIMESTAMP WITH TIME ZONE - an explicit cast is required
LINE 1: ... FROM "raw"."accounts" WHERE ($1 = '' OR CONCAT(' ', "id", ...
Every scalar in that list concatenates fine on its own — VARCHAR, BIGINT,
DOUBLE, BOOLEAN, DATE and TIMESTAMPTZ were each checked individually — so the
array column is the entire cause.
Cast each column in the predicate. The comparison is textual either way, so no
result changes, and the projection is untouched: casting there would change the
types the caller reads back. This follows the shape already used for MSSQL in
`mssql_needs_cast_for_eq`.
Both DuckDB quicksearch sites are covered, SELECT and COUNT. Fixing one leaves
the grid rendering while the row count still errors.
Tests include the live path: the Database Manager sends a
`-- WM_INTERNAL_DB_SELECT {...}` marker and the backend expands it, so the new
test drives that expansion with the real 27-column definition captured from a
failing job, `sync_id VARCHAR[]` included. It fails without the fix and passes
with it.
* fix(frontend): cast columns in the DuckDB quicksearch
Same defect as the Rust query builders, in the implementation that actually
runs. `make_select_query` / `make_count_query` in windmill-common have no callers
anywhere in the repo; the query the browser sends is built here.
DuckDB's CONCAT implicitly casts scalars but rejects nested types, and
quicksearch concatenates every visible column, so one LIST column makes a table
impossible to preview — both the page and its row count fail with
Binder Error: Cannot concatenate types VARCHAR, ..., VARCHAR[], ... and
TIMESTAMP WITH TIME ZONE - an explicit cast is required
The helper lives in select.ts and is imported by count.ts so the two cannot
drift, and both call sites are fixed: fixing only SELECT leaves the grid
rendering while the row count still errors.
* fix(duckdb): cast only list columns in quicksearch, leaving other SQL byte-identical
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(frontend): pin the DuckDB quicksearch column list byte-for-byte
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Ruben Fiszel <ruben@windmill.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -524,6 +524,26 @@ fn cols_to_simple(cols: &[ColumnDef]) -> Vec<SimpleColumn> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// DuckDB's `concat` doubles as list concatenation, so a LIST or ARRAY beside a
|
||||
/// VARCHAR is a binder error rather than an implicit cast, and quicksearch
|
||||
/// concatenates every visible column, so one of them makes the table
|
||||
/// unpreviewable. Everything else concatenates as text and stays uncast: the
|
||||
/// frontend twin of this predicate feeds app policy digests that a changed
|
||||
/// string invalidates.
|
||||
fn duckdb_quicksearch_columns(column_defs: &[ColumnDef]) -> String {
|
||||
visible_column_defs(column_defs)
|
||||
.map(|c| {
|
||||
let quoted = render_db_quoted_identifier(&c.field, DbType::Duckdb);
|
||||
if c.datatype.trim_end().ends_with(']') {
|
||||
format!("CAST({} AS VARCHAR)", quoted)
|
||||
} else {
|
||||
quoted
|
||||
}
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ")
|
||||
}
|
||||
|
||||
/// MSSQL `text`, `ntext`, and `image` types cannot be used with the `=` operator.
|
||||
/// This function wraps the column/param in CAST(...) when needed.
|
||||
fn mssql_needs_cast_for_eq(datatype: &str) -> bool {
|
||||
@@ -578,10 +598,14 @@ fn qi(identifier: &str, db_type: DbType) -> String {
|
||||
render_db_quoted_identifier(identifier, db_type)
|
||||
}
|
||||
|
||||
/// The columns a table preview shows, in the order [`build_visible_field_list`]
|
||||
/// renders them.
|
||||
fn visible_column_defs(column_defs: &[ColumnDef]) -> impl Iterator<Item = &ColumnDef> {
|
||||
column_defs.iter().filter(|c| c.ignored != Some(true))
|
||||
}
|
||||
|
||||
pub fn build_visible_field_list(column_defs: &[ColumnDef], db_type: DbType) -> Vec<String> {
|
||||
column_defs
|
||||
.iter()
|
||||
.filter(|c| c.ignored != Some(true))
|
||||
visible_column_defs(column_defs)
|
||||
.map(|c| render_db_quoted_identifier(&c.field, db_type))
|
||||
.collect()
|
||||
}
|
||||
@@ -1019,7 +1043,7 @@ pub fn make_select_query(
|
||||
|
||||
let quicksearch = format!(
|
||||
"($quicksearch = '' OR CONCAT({}) ILIKE '%' || $quicksearch || '%')",
|
||||
filtered_columns.join(", ")
|
||||
duckdb_quicksearch_columns(column_defs)
|
||||
);
|
||||
|
||||
query.push_str(&format!(
|
||||
@@ -1188,7 +1212,7 @@ pub fn make_count_query(
|
||||
if !filtered_columns.is_empty() {
|
||||
quicksearch_condition.push_str(&format!(
|
||||
" ($quicksearch = '' OR CONCAT(' ', {}) LIKE CONCAT('%', $quicksearch, '%'))",
|
||||
filtered_columns.join(", ")
|
||||
duckdb_quicksearch_columns(column_defs)
|
||||
));
|
||||
} else {
|
||||
quicksearch_condition.push_str(" ($quicksearch = '' OR 1 = 1)");
|
||||
@@ -3130,6 +3154,40 @@ mod tests {
|
||||
assert!(result.contains("LIMIT $limit::INT OFFSET $offset::INT"));
|
||||
}
|
||||
|
||||
/// Both the SELECT and the COUNT build the quicksearch predicate; fixing only
|
||||
/// one leaves the grid rendering while the row count errors out.
|
||||
#[test]
|
||||
fn test_duckdb_quicksearch_casts_only_list_columns() {
|
||||
let cols = vec![
|
||||
col("id", "VARCHAR"),
|
||||
col("tags", "VARCHAR[]"),
|
||||
col("pos", "INTEGER[3]"),
|
||||
col("meta", "STRUCT(a INTEGER)"),
|
||||
];
|
||||
|
||||
let select =
|
||||
make_select_query("my_table", &cols, None, DbType::Duckdb, None, None).unwrap();
|
||||
assert!(
|
||||
select.contains(
|
||||
"CONCAT(\"id\", CAST(\"tags\" AS VARCHAR), CAST(\"pos\" AS VARCHAR), \"meta\") ILIKE"
|
||||
),
|
||||
"got:\n{}",
|
||||
select
|
||||
);
|
||||
// The projection stays untouched: casting there would change the types
|
||||
// the caller reads back.
|
||||
assert!(select.contains("SELECT \"id\", \"tags\", \"pos\", \"meta\" FROM \"my_table\""));
|
||||
|
||||
let count = make_count_query(DbType::Duckdb, "my_table", None, &cols, None).unwrap();
|
||||
assert!(
|
||||
count.contains(
|
||||
"CONCAT(' ', \"id\", CAST(\"tags\" AS VARCHAR), CAST(\"pos\" AS VARCHAR), \"meta\") LIKE"
|
||||
),
|
||||
"got:\n{}",
|
||||
count
|
||||
);
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// SELECT - error cases
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
+35
@@ -0,0 +1,35 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { buildVisibleFieldList, duckdbQuicksearchColumns, type ColumnDef } from './utils'
|
||||
|
||||
function col(field: string, datatype: string, extra: Partial<ColumnDef> = {}): ColumnDef {
|
||||
return { field, datatype, ...extra } as ColumnDef
|
||||
}
|
||||
|
||||
describe('duckdbQuicksearchColumns', () => {
|
||||
it('casts list and array columns, and nothing else', () => {
|
||||
expect(
|
||||
duckdbQuicksearchColumns([
|
||||
col('id', 'VARCHAR'),
|
||||
col('tags', 'VARCHAR[]'),
|
||||
col('pos', 'INTEGER[3]'),
|
||||
col('meta', 'STRUCT(a INTEGER)')
|
||||
])
|
||||
).toBe('"id", CAST("tags" AS VARCHAR), CAST("pos" AS VARCHAR), "meta"')
|
||||
})
|
||||
|
||||
// This byte-identity is what keeps the policy digest of an already-deployed
|
||||
// Database Studio app valid; a table with no list column must produce the
|
||||
// query it produced before quicksearch learned to cast anything.
|
||||
it('emits the plain column list when no column is a list', () => {
|
||||
const columnDefs = [col('id', 'VARCHAR'), col('n', 'INTEGER'), col('at', 'TIMESTAMP')]
|
||||
expect(duckdbQuicksearchColumns(columnDefs)).toBe(
|
||||
buildVisibleFieldList(columnDefs, 'duckdb').join(', ')
|
||||
)
|
||||
})
|
||||
|
||||
it('skips ignored columns', () => {
|
||||
expect(
|
||||
duckdbQuicksearchColumns([col('id', 'VARCHAR'), col('tags', 'VARCHAR[]', { ignored: true })])
|
||||
).toBe('"id"')
|
||||
})
|
||||
})
|
||||
@@ -11,7 +11,12 @@ import type { AppInput, RunnableByName } from '$lib/components/apps/inputType'
|
||||
import { wrapDucklakeQuery } from '../../../../../ducklake'
|
||||
import type { DbType, DbInput } from '$lib/components/dbTypes'
|
||||
import { buildParameters } from '../utils'
|
||||
import { getLanguageByResourceType, type ColumnDef, buildVisibleFieldList } from '../utils'
|
||||
import {
|
||||
getLanguageByResourceType,
|
||||
type ColumnDef,
|
||||
buildVisibleFieldList,
|
||||
duckdbQuicksearchColumns
|
||||
} from '../utils'
|
||||
|
||||
export function makeCountQuery(
|
||||
dbType: DbType,
|
||||
@@ -118,8 +123,8 @@ export function makeCountQuery(
|
||||
}
|
||||
case 'duckdb':
|
||||
if (filteredColumns.length > 0) {
|
||||
quicksearchCondition += ` ($quicksearch = '' OR CONCAT(' ', ${filteredColumns.join(
|
||||
', '
|
||||
quicksearchCondition += ` ($quicksearch = '' OR CONCAT(' ', ${duckdbQuicksearchColumns(
|
||||
columnDefs
|
||||
)}) LIKE CONCAT('%', $quicksearch, '%'))`
|
||||
} else {
|
||||
quicksearchCondition += ` ($quicksearch = '' OR 1 = 1)`
|
||||
|
||||
@@ -11,7 +11,12 @@ import type { AppInput, RunnableByName } from '$lib/components/apps/inputType'
|
||||
import { wrapDucklakeQuery } from '../../../../../ducklake'
|
||||
import type { DbType, DbInput } from '$lib/components/dbTypes'
|
||||
import { buildParameters } from '../utils'
|
||||
import { getLanguageByResourceType, type ColumnDef, buildVisibleFieldList } from '../utils'
|
||||
import {
|
||||
getLanguageByResourceType,
|
||||
type ColumnDef,
|
||||
buildVisibleFieldList,
|
||||
duckdbQuicksearchColumns
|
||||
} from '../utils'
|
||||
|
||||
function makeSnowflakeSelectQuery(
|
||||
table: string,
|
||||
@@ -298,8 +303,8 @@ CASE WHEN :order_by = '${column.field}' AND :is_desc IS true THEN \`${column.fie
|
||||
)
|
||||
.join(',\n')}`
|
||||
|
||||
quicksearchCondition = `($quicksearch = '' OR CONCAT(${filteredColumns.join(
|
||||
', '
|
||||
quicksearchCondition = `($quicksearch = '' OR CONCAT(${duckdbQuicksearchColumns(
|
||||
columnDefs
|
||||
)}) ILIKE '%' || $quicksearch || '%')`
|
||||
|
||||
query += `SELECT ${filteredColumns.join(', ')} FROM ${table}\n`
|
||||
|
||||
@@ -305,11 +305,32 @@ export async function formatGraphqlSchema(schema: IntrospectionQuery): Promise<s
|
||||
return printSchema(buildClientSchema(schema))
|
||||
}
|
||||
|
||||
// Filter out hidden columns to avoid counting the wrong number of rows
|
||||
function visibleColumnDefs(columnDefs: ColumnDef[]): ColumnDef[] {
|
||||
return columnDefs.filter((columnDef: ColumnDef) => columnDef && columnDef.ignored !== true)
|
||||
}
|
||||
|
||||
export function buildVisibleFieldList(columnDefs: ColumnDef[], dbType: DbType) {
|
||||
// Filter out hidden columns to avoid counting the wrong number of rows
|
||||
return columnDefs
|
||||
.filter((columnDef: ColumnDef) => columnDef && columnDef.ignored !== true)
|
||||
.map((column) => renderDbQuotedIdentifier(column?.field, dbType))
|
||||
return visibleColumnDefs(columnDefs).map((column) =>
|
||||
renderDbQuotedIdentifier(column?.field, dbType)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* DuckDB's `concat` doubles as list concatenation, so a LIST or ARRAY beside a
|
||||
* VARCHAR is a binder error rather than an implicit cast, and quicksearch
|
||||
* concatenates every visible column, so one of them makes the table
|
||||
* unpreviewable. Everything else concatenates as text and stays uncast: this
|
||||
* query is digested into the policy of every deployed Database Studio app, and
|
||||
* a changed string invalidates it until the app is redeployed.
|
||||
*/
|
||||
export function duckdbQuicksearchColumns(columnDefs: ColumnDef[]): string {
|
||||
return visibleColumnDefs(columnDefs)
|
||||
.map((column) => {
|
||||
const quoted = renderDbQuotedIdentifier(column?.field, 'duckdb')
|
||||
return column?.datatype?.trimEnd().endsWith(']') ? `CAST(${quoted} AS VARCHAR)` : quoted
|
||||
})
|
||||
.join(', ')
|
||||
}
|
||||
|
||||
export function renderDbQuotedIdentifier(identifier: string, dbType: DbType): string {
|
||||
|
||||
Reference in New Issue
Block a user