From a5db8c8492c1c3887cdc8766216e4b44a190b848 Mon Sep 17 00:00:00 2001 From: Advit Arora Date: Tue, 25 Aug 2026 11:32:25 +0530 Subject: [PATCH 1/4] GH-47417: [C++] Fix JSON parser dropping nulls at the start of a list --- cpp/src/arrow/json/parser.cc | 37 ++++++++++++++++--------------- cpp/src/arrow/json/parser_test.cc | 8 +++++++ 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/cpp/src/arrow/json/parser.cc b/cpp/src/arrow/json/parser.cc index 53f856d8012e..1d50b13cca1b 100644 --- a/cpp/src/arrow/json/parser.cc +++ b/cpp/src/arrow/json/parser.cc @@ -529,50 +529,51 @@ 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) { - if (ARROW_PREDICT_FALSE(!builder.nullable)) { + Status AppendNull(BuilderPtr parent, int field_index, BuilderPtr* builder) { + if (ARROW_PREDICT_FALSE(!builder->nullable)) { return ParseError("a required field was null"); } - switch (builder.kind) { + switch (builder->kind) { case Kind::kNull: { - DCHECK_EQ(builder, parent.kind == Kind::kArray - ? Cast(parent)->value_builder() - : Cast(parent)->field_builder(field_index)); + DCHECK_EQ(*builder, + parent.kind == Kind::kArray + ? Cast(parent)->value_builder() + : Cast(parent)->field_builder(field_index)); // increment null count stored inline - builder.index += 1; + builder->index += 1; // update the parent, since changing builder doesn't affect parent if (parent.kind == Kind::kArray) { - Cast(parent)->value_builder(builder); + Cast(parent)->value_builder(*builder); } else { - Cast(parent)->field_builder(field_index, builder); + Cast(parent)->field_builder(field_index, *builder); } return Status::OK(); } case Kind::kBoolean: - return Cast(builder)->AppendNull(); + return Cast(*builder)->AppendNull(); case Kind::kNumber: - return Cast(builder)->AppendNull(); + return Cast(*builder)->AppendNull(); case Kind::kString: - return Cast(builder)->AppendNull(); + return Cast(*builder)->AppendNull(); case Kind::kNumberOrString: { - return Cast(builder)->AppendNull(); + return Cast(*builder)->AppendNull(); } case Kind::kArray: - return Cast(builder)->AppendNull(); + return Cast(*builder)->AppendNull(); case Kind::kObject: { - auto struct_builder = Cast(builder); + auto struct_builder = Cast(*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, &field_builder)); } return Status::OK(); } @@ -672,7 +673,7 @@ class HandlerBase : public BlockParser, /// /// @{ bool Null() { - status_ = builder_set_.AppendNull(builder_stack_.back(), field_index_, builder_); + status_ = builder_set_.AppendNull(builder_stack_.back(), field_index_, &builder_); return status_.ok(); } @@ -869,7 +870,7 @@ class HandlerBase : public BlockParser, if (ARROW_PREDICT_FALSE(!field_builder.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, &field_builder)); } absent_fields_stack_.Pop(); EndNested(); diff --git a/cpp/src/arrow/json/parser_test.cc b/cpp/src/arrow/json/parser_test.cc index 1b107aa020fd..802d353785dd 100644 --- a/cpp/src/arrow/json/parser_test.cc +++ b/cpp/src/arrow/json/parser_test.cc @@ -267,6 +267,14 @@ 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]])"}); +} + TEST(BlockParser, InferNewFields) { std::string src = R"( {} From 609dc0958a2a9897190cb4c74f973b4cb57b8e12 Mon Sep 17 00:00:00 2001 From: Advit Arora Date: Thu, 3 Sep 2026 10:39:59 +0530 Subject: [PATCH 2/4] GH-47417: [C++] Cover the explicit list(null()) schema under Error and Ignore --- cpp/src/arrow/json/parser_test.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/cpp/src/arrow/json/parser_test.cc b/cpp/src/arrow/json/parser_test.cc index 802d353785dd..e10b6c73f9db 100644 --- a/cpp/src/arrow/json/parser_test.cc +++ b/cpp/src/arrow/json/parser_test.cc @@ -235,6 +235,17 @@ 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]]"}); + } +} + TEST(BlockParser, Basics) { auto options = ParseOptions::Defaults(); options.unexpected_field_behavior = UnexpectedFieldBehavior::InferType; From 2bc1be2d59915d9ece3c1fc6e3ed1e5a0a7a43b8 Mon Sep 17 00:00:00 2001 From: Advit Arora Date: Thu, 22 Oct 2026 01:46:33 +0100 Subject: [PATCH 3/4] GH-47417: [C++] Return the updated builder from AppendNull --- cpp/src/arrow/json/parser.cc | 51 +++++++++++++++++-------------- cpp/src/arrow/json/parser_test.cc | 8 +++++ 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/cpp/src/arrow/json/parser.cc b/cpp/src/arrow/json/parser.cc index ad2d3781e4cd..15330ee0d0ec 100644 --- a/cpp/src/arrow/json/parser.cc +++ b/cpp/src/arrow/json/parser.cc @@ -526,58 +526,61 @@ 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) { - if (ARROW_PREDICT_FALSE(!builder->nullable)) { + Result AppendNull(BuilderPtr parent, int field_index) { + BuilderPtr builder = parent.kind == Kind::kArray + ? Cast(parent)->value_builder() + : Cast(parent)->field_builder(field_index); + if (ARROW_PREDICT_FALSE(!builder.nullable)) { return ParseError("a required field was null"); } - switch (builder->kind) { + switch (builder.kind) { case Kind::kNull: { - DCHECK_EQ(*builder, - parent.kind == Kind::kArray - ? Cast(parent)->value_builder() - : Cast(parent)->field_builder(field_index)); - // increment null count stored inline - builder->index += 1; + builder.index += 1; // update the parent, since changing builder doesn't affect parent if (parent.kind == Kind::kArray) { - Cast(parent)->value_builder(*builder); + Cast(parent)->value_builder(builder); } else { - Cast(parent)->field_builder(field_index, *builder); + Cast(parent)->field_builder(field_index, builder); } - return Status::OK(); + break; } case Kind::kBoolean: - return Cast(*builder)->AppendNull(); + RETURN_NOT_OK(Cast(builder)->AppendNull()); + break; case Kind::kNumber: - return Cast(*builder)->AppendNull(); + RETURN_NOT_OK(Cast(builder)->AppendNull()); + break; case Kind::kString: - return Cast(*builder)->AppendNull(); + RETURN_NOT_OK(Cast(builder)->AppendNull()); + break; case Kind::kNumberOrString: { - return Cast(*builder)->AppendNull(); + RETURN_NOT_OK(Cast(builder)->AppendNull()); + break; } case Kind::kArray: - return Cast(*builder)->AppendNull(); + RETURN_NOT_OK(Cast(builder)->AppendNull()); + break; case Kind::kObject: { - auto struct_builder = Cast(*builder); + auto struct_builder = Cast(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& scalar_values, BuilderPtr builder, @@ -670,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) { @@ -967,7 +972,7 @@ class ParseImpl : public BlockParser { if (ARROW_PREDICT_FALSE(!field_builder.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 1196e3a654e4..ae764c718047 100644 --- a/cpp/src/arrow/json/parser_test.cc +++ b/cpp/src/arrow/json/parser_test.cc @@ -245,6 +245,11 @@ TEST(BlockParserWithSchema, NullsInList) { 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) { @@ -303,6 +308,9 @@ TEST(BlockParser, NullsInList) { 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) { From d29f49ee2af1b935ef379a808a548e1d1aad4f63 Mon Sep 17 00:00:00 2001 From: Advit Arora Date: Thu, 22 Oct 2026 12:43:34 +0100 Subject: [PATCH 4/4] GH-47417: [C++] Inline the nullable check for absent fields --- cpp/src/arrow/json/parser.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cpp/src/arrow/json/parser.cc b/cpp/src/arrow/json/parser.cc index 15330ee0d0ec..08d83f0c67fb 100644 --- a/cpp/src/arrow/json/parser.cc +++ b/cpp/src/arrow/json/parser.cc @@ -968,8 +968,7 @@ class ParseImpl : public BlockParser { if (!absent_fields_stack_[i]) { continue; } - auto field_builder = Cast(parent)->field_builder(i); - if (ARROW_PREDICT_FALSE(!field_builder.nullable)) { + if (ARROW_PREDICT_FALSE(!Cast(parent)->field_builder(i).nullable)) { return ParseError("a required field was absent"); } RETURN_NOT_OK(builder_set_.AppendNull(parent, i));