Should PruningPredicate coerce?

Describe the bug

Currently when you pass a pruning predicate where the predicate has a different type as the targeted column it will not prune it, even though in theory the value is castable to the target column type.

The predicate looks like this: "month_id = '202502' AND date_id = '20250226'"

However the columns are int column not utf8. So in theory these string values can be casted to int but I don't believe this is happening. Is this something that should be added

This is our rust code btw

// delta-rs/crates/core/src/delta_datafusion/mod.rs
let pruning_predicate = PruningPredicate::try_new(predicate.clone(), logical_schema.clone())?;
let files_to_prune = pruning_predicate.prune(self.snapshot)?;

To Reproduce

Related delta-rs issue: delta-io/delta-rs#3278 (comment)

Expected behavior

Allow literal value coercion during pruning

Additional context

No response

I think the conclusion is that PruningPredicaate expects a properly coerced expression already

You can use SessionContext::create_physical_expr to automatically perform coercsion as shown in this example

datafusion/datafusion-examples/examples/expr_api.rs

Lines 540 to 543 in 5e49094

    
           // 1. Type coercion with `SessionContext::create_physical_expr` which implicitly applies type coercion before constructing the physical expr. 
        
           let physical_expr = 
        
               SessionContext::new().create_physical_expr(expr.clone(), &df_schema)?; 
        
           assert!(physical_expr.evaluate(&batch).is_ok());

There are a bunch of other examples there about how to apply coercion

@alamb

@alamb this is actually already done like that, but it still doesn't prune properly:

.map(|expr| context.create_physical_expr(expr, &df_schema).unwrap());

https://github.com/delta-io/delta-rs/blob/main/crates%2Fcore%2Fsrc%2Fdelta_datafusion%2Fmod.rs#L547-L550

🤔 What are the coerced expressions?

If it is like this:

cast(month_id, 'utf8') = '202502'

That is not going to prune because the cast is happening on month_id

The predicate needs to be

month_id = cast('202502')

This is done in this analyzer pass: https://github.com/apache/datafusion/blob/main/datafusion/optimizer/src/unwrap_cast_in_comparison.rs

Maybe we should do the same thing in create_physical_expr too 🤔

It's like cast(month_id, 'utf8') = '202502', see below:

[crates/core/src/delta_datafusion/mod.rs:551:9] self.filter.clone() = Some(
    BinaryExpr(
        BinaryExpr {
            left: BinaryExpr(
                BinaryExpr {
                    left: Cast(
                        Cast {
                            expr: Column(
                                Column {
                                    relation: None,
                                    name: "month_id",
                                },
                            ),
                            data_type: Utf8,
                        },
                    ),
                    op: Eq,
                    right: Literal(
                        Utf8("202502"),
                    ),
                },
            ),
            op: And,
            right: BinaryExpr(
                BinaryExpr {
                    left: Cast(
                        Cast {
                            expr: Column(
                                Column {
                                    relation: None,
                                    name: "date_id",
                                },
                            ),
                            data_type: Utf8,
                        },
                    ),
                    op: Eq,
                    right: Literal(
                        Utf8("20250226"),
                    ),
                },
            ),
        },
    ),
)

It's like cast(month_id, 'utf8') = '202502', see below:

So it seems like it would be a valuable thing to apply the type coercion rewriter in the expr simplifier then.

datafusion/datafusion/optimizer/src/unwrap_cast_in_comparison.rs

Lines 115 to 125 in 57a1221

    
               let mut expr_rewriter = UnwrapCastExprRewriter { 
        
                   schema: Arc::new(schema), 
        
               }; 
        
               let name_preserver = NamePreserver::new(&plan); 
        
               plan.map_expressions(|expr| { 
        
                   let original_name = name_preserver.save(&expr); 
        
                   expr.rewrite(&mut expr_rewriter) 
        
                       .map(|transformed| transformed.update_data(|e| original_name.restore(e))) 
        
               }) 
        
           }

I'll file a ticket.

Filed

SessionContext::create_physical_expr does not unwrap casts (and thus is not always optimal) #14987

@alamb

@alamb thanks! 🙏

So I was writing some tests for Move UnwrapCastInComparison into Simplifier #15012

I found that a predicate like this:

month_id = '202502'

Is actually coerced so that it does string comparison rather than integer comparison (note the predicate in the plan below is CAST(foo.i AS Utf8) = Utf8("2024") rather than `foo.i = CAST("2024" as Utf8)

DataFusion CLI v46.0.0
> create table foo(i int) as values (1), (2), (3);
0 row(s) fetched.
Elapsed 0.033 seconds.

> explain select i from foo where i = '2024';
+---------------+-------------------------------------------------------+
| plan_type     | plan                                                  |
+---------------+-------------------------------------------------------+
| logical_plan  | Filter: CAST(foo.i AS Utf8) = Utf8("2024")            |
|               |   TableScan: foo projection=[i]                       |
| physical_plan | CoalesceBatchesExec: target_batch_size=8192           |
|               |   FilterExec: CAST(i@0 AS Utf8) = 2024                |
|               |     DataSourceExec: partitions=1, partition_sizes=[1] |
|               |                                                       |
+---------------+-------------------------------------------------------+

So in other words, I think #15012 is necessary, but not sufficient, to fix this issue

Update: PR to add some tests:

https://github.com/apache/datafusion/pull/15034/files

Also filed a ticket for unwrapping that particular comparison expression

Avoid casting columns when comparing ints and strings #15035

Navigation Menu

Repository navigation

Should `PruningPredicate` coerce? #14944

Describe the bug

To Reproduce

Expected behavior

Additional context

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	// 1. Type coercion with `SessionContext::create_physical_expr` which implicitly applies type coercion before constructing the physical expr.
	let physical_expr =
	SessionContext::new().create_physical_expr(expr.clone(), &df_schema)?;
	assert!(physical_expr.evaluate(&batch).is_ok());

	let mut expr_rewriter = UnwrapCastExprRewriter {
	schema: Arc::new(schema),
	};

	let name_preserver = NamePreserver::new(&plan);
	plan.map_expressions(\|expr\| {
	let original_name = name_preserver.save(&expr);
	expr.rewrite(&mut expr_rewriter)
	.map(\|transformed\| transformed.update_data(\|e\| original_name.restore(e)))
	})
	}

Should PruningPredicate coerce? #14944

Description

Describe the bug

To Reproduce

Expected behavior

Additional context

Activity

alamb commented on Feb 28, 2025

ion-elgreco commented on Feb 28, 2025

alamb commented on Feb 28, 2025

ion-elgreco commented on Mar 1, 2025

alamb commented on Mar 3, 2025

alamb commented on Mar 3, 2025

ion-elgreco commented on Mar 3, 2025

alamb commented on Mar 5, 2025

alamb commented on Mar 5, 2025

alamb commented on Mar 5, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Should `PruningPredicate` coerce? #14944