Skip to content

Parquet stats pruning: IS NULL/IS NOT NULL Simplification for Literal Arguments #19092

Open

Description

@adriangb

This is a dump of a conversation with Claude Code

Problem

When a predicate like b IS NOT NULL OR a = 24 is rewritten for a file schema where column b doesn't exist, the b IS NOT NULL becomes IS NOT NULL(NULL literal). This should simplify to false, allowing the predicate to become a = 24 for proper row group pruning.

Currently:

  1. PhysicalExprSimplifier doesn't simplify IS NULL(literal) or IS NOT NULL(literal)
  2. build_predicate_expression treats IS NOT NULL(literal) as "unhandled" → returns true
  3. Result: true OR (pruning_expr) = true → no pruning possible

Solution

Implement both fixes so that IS NOT NULL(lit(1)) and similar expressions benefit from simplification at multiple levels.


Change 1: Add IS NULL/IS NOT NULL Simplification to PhysicalExprSimplifier

File: datafusion/physical-expr/src/simplifier/mod.rs

Modifications:

  1. Add new module import:

    pub mod is_null;
  2. Update f_up method to chain the new simplification:

    fn f_up(&mut self, node: Self::Node) -> Result<Transformed<Self::Node>> {
        let rewritten = simplify_not_expr(&node, self.schema)?
            .transform_data(|node| is_null::simplify_is_null_expr(&node))?
            .transform_data(|node| {
                unwrap_cast::unwrap_cast_in_comparison(node, self.schema)
            })?;
        Ok(rewritten)
    }

New File: datafusion/physical-expr/src/simplifier/is_null.rs

Create a new module following the pattern of not.rs:

//! Simplify IS NULL and IS NOT NULL expressions on literals
//!
//! - IS NULL(literal) -> true/false based on literal.value().is_null()
//! - IS NOT NULL(literal) -> true/false based on !literal.value().is_null()

use std::sync::Arc;
use datafusion_common::{tree_node::Transformed, Result, ScalarValue};
use crate::expressions::{lit, IsNullExpr, IsNotNullExpr, Literal};
use crate::PhysicalExpr;

pub fn simplify_is_null_expr(
    expr: &Arc<dyn PhysicalExpr>,
) -> Result<Transformed<Arc<dyn PhysicalExpr>>> {
    // Handle IS NULL(literal)
    if let Some(is_null_expr) = expr.as_any().downcast_ref::<IsNullExpr>() {
        if let Some(literal) = is_null_expr.arg().as_any().downcast_ref::<Literal>() {
            let result = literal.value().is_null();
            return Ok(Transformed::yes(lit(ScalarValue::Boolean(Some(result)))));
        }
    }

    // Handle IS NOT NULL(literal)
    if let Some(is_not_null_expr) = expr.as_any().downcast_ref::<IsNotNullExpr>() {
        if let Some(literal) = is_not_null_expr.arg().as_any().downcast_ref::<Literal>() {
            let result = !literal.value().is_null();
            return Ok(Transformed::yes(lit(ScalarValue::Boolean(Some(result)))));
        }
    }

    Ok(Transformed::no(Arc::clone(expr)))
}

Tests to Add (in mod.rs)

#[test]
fn test_simplify_is_null_literal() -> Result<()> {
    let schema = test_schema();
    let mut simplifier = PhysicalExprSimplifier::new(&schema);

    // IS NULL(1) -> false
    let is_null_expr = Arc::new(IsNullExpr::new(lit(ScalarValue::Int32(Some(1)))));
    let result = simplifier.simplify(is_null_expr)?;
    assert_eq!(as_literal(&result).value(), &ScalarValue::Boolean(Some(false)));

    // IS NULL(NULL) -> true
    let is_null_expr = Arc::new(IsNullExpr::new(lit(ScalarValue::Int32(None))));
    let result = simplifier.simplify(is_null_expr)?;
    assert_eq!(as_literal(&result).value(), &ScalarValue::Boolean(Some(true)));

    Ok(())
}

