From 6f37588c481cf15e63457ce2d20b175c756e5514 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 7 Aug 2026 08:14:53 +0000 Subject: [PATCH] fix: address review nits on the duckdb bump Name the unsupported DuckDB type rather than dumping the value, which may be arbitrarily large or hold data that does not belong in an error message, and say which column it came from. Co-Authored-By: Claude Opus 5 (1M context) --- backend/ee-repo-ref.txt | 2 +- backend/windmill-duckdb-ffi-internal/src/lib.rs | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index d72cf39517..f2f60b0de3 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -ec55b3c383631f7a0eefa7266a5c474b518b6aea \ No newline at end of file +a3ab9e057452a6508e7402c96a866dc1e78aaf44 \ No newline at end of file diff --git a/backend/windmill-duckdb-ffi-internal/src/lib.rs b/backend/windmill-duckdb-ffi-internal/src/lib.rs index 2cec7a2185..7577d98b94 100644 --- a/backend/windmill-duckdb-ffi-internal/src/lib.rs +++ b/backend/windmill-duckdb-ffi-internal/src/lib.rs @@ -621,7 +621,8 @@ fn row_to_value( for (i, key) in column_names.iter().enumerate() { let value: duckdb::types::Value = row.get(i).map_err(|e| e.to_string())?; let type_alias = &type_aliases[i]; - let json_value = duckdb_value_to_json_value(value, type_alias)?; + let json_value = duckdb_value_to_json_value(value, type_alias) + .map_err(|e| format!("column \"{key}\": {e}"))?; obj.insert(key.clone(), json_value); } serde_json::value::to_raw_value(&obj).map_err(|e| e.to_string()) @@ -715,12 +716,15 @@ fn duckdb_value_to_json_value( ), duckdb::types::Value::Union(value) => serde_json::Value::String(format!("{:?}", *value)), // `Value` is `#[non_exhaustive]`: a newer engine can hand back a variant this - // build has never seen. Name it instead of emitting a plausible-looking - // rendering that silently misrepresents the column. + // build has never seen. Name its type instead of emitting a plausible-looking + // rendering that silently misrepresents the column. The type, never the value: + // a row that reaches here is already unprintable, and it may be arbitrarily + // large or hold data that does not belong in a job's error message. other => { return Err(format!( - "Unsupported DuckDB value for this build: {:?}", - other + "DuckDB type {:?} is not supported by this worker's engine bindings; \ + cast the column to a supported type (e.g. VARCHAR) to return it", + other.data_type() )) } };