jagill commented on code in PR #6643:
URL: https://github.com/apache/arrow-rs/pull/6643#discussion_r1922753579
##########
arrow-json/src/reader/struct_array.rs:
##########
@@ -70,43 +74,84 @@ impl ArrayDecoder for StructArrayDecoder {
.is_nullable
.then(|| BooleanBufferBuilder::new(pos.len()));
- for (row, p) in pos.iter().enumerate() {
- let end_idx = match (tape.get(*p), nulls.as_mut()) {
- (TapeElement::StartObject(end_idx), None) => end_idx,
- (TapeElement::StartObject(end_idx), Some(nulls)) => {
- nulls.append(true);
- end_idx
- }
- (TapeElement::Null, Some(nulls)) => {
- nulls.append(false);
- continue;
+ match self.struct_mode {
Review Comment:
Re: wrong type decoding: I wrote them but left them sloppily in
`test_struct_decoding_list_length`. I've extracted them and extracted most of
the logic of the parsing closure into a function.
Re: option overlapping: Yeah, without a breaking API change, I see three
options:
1. Describe that strict_mode and explicit_nulls are effectively determined
by struct_mode in documentation. (As is)
2. Document that the "wrong" settings for strict_mode/explicit_nulls and
struct_mode is not allowed; panic in that case.
3. Explicitly set strict_mode/explicit_nulls when struct_mode is set, and
checkl when struct_mode/explicit_nulls is set. And document.
I think 3 is weird and probably leaves a footgun for later developers. I
don't have a strong opinion between either 1 or 2.
--
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]