#[test]
fn test_simplify_is_not_null_literal() -> Result<()> {
    let schema = test_schema();
    let mut simplifier = PhysicalExprSimplifier::new(&schema);

    // IS NOT NULL(1) -> true
    let expr = Arc::new(IsNotNullExpr::new(lit(ScalarValue::Int32(Some(1)))));
    let result = simplifier.simplify(expr)?;
    assert_eq!(as_literal(&result).value(), &ScalarValue::Boolean(Some(true)));

    // IS NOT NULL(NULL) -> false
    let expr = Arc::new(IsNotNullExpr::new(lit(ScalarValue::Int32(None))));
    let result = simplifier.simplify(expr)?;
    assert_eq!(as_literal(&result).value(), &ScalarValue::Boolean(Some(false)));

    Ok(())
}

Change 2: Handle Literal Arguments in build_predicate_expression

File: datafusion/pruning/src/pruning_predicate.rs

Modify build_predicate_expression function (~line 1415-1427):

Before:

if let Some(is_null) = expr_any.downcast_ref::<phys_expr::IsNullExpr>() {
    return build_is_null_column_expr(is_null.arg(), schema, required_columns, false)
        .unwrap_or_else(|| unhandled_hook.handle(expr));
}
if let Some(is_not_null) = expr_any.downcast_ref::<phys_expr::IsNotNullExpr>() {
    return build_is_null_column_expr(
        is_not_null.arg(),
        schema,
        required_columns,
        true,
    )
    .unwrap_or_else(|| unhandled_hook.handle(expr));
}

After:

if let Some(is_null) = expr_any.downcast_ref::<phys_expr::IsNullExpr>() {
    // If argument is a literal, evaluate directly
    if let Some(literal) = is_null.arg().as_any().downcast_ref::<phys_expr::Literal>() {
        let result = literal.value().is_null();
        return Arc::new(phys_expr::Literal::new(ScalarValue::Boolean(Some(result))));
    }
    return build_is_null_column_expr(is_null.arg(), schema, required_columns, false)
        .unwrap_or_else(|| unhandled_hook.handle(expr));
}
if let Some(is_not_null) = expr_any.downcast_ref::<phys_expr::IsNotNullExpr>() {
    // If argument is a literal, evaluate directly
    if let Some(literal) = is_not_null.arg().as_any().downcast_ref::<phys_expr::Literal>() {
        let result = !literal.value().is_null();
        return Arc::new(phys_expr::Literal::new(ScalarValue::Boolean(Some(result))));
    }
    return build_is_null_column_expr(
        is_not_null.arg(),
        schema,
        required_columns,
        true,
    )
    .unwrap_or_else(|| unhandled_hook.handle(expr));
}

Files to Modify

  1. datafusion/physical-expr/src/simplifier/mod.rs - Add is_null module import and chain simplification
  2. datafusion/physical-expr/src/simplifier/is_null.rs - New file for IS NULL/IS NOT NULL simplification
  3. datafusion/pruning/src/pruning_predicate.rs - Handle literal arguments in build_predicate_expression

Testing

  1. Run the original failing test:

    cargo test --package datafusion --test core_integration --all-features -- schema_adapter::schema_adapter_integration_tests::test_parquet_missing_column --exact --nocapture
    
  2. Run simplifier tests:

    cargo test --package datafusion-physical-expr --lib simplifier
    
  3. Run pruning predicate tests:

    cargo test --package datafusion-pruning --lib
    

