Joob1n opened a new issue, #345:
URL: https://github.com/apache/paimon-cpp/issues/345

   ### Search before asking
   
   - [x] I searched in the 
[issues](https://github.com/apache/paimon-cpp/issues) and found nothing similar.
   
   ### Paimon-cpp version
   
   v0.3.0. The affected code is unchanged on `main` (ccfe346670f3).
   
   ### Minimal reproduce step
   
   1. Write a Parquet file with a `DECIMAL(9,2)` column using a writer that 
stores decimals as `FIXED_LEN_BYTE_ARRAY` (Arrow with the default 
`store_decimal_as_integer = false`, Spark with 
`spark.sql.parquet.writeLegacyFormat=true`, or Hive). Precision 9 gives a 
4-byte fixed length. Enable the page index and write at least two pages, e.g. 
page 1 holds values 1.00..2.00 and page 2 holds 10.00..20.00.
   2. Register the file in a Paimon table (migrated table or format table) and 
read it through paimon-cpp with the predicate `col < 5.00`.
   3. Compare with the same predicate on a file written with 
`store_decimal_as_integer = true`.
   
   ### What doesn't meet your expectations?
   
   The rows from page 1 are missing from the result on the FIXED_LEN_BYTE_ARRAY 
file. The same predicate returns them on the INT32 file.
   
   Cause: `ColumnIndexFilter::CompareEncodedWithLiteral` 
(`src/paimon/format/parquet/column_index_filter.cpp`, `case 
FieldType::DECIMAL`) infers the physical type from the length of the encoded 
page min/max:
   
   - 4 bytes -> treated as INT32, decoded with a native-endian `memcpy`
   - 8 bytes -> treated as INT64, decoded with a native-endian `memcpy`
   - other lengths -> treated as FIXED_LEN_BYTE_ARRAY, decoded as big-endian 
two's complement via `Decimal::FromUnscaledBytes`
   
   Parquet allows DECIMAL as FIXED_LEN_BYTE_ARRAY for any precision with length 
`ceil((precision*log2(10)+1)/8)`. Precision 7-9 yields exactly 4 bytes and 
precision 17-18 exactly 8 bytes, so those columns hit the INT32/INT64 branches 
and their big-endian statistics are read little-endian.
   
   Example: page min 1.00 (unscaled 100) is stored as `00 00 00 64`. Read as 
little-endian int32 that is 1,677,721,600. `FilterPagesByLessThan` asks whether 
page min < 5.00, gets "no", and drops the page. Page-index filtering is 
pruning, not filtering, so nothing downstream can bring the rows back.
   
   ### Anything else?
   
   Paimon's own writers are not affected: `ParquetSchemaConverter` (Java) and 
`parquet_writer_builder.cpp` in this repo both use INT32 for precision <= 9 and 
INT64 for precision <= 18. That is also why the in-repo tests never see it; 
every test file is produced by the in-repo writer. The bug reaches users 
through Parquet files written elsewhere.
   
   `parquet_stats_extractor.cpp` in the same directory already handles this 
correctly for row-group statistics by dispatching on 
`primitive_node->physical_type()`. The page-index path should do the same 
instead of inferring the type from the length. `CalculateRowRanges` currently 
only receives `column_name_to_index`; the `::parquet::SchemaDescriptor` (or the 
per-column physical type) would need to be passed down from 
`FileReaderWrapper::CalculateFilteredRowRanges`, where 
`file_reader_->parquet_reader()->metadata()->schema()` is available.
   
   A regression test could write DECIMAL(9,2) and DECIMAL(18,2) columns with 
`store_decimal_as_integer` disabled and the page index enabled, then assert 
that `col < literal` keeps the page containing matching rows.
   
   ### Are you willing to submit a PR?
   
   - [ ] I'm willing to submit a PR!
   


-- 
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]

Reply via email to