You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "mzabaluev (via GitHub)" <gi...@apache.org> on 2026/07/29 11:13:42 UTC
[I] JSON decoder for RunEndEncoded accepts nulls even though the values field is non-nullable [arrow-rs]
mzabaluev opened a new issue, #10478:
URL: https://github.com/apache/arrow-rs/issues/10478
### Describe the bug
The [implementation](https://github.com/apache/arrow-rs/blob/005a2fd43f1b243ed43f4cfa356ebe8f0757939b/arrow-json/src/reader/run_end_array.rs#L51) for REE decoder forces the child field's decoder to accept nulls even though the Arrow schema for the values field may declare it non-nullable. As the REE encoding has no container-level null mask, this may smuggle null values into the values array that must not have them.
Furthermore, even if the REE field itself is non-nullable, the flag passed down to e.g. `StringArrayDecoder` is ignored. The resulting nulls are not invalidated by a post-hoc check the way `StructArrayDecoder` does.
### To Reproduce
Repro test case:
```rust
#[test]
fn test_read_run_end_encoded_nulls_in_non_nullable_values() {
// A `RunArray` has no validity buffer of its own, so unlike a struct child
// there is no parent-level mask that could make these nulls representable.
// Declaring the enclosing field nullable does not change that.
let ree_type = DataType::RunEndEncoded(
Arc::new(Field::new("run_ends", DataType::Int32, false)),
Arc::new(Field::new("values", DataType::Utf8, false)),
);
let schema = Arc::new(Schema::new(vec![Field::new(
"a",
ree_type,
true,
)]));
// An explicit null, and a missing field, should be both rejected.
for buf in [
r#"
{"a": "x"}
{"a": null}
{"a": "y"}
"#,
r#"
{"a": "x"}
{}
{"a": "y"}
"#,
] {
let mut decoder = ReaderBuilder::new(schema.clone()).build_decoder().unwrap();
let res = decoder
.decode(buf.as_bytes())
.and_then(|_| decoder.flush());
assert!(res.is_err());
}
}
```
### Expected behavior
The schema violation should produce an error.
### Additional context
Note that this is a breaking change, since the current implementation may tolerate nullable inputs, even if they produce non-compliant outputs.
--
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: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] JSON decoder for RunEndEncoded accepts nulls even though the values field is non-nullable [arrow-rs]
Posted by "mzabaluev (via GitHub)" <gi...@apache.org>.
mzabaluev commented on issue #10478:
URL: https://github.com/apache/arrow-rs/issues/10478#issuecomment-5265435201
#10485 is an open PR to fix this (so I held off on submitting my own), but it seems to have stalled after the initial review. Check the discussion there and feel free to pick it up. It's good if you are able to participate as a human and respond to feedback.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] JSON decoder for RunEndEncoded accepts nulls even though the values field is non-nullable [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey closed issue #10478: JSON decoder for RunEndEncoded accepts nulls even though the values field is non-nullable
URL: https://github.com/apache/arrow-rs/issues/10478
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] JSON decoder for RunEndEncoded accepts nulls even though the values field is non-nullable [arrow-rs]
Posted by "HussainQadri (via GitHub)" <gi...@apache.org>.
HussainQadri commented on issue #10478:
URL: https://github.com/apache/arrow-rs/issues/10478#issuecomment-5260858725
Hi, I'd like to work on this. I'll start by reproducing the issue and tracing how nullability is propagated through the RunEndEncoded JSON decoder, then add a regression test and fix. I'll follow up with a PR.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] JSON decoder for RunEndEncoded accepts nulls even though the values field is non-nullable [arrow-rs]
Posted by "minutechreview (via GitHub)" <gi...@apache.org>.
minutechreview commented on issue #10478:
URL: https://github.com/apache/arrow-rs/issues/10478#issuecomment-5261669078
I'd like to pick this up, with one question on the intended shape of the fix.
My reading of `RunEndEncodedArrayDecoder::new`:
```rust
let decoder = ctx.make_decoder(
values_field.data_type(),
values_field.is_nullable() || is_nullable,
)?;
```
The `|| is_nullable` is the part that doesn't hold up. A `RunArray` has no container-level validity buffer, so an outer "this REE field is nullable" can't be represented at the REE level — propagating it downward silently relaxes the values field's own nullability, which is exactly the smuggling you describe. `values_field.is_nullable()` alone looks like the correct capability to hand the child.
That covers the first half. The second half you raise is the part I'd rather not guess at: even with the flag correctly `false`, decoders like `StringArrayDecoder` ignore it, so nothing actually rejects a null that arrives from the tape — whereas `StructArrayDecoder` catches it after the fact. So dropping `|| is_nullable` alone would fix the schema-propagation bug but still let a literal `null` in the JSON land in a non-nullable values array.
Which would you prefer?
1. **Minimal** — drop `|| is_nullable` so the child decoder gets the values field's declared nullability, and add your repro as a regression test. Leaves the "decoder ignores the flag" gap open as a separate concern.
2. **Also validate in the REE decoder** — additionally check the decoded values array for nulls when `values_field.is_nullable()` is false and error out, mirroring the post-hoc approach in `StructArrayDecoder`.
3. **Fix it at the child-decoder level** — make the primitive/string decoders honour the flag they're given. Broader blast radius and presumably affects more than REE, so I'd assume you'd want that separately.
I'd lean towards (2) as the self-contained fix, but this is your codebase's convention rather than mine to set. Happy to start as soon as you point me at one — and if someone else is already on it, no problem at all.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org