wgtmac commented on PR #939: URL: https://github.com/apache/iceberg-cpp/pull/939#issuecomment-6110056473
Rebased onto latest `main` (which already contains #974 and #975) and pushed a follow-up commit `72c8a23d1` with three small fixes from the review. **Rebase / conflict** - `catalog/rest/json_serde.cc` still called `iceberg::rest::DataFileFromJson`. The merged #974 removed that REST forwarder, so the call now uses `iceberg::DataFileFromJson` and keeps the new `start`/`length` range validation (`CheckFileScanTaskNotSplit`). **Follow-up fixes** 1. **Duplicated strict integer parsing** — the REST-local `GetRangeValueOrDefault` re-implemented core's logic (and only checked the `uint64` upper bound). It is replaced by a shared `iceberg::GetJsonIntegerOrDefault` declared in `json_serde_internal.h`, implemented on top of core's `IntegerFromJson`, so both readers share one implementation with the same messages and the same `kJsonParseError` kind. 2. **Empty `delete-files`** — `ToJson(FileScanTask)` always emitted `"delete-files": []`, while Java omits the key when `deletes() == null`. The key is now omitted when the task has no delete files; the read side is unchanged (absent and empty array are both accepted). 3. **`partition_specs_by_id.at(spec_id)`** — it relied on an earlier `ToJson(data_file, ...)` call having validated that the spec exists, and would throw `std::out_of_range` if that order ever changed. It now does an explicit `find` and returns `ValidationFailed`. Static verification only: `-fsyntax-only` (C++23, `-Werror`) on the four touched translation units and `clang-format --dry-run --Werror` are clean; no local build or test run. -- 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]