Activity

  1. xudong963 commented on Dec 5, 2025

    @xudong963
    Member

    When a predicate like b IS NOT NULL OR a = 24 is rewritten for a file schema where column b doesn't exist

    Why can b be allowed not to exist? If so, there should be a early error report for the predicate?

  2. adriangb commented on Dec 5, 2025

    @adriangb
    ContributorAuthor

    Isn't that just normal schema evolution? A column b was added so older files won't have it.

  3. shashidhar-bm commented on Dec 20, 2025

    @shashidhar-bm
    Contributor

    take

  4. pepijnve commented on Dec 21, 2025

    @pepijnve
    Contributor
    • IS NULL(literal) -> true/false based on literal.value().is_null()
    • IS NOT NULL(literal) -> true/false based on !literal.value().is_null()

    @adriangb this seems like something simplify_const_expr should already be able to do. Any idea why it isn't in your case? When I just tested the proposed unit tests on main, they already pass.

  5. adriangb commented on Dec 21, 2025

    @adriangb
    ContributorAuthor

    This is after per file expression rewrites eg if the column is missing

  6. pepijnve commented on Dec 21, 2025

    @pepijnve
    Contributor

    This is after per file expression rewrites eg if the column is missing

    Wouldn’t another round of the generic simplify after that transform has happened also do the trick? Or is there too much overhead in doing that? An specialisation of const evaluation just for is null seems a bit ad hoc.

  7. adriangb commented on Dec 21, 2025

    @adriangb
    ContributorAuthor

    Yes a generic round would work but afaik we don’t have this simplification at the physical expression simplification layer

  8. adriangb commented on Dec 21, 2025

    @adriangb
    ContributorAuthor

    (Maybe I’m wrong about that but that was why I opened the issue)

  9. pepijnve commented on Dec 21, 2025

    @pepijnve
    Contributor

    The proposed unit tests already pass, so it's there to some extent at least.

  10. adriangb commented on Dec 21, 2025

    @adriangb
    ContributorAuthor

    Let me confirm and we can close the issue 🤣

  11. pepijnve commented on Dec 21, 2025

    @pepijnve
    Contributor

    I did a quick, slightly more complex test. NULL IS NOT NULL OR a = 24 simplifies to false OR a = 24, but that isn't simplified further to a = 24. Could that redundant or false get in the way of pruning?

  12. shashidhar-bm commented on Dec 21, 2025

    @shashidhar-bm
    Contributor

    I did a quick, slightly more complex test. NULL IS NOT NULL OR a = 24 simplifies to false OR a = 24, but that isn't simplified further to a = 24. Could that redundant or false get in the way of pruning?

    I tried a test to verify the value of tf.transformed for the expression NULL IS NOT NULL OR a = 24, and it returned false. When tf.transformed == false, simplification is skipped in the original code (line 479: expr = tf.data),

  13. adriangb commented on Dec 21, 2025

    @adriangb
    ContributorAuthor

    Ok I've done a bit of digging.

    Firstly I apologize for making a confusing issue. I ran into this while working on a bigger change and wanted to document it to tackle later but evidently even though I included a lot of detail I didn't include the right detail.

    I'm still not sure what the original e2e reproduction I had for this was, but since I opened this on December 4th it's possible it was fixed by #19130 (const simplifier) which was merged a couple days later, or by #19111 merged after that and changed the structure that @ShashidharM0118 points out in #19434 (comment) would have also caused this issue.

    I did find something interesting: #19136 introduced a new opportunity for optimization via simplification. If we have a constant column say a = 2 and the predicate a is not null we replace that to 2 is not null. That will get simplified later for the scan but as far as I can tell it does not get simplified before being fed into FilePruner. FilePruner also does not simplify the output of any dynamic filters. So I think if we added a simplifier pass in FilePruner we'd get some extra pruning in the case of constant_col is null.

  14. adriangb commented on Dec 21, 2025

    @adriangb
    ContributorAuthor

    Here's the reproducer for that:

    CREATE EXTERNAL TABLE t (
        x INT
    ) 
    STORED AS PARQUET
    LOCATION '/tmp/test-data/';
    
    INSERT INTO t VALUES (1), (1);
    
    EXPLAIN ANALYZE
    SELECT COUNT(*) FROM t WHERE x IS NULL;

    As you can see if has files_ranges_pruned_statistics=1 total → 1 matched. Adding a simplifier for FilePruner I see files_ranges_pruned_statistics=1 total → 0 matched.

    The diff is:

    diff --git a/datafusion/datasource-parquet/src/opener.rs b/datafusion/datasource-parquet/src/opener.rs
    index bea970f14..2f1dd192f 100644
    --- a/datafusion/datasource-parquet/src/opener.rs
    +++ b/datafusion/datasource-parquet/src/opener.rs
    @@ -303,9 +303,17 @@ impl FileOpener for ParquetOpener {
                         // 10` could hit this condition.
                         is_dynamic_physical_expr(p) || partitioned_file.has_statistics()
                     })
    -                .and_then(|p| {
    +                .cloned()
    +                .and_then(|mut p| {
    +                    // If we made any replacements of columns with literals run a simplifier pass.
    +                    // This handles cases like `literal_col is not null` -> `true`.
    +                    if !literal_columns.is_empty() {
    +                        let simplifier =
    +                            PhysicalExprSimplifier::new(&logical_file_schema);
    +                        p = simplifier.simplify(p).ok()?;
    +                    }
                         FilePruner::try_new(
    -                        Arc::clone(p),
    +                        p,
                             &logical_file_schema,
                             &partitioned_file,
                             predicate_creation_errors.clone(),
  15. pepijnve commented on Dec 21, 2025

    @pepijnve
    Contributor

    I was tinkering a bit out of curiosity. With the addition of the simplification in the collapsed details to the physical expression simplifier

    Details
    pub fn simplify_and_or_expr(
        expr: &Arc<dyn PhysicalExpr>,
    ) -> Result<Transformed<Arc<dyn PhysicalExpr>>> {
        // Check if this is a NOT expression
        let binary_expr = match expr.as_any().downcast_ref::<BinaryExpr>() {
            Some(b) if b.op().eq(&Operator::And) => {
                match (as_bool(b.left()), as_bool(b.right())) {
                    (Some(false), _) => Some(Arc::clone(b.left())),
                    (_, Some(false)) => Some(Arc::clone(b.right())),
                    (Some(true), _) => Some(Arc::clone(b.right())),
                    (_, Some(true)) => Some(Arc::clone(b.left())),
                    _ => None,
                }
            },
            Some(b) if b.op().eq(&Operator::Or) => {
                match (as_bool(b.left()), as_bool(b.right())) {
                    (Some(true), _) => Some(Arc::clone(b.left())),
                    (_, Some(true)) => Some(Arc::clone(b.right())),
                    (Some(false), _) => Some(Arc::clone(b.right())),
                    (_, Some(false)) => Some(Arc::clone(b.left())),
                    _ => None,
                }
            },
            _ => None,
        };
    
        Ok(match binary_expr {
            None => Transformed::no(Arc::clone(expr)),
            Some(e) => Transformed::yes(e),
        })
    }
    
    fn as_bool(expr: &Arc<dyn PhysicalExpr>) -> Option<bool> {
        match expr.as_any().downcast_ref::<Literal>() {
            Some(l) => {
                match l.value() {
                    ScalarValue::Boolean(b) => b.clone(),
                    _ => None,
                }
            },
            _ => None,
        }
    }
    

    this test case passes, so NULL IS NOT NULL or c1 = 24 gets simplified to c1 = 24. Not sure how useful this is, but seems like it might be nice to strip away cruft after const evaluation.

    #[test]
    fn test_simplify_null_is_not_null_or() -> Result<()> {
        let schema = test_schema();
        let mut simplifier = PhysicalExprSimplifier::new(&schema);
    
        let c1_is_24 = binary(
            col("c1", &schema)?,
            Operator::Eq,
            lit(ScalarValue::Int32(Some(24))),
            &schema,
        )?;
        let expr = binary(
            is_not_null(lit(ScalarValue::Int32(None)))?,
            Operator::Or,
            Arc::clone(&c1_is_24),
            &schema,
        )?;
        let result = simplifier.simplify(expr)?;
        assert_eq!(&result, &c1_is_24);
    
        Ok(())
    }
    
  16. adriangb commented on Dec 21, 2025

    @adriangb
    ContributorAuthor

    Hmm that's interesting, and I don't see any reason not to do it, but I don't think that's necessary, PruningPredicate should be able to handle that case.

  17. pepijnve commented on Dec 22, 2025

    @pepijnve
    Contributor

    I'll make a separate PR for that. I had a quick look at PruningPredicate and it indeed already has more or less the same code to simplify and true and or false. I think it's a cleaner separation of concerns if the simplifier takes care of this instead.

    In other words, in PruningPredicate::try_new perhaps it would be preferable to always run the incoming expression through the simplifier so that build_predicate_expression only has to deal with the simplified versions instead of implementing ad hoc inline simplification.

  18. shashidhar-bm commented on Dec 22, 2025

    @shashidhar-bm
    Contributor

    In other words, in PruningPredicate::try_new perhaps it would be preferable to always run the incoming expression through the simplifier so that build_predicate_expression only has to deal with the simplified versions instead of implementing ad hoc inline simplification.

    i tried the exact same thing in #19434, can you please check it once?

  19. pepijnve commented on Dec 22, 2025

    @pepijnve
    Contributor

    @ShashidharM0118 that looks ok to me. I'm just sharing thoughts from the sidelines though. @adriangb is probably best placed to check.

    The PR I had in mind would be complementary to this: pepijnve@2e160e0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions