You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "anoopj (via GitHub)" <gi...@apache.org> on 2026/10/09 21:32:35 UTC

[PR] test(reader): widen FLBA decimal page-index coverage and clarify docs [iceberg-rust]

anoopj opened a new pull request, #3386:
URL: https://github.com/apache/iceberg-rust/pull/3386

   ## What changes are included in this PR?
   
   Address review feedback on #3328:
   - Reframe the width-guard doc and inline comment as defence-in-depth against a non-conforming writer
   - Hoist the duplicated min/max decode into a single closure.
   - Add page-index tests
   
   ## Are these changes tested?
   
   Added tests
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] test(reader): widen FLBA decimal page-index coverage and clarify docs [iceberg-rust]

Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka merged PR #3386:
URL: https://github.com/apache/iceberg-rust/pull/3386


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] test(reader): widen FLBA decimal page-index coverage and clarify docs [iceberg-rust]

Posted by "JeonDaehong (via GitHub)" <gi...@apache.org>.
JeonDaehong commented on code in PR #3386:
URL: https://github.com/apache/iceberg-rust/pull/3386#discussion_r4236397619


##########
crates/iceberg/src/expr/visitors/page_index_evaluator.rs:
##########
@@ -548,9 +540,11 @@ impl<'a> PageIndexEvaluator<'a> {
     /// Parquet stores Iceberg decimals with precision > 18 as big-endian
     /// two's-complement `FIXED_LEN_BYTE_ARRAY` of width `type_length`.
     ///
-    /// Returns an error for a bound whose width differs from `type_length`: that
-    /// signals a `column_index_truncate_length` truncation, and a truncated byte
-    /// prefix decodes to the wrong number under two's-complement.
+    /// Returns an error for a bound whose width differs from `type_length`.
+    /// Conforming writers never truncate a decimal column index (a truncated

Review Comment:
   I checked this claim against both writers: `parquet` 59.3.0 (the version in `Cargo.lock`) skips truncation for `Decimal`/`Float16` `FIXED_LEN_BYTE_ARRAY` in `can_truncate_value`, while `parquet-java` 1.17.1's `BinaryTruncator.getTruncator` only truncates `String`/`Enum`/`JSON`/`BSON` and returns the no-op truncator for decimals. Therefore, the "non-conforming writer" framing holds for files written by both `iceberg-rust` and `iceberg-java`.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org