alamb commented on code in PR #8930:
URL: https://github.com/apache/arrow-datafusion/pull/8930#discussion_r1463125656
##########
datafusion/core/src/datasource/physical_plan/parquet/row_groups.rs:
##########
@@ -209,16 +214,61 @@ impl PruningStatistics for BloomFilterStatistics {
let known_not_present = values
.iter()
- .map(|value| match value {
- ScalarValue::Utf8(Some(v)) => sbbf.check(&v.as_str()),
- ScalarValue::Boolean(Some(v)) => sbbf.check(v),
- ScalarValue::Float64(Some(v)) => sbbf.check(v),
- ScalarValue::Float32(Some(v)) => sbbf.check(v),
- ScalarValue::Int64(Some(v)) => sbbf.check(v),
- ScalarValue::Int32(Some(v)) => sbbf.check(v),
- ScalarValue::Int16(Some(v)) => sbbf.check(v),
- ScalarValue::Int8(Some(v)) => sbbf.check(v),
- _ => true,
+ .map(|value| {
+ match value {
+ ScalarValue::Utf8(Some(v)) => sbbf.check(&v.as_str()),
+ ScalarValue::Boolean(Some(v)) => sbbf.check(v),
+ ScalarValue::Float64(Some(v)) => sbbf.check(v),
+ ScalarValue::Float32(Some(v)) => sbbf.check(v),
+ ScalarValue::Int64(Some(v)) => sbbf.check(v),
+ ScalarValue::Int32(Some(v)) => sbbf.check(v),
+ ScalarValue::Int16(Some(v)) => sbbf.check(v),
+ ScalarValue::Int8(Some(v)) => sbbf.check(v),
+ ScalarValue::Decimal128(Some(v), p, s) => match
parquet_type {
+ Type::INT32 => {
+
//https://github.com/apache/parquet-format/blob/eb4b31c1d64a01088d02a2f9aefc6c17c54cc6fc/Encodings.md?plain=1#L35-L42
+ // All physical type are little-endian
+ if *p > 9 {
+ //DECIMAL can be used to annotate the
following types:
+ //
+ // int32: for 1 <= precision <= 9
+ // int64: for 1 <= precision <= 18
+ return true;
+ }
+ let b = (*v as i32).to_le_bytes();
+ let decimal = Decimal::Int32 {
Review Comment:
It might be good to add a note here about the upstream arrow ticket
```suggestion
// Use Decimal constructor after
https://github.com/apache/arrow-rs/issues/5325
let decimal = Decimal::Int32 {
```
##########
datafusion/core/tests/parquet/mod.rs:
##########
@@ -549,6 +552,22 @@ fn create_data_batch(scenario: Scenario) ->
Vec<RecordBatch> {
make_decimal_batch(vec![2000, 3000, 3000, 4000, 6000], 9, 2),
]
}
+ Scenario::DecimalBloomFilterInt32 => {
+ // decimal record batch
+ vec![
+ make_decimal_batch(vec![100, 200, 300, 400, 500], 6, 2),
+ make_decimal_batch(vec![100, 200, 300, 400, 600], 6, 2),
+ make_decimal_batch(vec![100, 200, 300, 400, 600], 6, 2),
+ ]
+ }
+ Scenario::DecimalBloomFilterInt64 => {
+ // decimal record batch
+ vec![
+ make_decimal_batch(vec![100, 200, 300, 400, 500], 9, 2),
Review Comment:
I recommend using values here that are greater than 4 bytes (to distinguish
between the code in i32 and i64)
Maybe something like
```suggestion
make_decimal_batch(vec![100000, 200000, 300000, 400000,
500000], 12, 5),
```
##########
datafusion/core/src/datasource/physical_plan/parquet/row_groups.rs:
##########
@@ -209,16 +214,61 @@ impl PruningStatistics for BloomFilterStatistics {
let known_not_present = values
.iter()
- .map(|value| match value {
- ScalarValue::Utf8(Some(v)) => sbbf.check(&v.as_str()),
- ScalarValue::Boolean(Some(v)) => sbbf.check(v),
- ScalarValue::Float64(Some(v)) => sbbf.check(v),
- ScalarValue::Float32(Some(v)) => sbbf.check(v),
- ScalarValue::Int64(Some(v)) => sbbf.check(v),
- ScalarValue::Int32(Some(v)) => sbbf.check(v),
- ScalarValue::Int16(Some(v)) => sbbf.check(v),
- ScalarValue::Int8(Some(v)) => sbbf.check(v),
- _ => true,
+ .map(|value| {
+ match value {
+ ScalarValue::Utf8(Some(v)) => sbbf.check(&v.as_str()),
+ ScalarValue::Boolean(Some(v)) => sbbf.check(v),
+ ScalarValue::Float64(Some(v)) => sbbf.check(v),
+ ScalarValue::Float32(Some(v)) => sbbf.check(v),
+ ScalarValue::Int64(Some(v)) => sbbf.check(v),
+ ScalarValue::Int32(Some(v)) => sbbf.check(v),
+ ScalarValue::Int16(Some(v)) => sbbf.check(v),
+ ScalarValue::Int8(Some(v)) => sbbf.check(v),
+ ScalarValue::Decimal128(Some(v), p, s) => match
parquet_type {
+ Type::INT32 => {
+
//https://github.com/apache/parquet-format/blob/eb4b31c1d64a01088d02a2f9aefc6c17c54cc6fc/Encodings.md?plain=1#L35-L42
+ // All physical type are little-endian
+ if *p > 9 {
+ //DECIMAL can be used to annotate the
following types:
+ //
+ // int32: for 1 <= precision <= 9
+ // int64: for 1 <= precision <= 18
+ return true;
+ }
+ let b = (*v as i32).to_le_bytes();
+ let decimal = Decimal::Int32 {
+ value: b,
+ precision: *p as i32,
+ scale: *s as i32,
+ };
+ sbbf.check(&decimal)
+ }
+ Type::INT64 => {
+ if *p > 18 {
+ return true;
+ }
+ let b = (*v as i64).to_le_bytes();
+ let decimal = Decimal::Int64 {
+ value: b,
+ precision: *p as i32,
+ scale: *s as i32,
+ };
+ sbbf.check(&decimal)
+ }
+ Type::FIXED_LEN_BYTE_ARRAY => {
+ // keep with from_bytes_to_i128
+ let b = v.to_be_bytes().to_vec();
+ let decimal = Decimal::Bytes {
Review Comment:
```suggestion
// Use Decimal constructor after
https://github.com/apache/arrow-rs/issues/5325
let decimal = Decimal::Bytes {
```
##########
datafusion/core/src/datasource/physical_plan/parquet/row_groups.rs:
##########
@@ -209,16 +214,61 @@ impl PruningStatistics for BloomFilterStatistics {
let known_not_present = values
.iter()
- .map(|value| match value {
- ScalarValue::Utf8(Some(v)) => sbbf.check(&v.as_str()),
- ScalarValue::Boolean(Some(v)) => sbbf.check(v),
- ScalarValue::Float64(Some(v)) => sbbf.check(v),
- ScalarValue::Float32(Some(v)) => sbbf.check(v),
- ScalarValue::Int64(Some(v)) => sbbf.check(v),
- ScalarValue::Int32(Some(v)) => sbbf.check(v),
- ScalarValue::Int16(Some(v)) => sbbf.check(v),
- ScalarValue::Int8(Some(v)) => sbbf.check(v),
- _ => true,
+ .map(|value| {
+ match value {
+ ScalarValue::Utf8(Some(v)) => sbbf.check(&v.as_str()),
+ ScalarValue::Boolean(Some(v)) => sbbf.check(v),
+ ScalarValue::Float64(Some(v)) => sbbf.check(v),
+ ScalarValue::Float32(Some(v)) => sbbf.check(v),
+ ScalarValue::Int64(Some(v)) => sbbf.check(v),
+ ScalarValue::Int32(Some(v)) => sbbf.check(v),
+ ScalarValue::Int16(Some(v)) => sbbf.check(v),
+ ScalarValue::Int8(Some(v)) => sbbf.check(v),
+ ScalarValue::Decimal128(Some(v), p, s) => match
parquet_type {
+ Type::INT32 => {
+
//https://github.com/apache/parquet-format/blob/eb4b31c1d64a01088d02a2f9aefc6c17c54cc6fc/Encodings.md?plain=1#L35-L42
+ // All physical type are little-endian
+ if *p > 9 {
+ //DECIMAL can be used to annotate the
following types:
+ //
+ // int32: for 1 <= precision <= 9
+ // int64: for 1 <= precision <= 18
+ return true;
+ }
+ let b = (*v as i32).to_le_bytes();
+ let decimal = Decimal::Int32 {
+ value: b,
+ precision: *p as i32,
+ scale: *s as i32,
+ };
+ sbbf.check(&decimal)
+ }
+ Type::INT64 => {
+ if *p > 18 {
+ return true;
+ }
+ let b = (*v as i64).to_le_bytes();
+ let decimal = Decimal::Int64 {
Review Comment:
```suggestion
// Use Decimal constructor after
https://github.com/apache/arrow-rs/issues/5325
let decimal = Decimal::Int64 {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
