Skip to content

Commit 4582f1e

Browse files
committed
feat: implement gated validation for extension types to prevent metadata propagation
1 parent 38f84dc commit 4582f1e

2 files changed

Lines changed: 67 additions & 34 deletions

File tree

‎datafusion/expr/src/logical_plan/builder.rs‎

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use std::cmp::Ordering;
2222
use std::collections::{HashMap, HashSet};
2323
use std::iter::once;
2424
use std::sync::Arc;
25+
use datafusion_common::metadata::check_metadata_with_storage_equal;
2526

2627
use crate::dml::CopyTo;
2728
use crate::expr::{Alias, PlannedReplaceSelectItem, Sort as SortExpr};
@@ -274,28 +275,42 @@ impl LogicalPlanBuilder {
274275
for j in 0..n_cols {
275276
let field = schema.field(j);
276277
let field_type = field.data_type();
277-
let field_nullable = field.is_nullable();
278-
let field_metadata = FieldMetadata::new_from_field(field);
278+
let field_metadata = field.metadata();
279+
let is_target_ext = field_metadata.contains_key(arrow_schema::extension::EXTENSION_TYPE_NAME_KEY);
280+
let column_name = format!("column {}", j);
279281
for row in values.iter() {
280282
let value = &row[j];
281-
let data_type = value.get_type(schema)?;
282-
283-
if !data_type.equals_datatype(field_type)
284-
&& !can_cast_types(&data_type, field_type)
285-
{
286-
return exec_err!(
287-
"type mismatch and can't cast to got {} and {}",
288-
data_type,
289-
field_type
290-
);
283+
let value_type = value.get_type(schema)?;
284+
if value_type == DataType::Null { continue; }
285+
286+
if is_target_ext {
287+
let value_meta = value.metadata(schema)?;
288+
let value_meta_map = value_meta.to_hashmap();
289+
let is_value_ext = value_meta_map.contains_key(arrow_schema::extension::EXTENSION_TYPE_NAME_KEY);
290+
291+
if is_value_ext && value_type == *field_type {
292+
check_metadata_with_storage_equal(
293+
(&value_type, Some(&value_meta_map)),
294+
(field_type, Some(field_metadata)),
295+
&column_name,
296+
" in VALUES list",
297+
)?;
298+
} else if !can_cast_types(&value_type, field_type) {
299+
return plan_err!("Cannot cast {} to extension type at column {}", value_type, column_name);
300+
}
301+
} else {
302+
// Optimized path for standard types
303+
if !value_type.equals_datatype(field_type) && !can_cast_types(&value_type, field_type) {
304+
return plan_err!("Cannot cast {} to {} for column {}", value_type, field_type, column_name);
305+
}
291306
}
292307
}
293-
let metadata = if field_metadata.is_empty() {
294-
None
295-
} else {
296-
Some(field_metadata)
297-
};
298-
fields.push_with_metadata(field_type.to_owned(), field_nullable, metadata);
308+
309+
fields.push_with_metadata(
310+
field_type.clone(),
311+
field.is_nullable(),
312+
Some(FieldMetadata::new_from_field(field))
313+
);
299314
}
300315

301316
Self::infer_inner(values, fields, schema)

‎datafusion/sql/src/statement.rs‎

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ use datafusion_expr::dml::{CopyTo, InsertOp};
4343
use datafusion_expr::expr_rewriter::normalize_col_with_schemas_and_ambiguity_check;
4444
use datafusion_expr::logical_plan::DdlStatement;
4545
use datafusion_expr::logical_plan::builder::project;
46+
use datafusion_common::metadata::check_metadata_with_storage_equal;
4647
use datafusion_expr::utils::expr_to_columns;
4748
use datafusion_expr::{
4849
Analyze, Cast, CreateCatalog, CreateCatalogSchema,
@@ -530,23 +531,40 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
530531
.fields()
531532
.iter()
532533
.zip(input_fields)
533-
.map(|(field, input_field)| {
534-
let metadata =
535-
FieldMetadata::new_from_field(field.as_ref());
536-
let alias_metadata = if metadata.is_empty() {
537-
None
534+
.map(|(field, input_field)| -> Result<Expr> {
535+
let input_meta = input_field.metadata();
536+
let target_meta = field.metadata();
537+
538+
let is_target_ext = target_meta.contains_key(arrow_schema::extension::EXTENSION_TYPE_NAME_KEY);
539+
let is_input_ext = input_meta.contains_key(arrow_schema::extension::EXTENSION_TYPE_NAME_KEY);
540+
541+
if !is_target_ext && !is_input_ext {
542+
return if input_field.data_type() == field.data_type() {
543+
Ok(col(input_field.name()).alias(field.name()))
544+
} else {
545+
Ok(col(input_field.name())
546+
.cast_to(field.data_type(), input_schema)?
547+
.alias(field.name()))
548+
};
549+
}
550+
551+
let metadata_matches = check_metadata_with_storage_equal(
552+
(input_field.data_type(), Some(input_meta)),
553+
(field.data_type(), Some(target_meta)),
554+
"input", "target"
555+
).is_ok();
556+
557+
if metadata_matches {
558+
Ok(col(input_field.name()).alias(field.name()))
538559
} else {
539-
Some(metadata)
540-
};
541-
Expr::Cast(Cast::new_from_field(
542-
Box::new(col(input_field.name())),
543-
Arc::clone(field),
544-
))
545-
.alias_with_metadata(field.name(), alias_metadata)
546-
})
547-
.collect::<Vec<_>>();
548-
549-
LogicalPlanBuilder::from(plan.clone())
560+
Ok(Expr::Cast(datafusion_expr::Cast::new_from_field(
561+
Box::new(col(input_field.name())),
562+
Arc::clone(field),
563+
)).alias(field.name()))
564+
}
565+
}).collect::<Result<Vec<_>>>()?;
566+
567+
LogicalPlanBuilder::from(plan.clone())
550568
.project(project_exprs)?
551569
.build()?
552570
} else {

0 commit comments

Comments
 (0)