Skip to content

Commit

Permalink
fix(json_parse): Clear state after encountering an error (facebookinc…
Browse files Browse the repository at this point in the history
…ubator#12150)

Summary:

`json_parse` if it encounters an error and throws in a sensitive part of the code , doesnt clear out its previous state when processing the next row. This can cause it to throw when processing the next valid row.

Reviewed By: xiaoxmeng, amitkdutta

Differential Revision: D68534578
  • Loading branch information
Krishna Pai authored and facebook-github-bot committed Jan 23, 2025
1 parent 1f190f4 commit 5f34ab9
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 0 deletions.
7 changes: 7 additions & 0 deletions velox/functions/prestosql/JsonFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ class JsonParseFunction : public exec::VectorFunction {
auto size = prepareInput(value, needNormalizes[row]);
if (auto error = parse(size, needNormalizes[row])) {
context.setVeloxExceptionError(row, errors_[error]);
clearState();
return;
}
auto outputSize = concatViews(views_, output);
Expand Down Expand Up @@ -457,6 +458,12 @@ class JsonParseFunction : public exec::VectorFunction {
}
}

void clearState() const {
fields_.clear();
sortIndices_.clear();
fastSortKeys_.clear();
}

mutable folly::once_flag initializeErrors_;
mutable std::exception_ptr errors_[simdjson::NUM_ERROR_CODES];
// Padding is needed in case string view is inlined.
Expand Down
15 changes: 15 additions & 0 deletions velox/functions/prestosql/tests/JsonFunctionsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,21 @@ TEST_F(JsonFunctionsTest, jsonParse) {
jsonParse("{\"k1\\"), "Invalid escape sequence at the end of string");
VELOX_ASSERT_USER_THROW(
jsonParse("{\"k1\\u"), "Invalid escape sequence at the end of string");

// Ensure state is cleared after invalid json
{
data = makeRowVector({makeFlatVector<StringView>({
R"({"key":1578377,"name":"Alto Ma\\u00e9 \\"A\\"","type":"cities"})", // invalid json
R"([{"k1": "v1" }, {"k2": "v2" }])" // valid json
})});

result = evaluate("try(json_parse(c0))", data);

expected = makeNullableFlatVector<StringView>(
{std::nullopt, R"([{"k1":"v1"},{"k2":"v2"}])"}, JSON());

velox::test::assertEqualVectors(expected, result);
}
}

TEST_F(JsonFunctionsTest, canonicalization) {
Expand Down

0 comments on commit 5f34ab9

Please sign in to comment.