Skip to content

Prevent silent column type coercion #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
VLCDaniel merged 1 commit into from
Apr 9, 2026
Merged

Prevent silent column type coercion #14

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions crates/core/src/operations/update.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ use datafusion::{
error::DataFusionError,
execution::context::SessionState,
logical_expr::{
Extension, LogicalPlan, LogicalPlanBuilder, UserDefinedLogicalNode, case, col, lit, when,
Extension, LogicalPlan, LogicalPlanBuilder, UserDefinedLogicalNode, case, cast, col, lit,
when,
},
physical_plan::{ExecutionPlan, metrics::MetricBuilder},
physical_planner::{ExtensionPlanner, PhysicalPlanner},
Expand Down Expand Up @@ -342,10 +343,18 @@ async fn execute(
.into_iter()
.map(|field| {
let expr = match updates.get(field.name()) {
Some(expr) => case(col(UPDATE_PREDICATE_COLNAME))
.when(lit(true), expr.to_owned())
.otherwise(col(Column::from_name(field.name())))?
.alias(field.name()),
Some(expr) => {
// Cast the update expression to the target column type so that
// (a) the CASE branches are always the same type and DataFusion cannot
// silently widen the column (e.g. Int32 → Utf8), and
// (b) incompatible assignments (e.g. a non-numeric string into an Int32
// column) are caught during plan optimisation via constant folding.
let typed_expr = cast(expr.to_owned(), field.data_type().clone());
Copy link
Copy Markdown
Collaborator Author

@VLCDaniel VLCDaniel Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DataFusion silently coerces incompatible types in CASE expressions (e.g. promoting Int32 to Utf8 when a string expression is assigned to an integer column) instead of erroring. Without an explicit check, a type-mismatched update would succeed and write a parquet file with the wrong column type

test_expected_failures exposed this, this is how it fails now:

  Optimizer rule 'simplify_expressions' failed
  caused by
  Arrow error: Cast error: Cannot cast string 'a string' to value of Int32 type

case(col(UPDATE_PREDICATE_COLNAME))
.when(lit(true), typed_expr)
.otherwise(col(Column::from_name(field.name())))?
.alias(field.name())
}
None => col(Column::from_name(field.name())),
};
Ok::<_, DataFusionError>(expr)
Expand Down
Loading