This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 2cadeab93eb GH-47417: [C++] Fix JSON parser losing nulls in a
null-typed list (#51108)
2cadeab93eb is described below
commit 2cadeab93eb6e5e9e9707899ebd042a4c4a8302d
Author: Advit Arora <[email protected]>
AuthorDate: Mon Oct 26 10:11:40 2026 +0000
GH-47417: [C++] Fix JSON parser losing nulls in a null-typed list (#51108)
### Rationale for this change
`{"a": [null, 1]}` silently parses to `[[1]]` in release builds, and aborts
in debug.
This was introduced in `b7054c21aab` (0.14.0).
### What changes are included in this PR?
`AppendNull` looks up its child from the parent and returns the updated
`BuilderPtr`, instead of taking a copy from the caller. `Null()` writes it back
to `builder_`, so `builder_` no longer drifts from the parent's copy.
### Are these changes tested?
Two new tests, `BlockParser.NullsInList` (inferred schema) and
`BlockParserWithSchema.NullsInList` (explicit schema), parse lists of nulls,
including `{"a": [{"b": null}, null]}`. Both fail in debug without the fix.
### Are there any user-facing changes?
`{"a": [null, 1]}` now parses to `[[null, 1]]`.
**This PR contains a "Critical Fix".** Well-formed JSON silently loses list
elements.
* GitHub Issue: #47417
### Was AI used for this PR?
In accordance to the [AI generation
guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code),
please disclose below whether and how AI was used in this PR.
**PR code and description written by:**
- [x] Human
- [ ] AI
**Reviewed before submission by:**
- [x] Human
- [ ] AI
- [ ] Not reviewed
Authored-by: Advit Arora <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/json/parser.cc | 41 ++++++++++++++++++++++-----------------
cpp/src/arrow/json/parser_test.cc | 27 ++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/cpp/src/arrow/json/parser.cc b/cpp/src/arrow/json/parser.cc
index 69373d07fb6..08d83f0c67f 100644
--- a/cpp/src/arrow/json/parser.cc
+++ b/cpp/src/arrow/json/parser.cc
@@ -526,16 +526,15 @@ class RawBuilderSet {
/// Appending null is slightly tricky since null count is stored inline
/// for builders of Kind::kNull. Append nulls using this helper
- Status AppendNull(BuilderPtr parent, int field_index, BuilderPtr builder) {
+ Result<BuilderPtr> AppendNull(BuilderPtr parent, int field_index) {
+ BuilderPtr builder = parent.kind == Kind::kArray
+ ? Cast<Kind::kArray>(parent)->value_builder()
+ :
Cast<Kind::kObject>(parent)->field_builder(field_index);
if (ARROW_PREDICT_FALSE(!builder.nullable)) {
return ParseError("a required field was null");
}
switch (builder.kind) {
case Kind::kNull: {
- DCHECK_EQ(builder, parent.kind == Kind::kArray
- ? Cast<Kind::kArray>(parent)->value_builder()
- :
Cast<Kind::kObject>(parent)->field_builder(field_index));
-
// increment null count stored inline
builder.index += 1;
@@ -545,38 +544,43 @@ class RawBuilderSet {
} else {
Cast<Kind::kObject>(parent)->field_builder(field_index, builder);
}
- return Status::OK();
+ break;
}
case Kind::kBoolean:
- return Cast<Kind::kBoolean>(builder)->AppendNull();
+ RETURN_NOT_OK(Cast<Kind::kBoolean>(builder)->AppendNull());
+ break;
case Kind::kNumber:
- return Cast<Kind::kNumber>(builder)->AppendNull();
+ RETURN_NOT_OK(Cast<Kind::kNumber>(builder)->AppendNull());
+ break;
case Kind::kString:
- return Cast<Kind::kString>(builder)->AppendNull();
+ RETURN_NOT_OK(Cast<Kind::kString>(builder)->AppendNull());
+ break;
case Kind::kNumberOrString: {
- return Cast<Kind::kNumberOrString>(builder)->AppendNull();
+ RETURN_NOT_OK(Cast<Kind::kNumberOrString>(builder)->AppendNull());
+ break;
}
case Kind::kArray:
- return Cast<Kind::kArray>(builder)->AppendNull();
+ RETURN_NOT_OK(Cast<Kind::kArray>(builder)->AppendNull());
+ break;
case Kind::kObject: {
auto struct_builder = Cast<Kind::kObject>(builder);
RETURN_NOT_OK(struct_builder->AppendNull());
for (int i = 0; i < struct_builder->num_fields(); ++i) {
- auto field_builder = struct_builder->field_builder(i);
- RETURN_NOT_OK(AppendNull(builder, i, field_builder));
+ RETURN_NOT_OK(AppendNull(builder, i));
}
- return Status::OK();
+ break;
}
default:
return Status::NotImplemented("invalid builder Kind");
}
+ return builder;
}
Status Finish(const std::shared_ptr<Array>& scalar_values, BuilderPtr
builder,
@@ -669,7 +673,9 @@ class ParseImpl : public BlockParser {
Status Null() {
DCHECK(!builder_stack_.empty());
- return builder_set_.AppendNull(builder_stack_.back(), field_index_,
builder_);
+ ARROW_ASSIGN_OR_RAISE(builder_,
+ builder_set_.AppendNull(builder_stack_.back(),
field_index_));
+ return Status::OK();
}
Status HandleUnexpectedField(std::string_view key, sj::value& value) {
@@ -962,11 +968,10 @@ class ParseImpl : public BlockParser {
if (!absent_fields_stack_[i]) {
continue;
}
- auto field_builder = Cast<Kind::kObject>(parent)->field_builder(i);
- if (ARROW_PREDICT_FALSE(!field_builder.nullable)) {
+ if
(ARROW_PREDICT_FALSE(!Cast<Kind::kObject>(parent)->field_builder(i).nullable)) {
return ParseError("a required field was absent");
}
- RETURN_NOT_OK(builder_set_.AppendNull(parent, i, field_builder));
+ RETURN_NOT_OK(builder_set_.AppendNull(parent, i));
}
absent_fields_stack_.Pop();
EndNested();
diff --git a/cpp/src/arrow/json/parser_test.cc
b/cpp/src/arrow/json/parser_test.cc
index 6c32fd7cab3..ae764c71804 100644
--- a/cpp/src/arrow/json/parser_test.cc
+++ b/cpp/src/arrow/json/parser_test.cc
@@ -236,6 +236,22 @@ TEST(BlockParserWithSchema, FailOnIncompleteJson) {
ASSERT_RAISES(Invalid, ParseFromString(options, "{\"a\":0, \"b\"", &parsed));
}
+TEST(BlockParserWithSchema, NullsInList) {
+ auto options = ParseOptions::Defaults();
+ options.explicit_schema = schema({field("a", list(null()))});
+ for (auto behavior :
+ {UnexpectedFieldBehavior::Error, UnexpectedFieldBehavior::Ignore}) {
+ options.unexpected_field_behavior = behavior;
+ AssertParseColumns(options, R"({"a": [null, null]})", {field("a",
list(null()))},
+ {"[[null, null]]"});
+ }
+
+ auto type = list(struct_({field("b", null())}));
+ options.explicit_schema = schema({field("a", type)});
+ AssertParseColumns(options, R"({"a": [{"b": null}, null]})", {field("a",
type)},
+ {R"([[{"b": null}, null]])"});
+}
+
TEST(BlockParserWithSchema, ValidateIgnoredFields) {
auto options = ParseOptions::Defaults();
options.explicit_schema = schema({field("known", int64())});
@@ -286,6 +302,17 @@ TEST(BlockParser, Null) {
R"([{"plain": null}, {"plain": null}])"});
}
+TEST(BlockParser, NullsInList) {
+ auto options = ParseOptions::Defaults();
+ options.unexpected_field_behavior = UnexpectedFieldBehavior::InferType;
+ AssertParseColumns(options, R"({"a": [null, null], "b": [null, "hi",
null]})",
+ {field("a", list(null())), field("b", list(utf8()))},
+ {"[[null, null]]", R"([[null, "hi", null]])"});
+ AssertParseColumns(options, R"({"a": [{"b": null}, null]})",
+ {field("a", list(struct_({field("b", null())})))},
+ {R"([[{"b": null}, null]])"});
+}
+
TEST(BlockParser, InferNewFields) {
std::string src = R"(
{}
