manuzhang commented on code in PR #755:
URL: https://github.com/apache/iceberg-cpp/pull/755#discussion_r3636922209
##########
src/iceberg/util/truncate_util.h:
##########
@@ -84,7 +86,15 @@ class ICEBERG_EXPORT TruncateUtils {
template <typename T>
requires std::is_same_v<T, int32_t> || std::is_same_v<T, int64_t>
static inline T TruncateInteger(T v, int32_t W) {
- return v - (((v % W) + W) % W);
Review Comment:
+1 to open a discussion on the dev list. Actually, this boundary behavior
has been reported in the Java repository before:
- [apache/iceberg#9767](https://github.com/apache/iceberg/issues/9767)
reproduces `INT_MIN` wrapping to a large positive partition value and causing
incorrect partition pruning.
- [apache/iceberg#9768](https://github.com/apache/iceberg/issues/9768)
notes that the specification is unclear when the mathematical truncate result
is outside the source type.
- [apache/iceberg#13105](https://github.com/apache/iceberg/issues/13105)
reproduces the same `LONG_MIN`, width `10` case handled here: the result
becomes `LONG_MAX - 1`, and a query such as `a < 0` misses the row.
- [apache/iceberg#12915](https://github.com/apache/iceberg/issues/12915)
covers the analogous decimal boundary and also mentions the integer
minimum-value problem.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
