Skip to content

Fix crash on unparseable timestamps in stats JSON #12

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

Closed
VLCDaniel wants to merge 2 commits into from
Closed

Fix crash on unparseable timestamps in stats JSON #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f94d506
[HSTACK] - fix crash on unparseable timestamps in stats JSON
Apr 7, 2026
d999da1
fix: reject update expressions with incompatible column types
Apr 7, 2026
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
16 changes: 15 additions & 1 deletion crates/core/src/kernel/snapshot/iterators/scan_row.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use delta_kernel::engine::arrow_conversion::TryIntoKernel;
use delta_kernel::engine::arrow_data::ArrowEngineData;
use delta_kernel::engine::parse_json;
use delta_kernel::EngineData;
use delta_kernel::expressions::Scalar;
use delta_kernel::expressions::UnaryExpressionOp;
use delta_kernel::scan::scan_row_schema;
Expand Down Expand Up @@ -109,7 +110,7 @@
)?)
}

pub(crate) fn parse_stats_column_with_schema(

Check warning on line 113 in crates/core/src/kernel/snapshot/iterators/scan_row.rs

View workflow job for this annotation

GitHub Actions / Integration Tests (LakeFS v1.48) 

function `parse_stats_column_with_schema` is never used

Check warning on line 113 in crates/core/src/kernel/snapshot/iterators/scan_row.rs

View workflow job for this annotation

GitHub Actions / build (ubuntu-latest) 

function `parse_stats_column_with_schema` is never used

Check warning on line 113 in crates/core/src/kernel/snapshot/iterators/scan_row.rs

View workflow job for this annotation

GitHub Actions / aws-native-tls 

function `parse_stats_column_with_schema` is never used

Check warning on line 113 in crates/core/src/kernel/snapshot/iterators/scan_row.rs

View workflow job for this annotation

GitHub Actions / Integration Tests (HDFS) 

function `parse_stats_column_with_schema` is never used
sn: &KernelSnapshot,
batch: &RecordBatch,
stats_schema: KernelSchemaRef,
Expand Down Expand Up @@ -142,7 +143,20 @@
let stats_batch = batch.project(&[stats_idx])?;
let stats_data = Box::new(ArrowEngineData::new(stats_batch));

let parsed = parse_json(stats_data, stats_schema)?;
let parsed: Box<dyn EngineData> = match parse_json(stats_data, stats_schema.clone()) {
Ok(p) => p,
Err(e) => {
warn!("Failed to parse stats JSON, treating stats as absent: {e}");
let arrow_schema: Arc<arrow_schema::Schema> = Arc::new(stats_schema.as_ref().try_into_arrow()?);
let null_columns: Vec<_> = arrow_schema
.fields()
.iter()
.map(|f| new_null_array(f.data_type(), batch.num_rows()))
.collect();
let null_batch = RecordBatch::try_new(arrow_schema, null_columns)?;
Box::new(ArrowEngineData::new(null_batch))
}
};
let parsed: RecordBatch = ArrowEngineData::try_from_engine_data(parsed)?.into();

let stats_array: Arc<StructArray> = Arc::new(parsed.into());
Expand Down
20 changes: 15 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 @@ -262,6 +263,7 @@ impl ExtensionPlanner for UpdateMetricExtensionPlanner {
}
}


#[allow(clippy::too_many_arguments)]
#[tracing::instrument(
skip_all,
Expand Down Expand Up @@ -342,10 +344,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