qzyu999 commented on PR #50122:
URL: https://github.com/apache/arrow/pull/50122#issuecomment-4815343351
Force-pushed a refactored version. The initial implementation carried over
some Go patterns (manual start/finish without safety guarantees, separate
lookup buffer). After reviewing the feedback — particularly around
initialization from existing buffers and the question about modifying existing
variants, I reworked the builder with C++ idiom
at the center of the design.
Key changes:
- **RAII scopes** (`ObjectScope`/`ListScope`) with auto-rollback on
destruction replace
the previous unguarded start/finish pattern
- **`[[nodiscard]]`** on scope-returning functions prevents silent discard
- **Transparent hasher** on `dict_` eliminates the old `lookup_buf_` member
variable
- **Sorted-check optimization** in `FinishObject` (skip sort when fields
already ordered)
- Move-only builder (copy deleted) enforces single ownership
Regarding the earlier review questions about modifying existing variants and
type mismatch testing, I've addressed these in replies above with architectural
context from the refactored design.
All 335 tests pass end-to-end with `BUILD_WARNING_LEVEL=CHECKIN` (87
encoder-specific).
--
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]
