Skip to content
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.

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

feat:Support applying parquet bloom filters to StringView columns #12503

Merged
merged 1 commit into from
Sep 18, 2024
Merged
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,12 @@ impl PruningStatistics for BloomFilterStatistics {
.iter()
.map(|value| {
match value {
ScalarValue::Utf8(Some(v)) => sbbf.check(&v.as_str()),
ScalarValue::Binary(Some(v)) => sbbf.check(v),
ScalarValue::Utf8(Some(v)) | ScalarValue::Utf8View(Some(v)) => {
sbbf.check(&v.as_str())
}
ScalarValue::Binary(Some(v)) | ScalarValue::BinaryView(Some(v)) => {
sbbf.check(v)
}
Comment on lines +267 to +272
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I double checked that the tests cover this code like this:

--- a/datafusion/core/src/datasource/physical_plan/parquet/row_group_filter.rs
+++ b/datafusion/core/src/datasource/physical_plan/parquet/row_group_filter.rs
@@ -264,9 +264,12 @@ impl PruningStatistics for BloomFilterStatistics {
             .iter()
             .map(|value| {
                 match value {
-                    ScalarValue::Utf8(Some(v)) | ScalarValue::Utf8View(Some(v)) => {
+                    ScalarValue::Utf8(Some(v))  => {
                         sbbf.check(&v.as_str())
                     }
+                    ScalarValue::Utf8View(Some(v)) => {
+                        panic!("String view bloom filter not implemented yet");
+                    }
                     ScalarValue::Binary(Some(v)) | ScalarValue::BinaryView(Some(v)) => {
                         sbbf.check(v)
                     }
@@ -1439,6 +1442,7 @@ mod tests {
             }
         }

+
cargo test -p datafusion -- row_group_filter

running 21 tests
test datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_multiple_expr ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::row_group_pruning_predicate_missing_stats ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_simple_expr ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_sql_in ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::row_group_pruning_predicate_file_schema ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::row_group_pruning_predicate_simple_expr ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::row_group_pruning_predicate_decimal_type2 ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::row_group_pruning_predicate_decimal_type ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::row_group_pruning_predicate_decimal_type3 ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::row_group_pruning_predicate_null_expr ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::row_group_pruning_predicate_partial_expr ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::row_group_pruning_predicate_decimal_type4 ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::row_group_pruning_predicate_decimal_type5 ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::row_group_pruning_predicate_eq_null_expr ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_with_exists_2_values ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_with_exists_3_values ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_with_exists_value ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_with_or_not_eq ... ok
test datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_with_exists_3_values_view ... FAILED
test datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_multiple_expr_view ... FAILED
test datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_without_bloom_filter ... ok

failures:

---- datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_with_exists_3_values_view stdout ----
thread 'datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_with_exists_3_values_view' panicked at datafusion/core/src/datasource/physical_plan/parquet/row_group_filter.rs:271:25:
String bloom filter not implemented yet

---- datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_multiple_expr_view stdout ----
thread 'datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_multiple_expr_view' panicked at datafusion/core/src/datasource/physical_plan/parquet/row_group_filter.rs:271:25:
String bloom filter not implemented yet
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_multiple_expr_view
    datasource::physical_plan::parquet::row_group_filter::tests::test_row_group_bloom_filter_pruning_predicate_with_exists_3_values_view

ScalarValue::FixedSizeBinary(_size, Some(v)) => sbbf.check(v),
ScalarValue::Boolean(Some(v)) => sbbf.check(v),
ScalarValue::Float64(Some(v)) => sbbf.check(v),
Expand Down Expand Up @@ -1219,6 +1223,25 @@ mod tests {
.await
}

#[tokio::test]
async fn test_row_group_bloom_filter_pruning_predicate_multiple_expr_view() {
BloomFilterTest::new_data_index_bloom_encoding_stats()
.with_expect_all_pruned()
// generate pruning predicate `(String = "Hello_Not_exists" OR String = "Hello_Not_exists2")`
.run(
lit("1").eq(lit("1")).and(
col(r#""String""#)
.eq(Expr::Literal(ScalarValue::Utf8View(Some(String::from(
"Hello_Not_Exists",
)))))
.or(col(r#""String""#).eq(Expr::Literal(ScalarValue::Utf8View(
Some(String::from("Hello_Not_Exists2")),
)))),
),
)
.await
}

#[tokio::test]
async fn test_row_group_bloom_filter_pruning_predicate_sql_in() {
// load parquet file
Expand Down Expand Up @@ -1286,6 +1309,26 @@ mod tests {
.await
}

#[tokio::test]
async fn test_row_group_bloom_filter_pruning_predicate_with_exists_3_values_view() {
BloomFilterTest::new_data_index_bloom_encoding_stats()
.with_expect_none_pruned()
// generate pruning predicate `(String = "Hello") OR (String = "the quick") OR (String = "are you")`
.run(
col(r#""String""#)
.eq(Expr::Literal(ScalarValue::Utf8View(Some(String::from(
"Hello",
)))))
.or(col(r#""String""#).eq(Expr::Literal(ScalarValue::Utf8View(
Some(String::from("the quick")),
))))
.or(col(r#""String""#).eq(Expr::Literal(ScalarValue::Utf8View(
Some(String::from("are you")),
)))),
)
.await
}

#[tokio::test]
async fn test_row_group_bloom_filter_pruning_predicate_with_or_not_eq() {
BloomFilterTest::new_data_index_bloom_encoding_stats()
Expand Down