Skip to content

Commit

Permalink
Make json_array functions return null with malformed json (#10630)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #10630

This is match the behavior of the equivalent Presto functions, which return NULL when they are given malformed json.

Reviewed By: kevinwilfong

Differential Revision: D60557066

fbshipit-source-id: 980cf1f73394c99fc2ee4bf24d3e005b90cd5351
  • Loading branch information
Daniel Hunte authored and facebook-github-bot committed Aug 2, 2024
1 parent 84428f9 commit ce1566b
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
9 changes: 9 additions & 0 deletions velox/functions/prestosql/JsonFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ struct JsonArrayContainsFunction {
if (simdjsonParse(paddedJson).get(jsonDoc)) {
return false;
}
if (jsonDoc.type().error()) {
return false;
}

if (jsonDoc.type() != simdjson::ondemand::json_type::array) {
return false;
Expand Down Expand Up @@ -132,6 +135,9 @@ struct JsonArrayLengthFunction {
if (simdjsonParse(paddedJson).get(jsonDoc)) {
return false;
}
if (jsonDoc.type().error()) {
return false;
}

if (jsonDoc.type() != simdjson::ondemand::json_type::array) {
return false;
Expand Down Expand Up @@ -159,6 +165,9 @@ struct JsonArrayGetFunction {
if (simdjsonParse(paddedJson).get(jsonDoc)) {
return false;
}
if (jsonDoc.type().error()) {
return false;
}

if (jsonDoc.type() != simdjson::ondemand::json_type::array) {
return false;
Expand Down
14 changes: 14 additions & 0 deletions velox/functions/prestosql/tests/JsonFunctionsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,9 @@ TEST_F(JsonFunctionsTest, jsonArrayLength) {
EXPECT_EQ(jsonArrayLength(R"({"k1":"v1"})"), std::nullopt);
EXPECT_EQ(jsonArrayLength(R"({"k1":[0,1,2]})"), std::nullopt);
EXPECT_EQ(jsonArrayLength(R"({"k1":[0,1,2], "k2":"v1"})"), std::nullopt);

// Malformed Json.
EXPECT_EQ(jsonArrayLength(R"((})"), std::nullopt);
}

TEST_F(JsonFunctionsTest, jsonArrayGet) {
Expand All @@ -372,6 +375,9 @@ TEST_F(JsonFunctionsTest, jsonArrayGet) {
EXPECT_FALSE(arrayGet("{}", 1).has_value());
EXPECT_FALSE(arrayGet("[]", 1).has_value());

// Malformed json.
EXPECT_FALSE(arrayGet("([1]})", 0).has_value());

EXPECT_EQ(arrayGet("[1, 2, 3]", 0), "1");
EXPECT_EQ(arrayGet("[1, 2, 3]", 1), "2");
EXPECT_EQ(arrayGet("[1, 2, 3]", 2), "3");
Expand Down Expand Up @@ -589,6 +595,14 @@ TEST_F(JsonFunctionsTest, jsonArrayContainsString) {
true);
}

TEST_F(JsonFunctionsTest, jsonArrayContainsMalformed) {
auto [jsonVector, _] = makeVectors(R"([]})");
EXPECT_EQ(
evaluateOnce<bool>(
"json_array_contains(c0, 'a')", makeRowVector({jsonVector})),
std::nullopt);
}

TEST_F(JsonFunctionsTest, jsonSize) {
EXPECT_EQ(jsonSize(R"({"k1":{"k2": 999}, "k3": 1})", "$.k1.k2"), 0);
EXPECT_EQ(jsonSize(R"({"k1":{"k2": 999}, "k3": 1})", "$.k1"), 1);
Expand Down

0 comments on commit ce1566b

Please sign in to comment.