qzyu999 commented on PR #50121:
URL: https://github.com/apache/arrow/pull/50121#issuecomment-4815318185
I've force-pushed a refactored version that addresses all review feedback.
After carefully reviewing the comments, it became clear that several design
choices in the earlier iteration stemmed from initially trying to follow Go's
implementation patterns (free functions, manual buffer management,
linear/binary threshold). I've since reworked the implementation from scratch
with C++ ergonomics and idiom as the guiding principle.
Key changes in this force-push:
- **View classes** (`VariantView`, `VariantObjectView`, `VariantArrayView`)
replace the
previous free-function API, parse headers once at construction, O(log n)
binary search
always (no threshold), `std::optional` for not-found semantics
- **Numeric coercion accessors** (`as_int64_coerced`, `as_double_coerced`)
for Rust parity
- **Recursive validation** (`ValidateVariant`) for untrusted input
- **Shared internal utility** (`variant_internal_util.h`) consolidates
ReadLE helpers
- Previous `variant_internal.h` naming confusion resolved, main API is
`variant.h`
- Test utility renamed to `variant_internal_test_util.h` (ensures it's not
installed)
All 335 tests pass end-to-end with `BUILD_WARNING_LEVEL=CHECKIN` (134
decoder-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]
