Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

my-vegetable-has-exploded
Copy link
Contributor

Which issue does this PR close?

Closes #12499

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added the core Core DataFusion crate label Sep 17, 2024
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you so much @my-vegetable-has-exploded -- this looks really great. I verified the test coverage. 🚀

Comment on lines +267 to +272
ScalarValue::Utf8(Some(v)) | ScalarValue::Utf8View(Some(v)) => {
sbbf.check(&v.as_str())
}
ScalarValue::Binary(Some(v)) | ScalarValue::BinaryView(Some(v)) => {
sbbf.check(v)
}
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

@alamb alamb merged commit d3fb595 into apache:main Sep 18, 2024
24 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
core Core DataFusion crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support applying parquet bloom filters to StringView columns
2 participants