You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "LuciferYang (via GitHub)" <gi...@apache.org> on 2026/08/07 16:03:14 UTC
[PR] fix: reject out-of-range integers when parsing JSON literals [iceberg-cpp]
LuciferYang opened a new pull request, #878:
URL: https://github.com/apache/iceberg-cpp/pull/878
## What
`LiteralFromJson` accepted JSON integers beyond the signed 64-bit range and produced a wrong literal instead of a parse error. nlohmann reports unsigned integers as `is_number_integer()`, and `get<int64_t>()` converts values above INT64_MAX silently rather than throwing, so the `kInt` branch ran its int32 range check on the already-wrapped value and the `kLong` branch had no range check at all. `18446744073709551615` parsed as `Literal::Int(-1)`, and `9223372036854775808` as `Literal::Long(INT64_MIN)`. The untyped overload had the same hole.
The live consequence is on table metadata: `initial-default` / `write-default` go through this parser, `ValidateDefault` has no integer range check, and the value is later materialized into a returned column. A metadata file that Java rejects reads back as `-1` in C++. The expression path is latent for now, since no reader evaluates `ReaderOptions::filter` yet.
Fixes #877.
## How
Added `GetInt64Checked`, which rejects unsigned nodes above INT64_MAX before the conversion, and called it from the `kInt` branch, the `kLong` branch, and the untyped overload. This mirrors Java, where `SingleValueParser` and `ExpressionParser` guard the same paths with `canConvertToInt()` / `canConvertToLong()`.
The `is_number_unsigned()` half of the guard is load-bearing: `get<uint64_t>()` on a negative node yields its two's-complement value, which compares above INT64_MAX and would reject every negative literal. Both halves now have accept-side tests.
Also in this PR, both on lines the fix touches: the int32 narrowing on the `kInt` path gained the coverage it never had, and the two out-of-range messages now use one wording instead of saying "int" in one place and "long" in the other.
## Testing
`expression_test` 526 tests and the full `ctest` suite (18/18) pass. Each new test was checked against a mutation of the code it guards:
- `>` changed to `>=` in the guard: only `LongMax` fails (the `ULL` suffix there is load-bearing, a signed INT64_MAX node would skip the unsigned branch entirely).
- `is_number_unsigned() &&` dropped: `LongMin`, `IntNegative` and `AcceptsNegativeIntegerUntyped` fail.
- int32 range check deleted: `IntAboveInt32Max` and `IntBelowInt32Min` fail.
Verified fail-without / pass-with for the three overflow-rejection cases as well.
## Out of scope
`GetTypedJsonValue` in `src/iceberg/util/json_util_internal.h` truncates out-of-range integers the same way, so `FieldFromJson({"id": 2147483648, ...})` yields `field_id = -2147483648` silently. That helper has on the order of 80 call sites and is left for a follow-up rather than widened into this 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: 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] fix: reject out-of-range integers when parsing JSON literals [iceberg-cpp]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #878:
URL: https://github.com/apache/iceberg-cpp/pull/878#issuecomment-5226364360
friendly ping @wgtmac
--
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] fix: reject out-of-range integers when parsing JSON literals [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac merged PR #878:
URL: https://github.com/apache/iceberg-cpp/pull/878
--
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] fix: reject out-of-range integers when parsing JSON literals [iceberg-cpp]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #878:
URL: https://github.com/apache/iceberg-cpp/pull/878#issuecomment-5311101205
Than you @wgtmac
--
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