You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "kamcheungting-db (via GitHub)" <gi...@apache.org> on 2026/09/17 08:25:03 UTC
[PR] fix(serde): align FileScanTask JSON with Java core [iceberg-cpp]
kamcheungting-db opened a new pull request, #939:
URL: https://github.com/apache/iceberg-cpp/pull/939
## Summary
- Align core `FileScanTask` JSON serde with Java:
- embed `schema` and `spec`
- serialize `task-type`, `data-file`, `start`, `length`, and `delete-files`
- align ContentFile partition, binary, enum, and numeric handling
- Reject unsupported split tasks exactly when:
- `start != 0`, or
- `length != file_size`
- Keep REST FileScanTask serde behavior separate.
## Testing
- `json_serde_test`: 134 passed
- `rest_catalog_test`: 332 passed, 1 existing test skipped
- Verified the patch applies and builds cleanly against latest `origin/main`
--
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] feat(serde): add core FileScanTask JSON support [iceberg-cpp]
Posted by "kamcheungting-db (via GitHub)" <gi...@apache.org>.
kamcheungting-db commented on PR #939:
URL: https://github.com/apache/iceberg-cpp/pull/939#issuecomment-5907123302
I split the original change into a reviewable stack:
1. #974 — mechanical `DataFile` serde move into core
2. #975 — Java-compatible `DataFile` JSON
3. **#939 — core `FileScanTask` serde and split validation**
4. #940 — strict schema and partition ID parsing
For the incremental diff that belongs only to this PR, use:
https://github.com/kamcheungting-db/iceberg-cpp/compare/datafile-serde-java...filescan-task-serde
The key distinction in this layer is:
- Core task JSON is self-contained and uses inline `"delete-files": [...]`.
- REST may use response-local `"delete-file-references": [0]`, which remains REST-only.
- For a 100-byte file, only `start = 0` and `length = 100` are accepted; a split such as `start = 10` is rejected instead of silently becoming a whole-file task.
The PR description now includes a complete JSON example and the stack order.
--
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] feat(serde): support standalone FileScanTask JSON in core [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac merged PR #939:
URL: https://github.com/apache/iceberg-cpp/pull/939
--
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] feat(serde): support standalone FileScanTask JSON in core [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
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: 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