Skip to content

parquet RowGroup pruning for Dictionary(Decimal) type incorrect #13821

Closed

Description

@korowa

Describe the bug

Parquet RowGroup pruning by statistics works incorrectly for Dictionary(Decimal) type.

To Reproduce

use arrow;
use arrow::array::{ArrayRef, Decimal128Array, DictionaryArray, Int32Array, RecordBatch};
use datafusion::error::Result;
use datafusion::prelude::*;
use parquet::arrow::ArrowWriter;
use parquet::file::properties::{EnabledStatistics, WriterProperties};
use std::fs::File;
use std::sync::Arc;

#[tokio::main]
async fn main() -> Result<()> {
    // Prepare record batch
    let array_values = Decimal128Array::from_iter_values(vec![10, 20, 30])
        .with_precision_and_scale(4, 1)?;
    let array_keys = Int32Array::from_iter_values(vec![0, 1, 2]);
    let array = Arc::new(DictionaryArray::new(array_keys, Arc::new(array_values)));
    let batch = RecordBatch::try_from_iter(vec![("col", array as ArrayRef)])?;

    // Write batch to parquet
    let file_path = "dictionary_decimal.parquet";

    let file = File::create(file_path)?;
    let properties = WriterProperties::builder()
        .set_statistics_enabled(EnabledStatistics::Chunk)
        .set_bloom_filter_enabled(true)
        .build();
    let mut writer = ArrowWriter::try_new(file, batch.schema(), Some(properties))?;

    writer.write(&batch)?;
    writer.flush()?;
    writer.close()?;

    // Prepare context
    let config = SessionConfig::default()
        .with_parquet_bloom_filter_pruning(true)
        .with_collect_statistics(true);
    let ctx = SessionContext::new_with_config(config);

    ctx.register_parquet("t", file_path, ParquetReadOptions::default())
        .await?;

    // In case pruning predicate not created (due to cast), there is a record in resultset
    ctx.sql("select * from t where col = 1")
        .await?
        .show()
        .await?;

    println!();

    // In case of triggered RowGroup pruning -- the only RowGroup eliminated while pruning by statistics
    ctx.sql("select * from t where col = cast(1 as decimal(4, 1))")
        .await?
        .show()
        .await?;

    Ok(())
}

Expected behavior

Results from both queries from the script above should match

Additional context

The problem also happens with bloom filters (if enable them in pattern matching expressions in prune_by_bloom_filters), so there is a chance that ArrowWriter produces incorrect metadata (statistics / bloom filters).

+ after adding larger value to the batch (like 100 as i128 which is 10.0 when casted to decimal(4, 1)), RG is not pruned, so perhaps something like * pow(10, scale) lost while writing statistics / calculating filters.

Activity

  1. changed the title [-]parquet RowGroup pruning for `Dictionary(Decimal)` type[/-] [+]parquet RowGroup pruning for `Dictionary(Decimal)` type incorreft[/+] on Dec 17, 2024
  2. changed the title [-]parquet RowGroup pruning for `Dictionary(Decimal)` type incorreft[/-] [+]parquet RowGroup pruning for `Dictionary(Decimal)` type incorrect[/+] on Dec 17, 2024
  3. kosiew commented on Dec 20, 2024

    @kosiew
    Contributor

    Research findings:

    with parquest_pruning turned off

        let config = SessionConfig::default()
            .with_parquet_bloom_filter_pruning(true)
            .with_parquet_pruning(false) 
            .with_collect_statistics(true);
    

    both queries return the same result:

    +-----+
    | col |
    +-----+
    | 1.0 |
    +-----+

  4. lichuang commented on Dec 22, 2024

    @lichuang

    take

  5. lichuang commented on Jan 8, 2025

    @lichuang

    after i dig into this bug, here is the report:

    cast(1 as decimal(4, 1)) will get a value Decimal128(Some(10),4,1), and eq operator transfer to two operation:

    • compare with the min value, which is PrimitiveArray<Decimal128(4, 1)>[1]} lteq Decimal128(Some(10),4,1), and return true;
    • compare with the max value, which is Decimal128(Some(10),4,1) lteq PrimitiveArray<Decimal128(4, 1)>[4]}, and return false;

    so select * from t where col = cast(1 as decimal(4, 1)) cannot filter any columns. It seems that col = 1 and col = cast(1 as decimal(4, 1)) has not same semantic.

  6. kosiew commented on Jan 13, 2025

    @kosiew
    Contributor

    Would it be a good approach to fix this by coercing both sides of BinaryExpr like
    col = CAST(1 AS DECIMAL(4, 1)))
    to a common type?

    Here's a conceptual fix:

    impl BinaryExpr {
        pub fn new(left: Box<Expr>, op: Operator, right: Box<Expr>) -> Self {
            // Apply type coercion to ensure both sides have the same type
            let (coerced_left, coerced_right) = coerce_types_if_needed(*left, *right);
            Self {
                left: Box::new(coerced_left),
                op,
                right: Box::new(coerced_right),
            }
        }
    }
    
  7. korowa commented on Jan 16, 2025

    @korowa
    ContributorAuthor

    @kosiew the root cause of the issue is how arrow writer handles data for Dictionary(Decimal), and I suppose it'll mostly be fixed by apache/arrow-rs#6987

    Regarding RG pruning not being enabled in case literal and column have different datatypes -- I think adding some safe casting to enable it would be a useful feature. For this, perhaps, PruningPredicate and its interactions with ParquetExec will be a good place to start dig in.

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions