Skip to content

Commit 8161d65

Browse files
committed
fix from_json function
* prevent from_json from concatenating duplicate case-insensitive fields. * make spark from_json case-sensitive
1 parent 3b9a9ed commit 8161d65

4 files changed

Lines changed: 97 additions & 31 deletions

File tree

‎bolt/functions/prestosql/tests/JsonCastTest.cpp‎

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,7 +1016,7 @@ TEST_F(JsonCastTest, toRow) {
10161016
auto map = makeNullableFlatVector<JsonNativeType>(
10171017
{R"({"c0":123,"c1":"abc","c2":true})"_sv,
10181018
R"({"c1":"abc","c2":true,"c0":123})"_sv,
1019-
R"({"c0":123,"c2":true,"c0":456})"_sv,
1019+
R"({"c10":123,"c2":true,"c0":456})"_sv,
10201020
R"({"c3":123,"c4":"abc","c2":false})"_sv,
10211021
R"({"c0":null,"c2":false})"_sv,
10221022
R"({"c0":null,"c2":null,"c1":null})"_sv},
@@ -1037,17 +1037,17 @@ TEST_F(JsonCastTest, toRow) {
10371037

10381038
// Use a mix of lower case and upper case JSON keys.
10391039
map = makeNullableFlatVector<JsonNativeType>(
1040-
{R"({"c0":123,"c1":"abc","c2":true})"_sv,
1041-
R"({"c1":"abc","c2":true,"c0":123})"_sv,
1042-
R"({"c0":123,"c2":true,"c0":456})"_sv,
1043-
R"({"c3":123,"c4":"abc","c2":false})"_sv,
1040+
{R"({"C0":123,"C1":"abc","C2":true})"_sv,
1041+
R"({"c1":"abc","C2":true,"c0":123})"_sv,
1042+
R"({"C10":123,"C2":true,"c0":456})"_sv,
1043+
R"({"c3":123,"C4":"abc","c2":false})"_sv,
10441044
R"({"c0":null,"c2":false})"_sv,
1045-
R"({"c0":null,"c2":null,"c1":null})"_sv},
1045+
R"({"c0":null,"c2":null,"C1":null})"_sv},
10461046
JSON());
10471047
testCast(map, makeRowVector({child4, child5, child6}));
10481048

10491049
// Use a mix of lower case and upper case field names in target ROW type.
1050-
testCast(map, makeRowVector({child4, child5, child6}));
1050+
testCast(map, makeRowVector({"c0", "C1", "C2"}, {child4, child5, child6}));
10511051

10521052
// Test casting to ROW from JSON null.
10531053
auto null = makeNullableFlatVector<JsonNativeType>({"null"_sv}, JSON());
@@ -1308,3 +1308,35 @@ TEST_F(JsonCastTest, tryCastFromJson) {
13081308
evaluateAndVerify(
13091309
JSON(), ROW({REAL()}), makeRowVector({data}), expectedRow, true);
13101310
}
1311+
1312+
TEST_F(JsonCastTest, duplicateCaseInsensitiveFields) {
1313+
auto input = makeFlatVector<JsonNativeType>(
1314+
{R"({"Name":"first","name":"second"})"_sv}, JSON());
1315+
1316+
BOLT_ASSERT_USER_THROW(
1317+
evaluateCast(JSON(), ROW({"name"}, {VARCHAR()}), makeRowVector({input})),
1318+
"Duplicate field: name");
1319+
}
1320+
1321+
TEST_F(JsonCastTest, fromJsonIgnoresDuplicateCaseInsensitiveFields) {
1322+
auto input = makeFlatVector<StringView>(
1323+
{R"({"FieldA":["READ","BLUE"],"Foo":"Bar","fieldA":["red","blue"],"foo":"bar"})"},
1324+
JSON());
1325+
auto resultType = ROW({"FieldA", "Foo"}, {ARRAY(VARCHAR()), VARCHAR()});
1326+
auto inputRow = makeRowVector({input});
1327+
SelectivityVector rows(input->size());
1328+
exec::ExprSet exprSet({}, &execCtx_);
1329+
exec::EvalCtx context(&execCtx_, &exprSet, inputRow.get());
1330+
VectorPtr result;
1331+
1332+
auto* jsonCast =
1333+
dynamic_cast<const JsonCastOperator*>(JsonCastOperator::get().get());
1334+
ASSERT_NE(jsonCast, nullptr);
1335+
jsonCast->castFrom(
1336+
*input, context, rows, resultType, result, /*isFromJson=*/true);
1337+
1338+
auto fieldA = makeArrayVector<std::string>({{"READ", "BLUE"}});
1339+
auto foo = makeFlatVector<StringView>({"Bar"});
1340+
auto expected = makeRowVector({"FieldA", "Foo"}, {fieldA, foo});
1341+
bytedance::bolt::test::assertEqualVectors(expected, result);
1342+
}

‎bolt/functions/prestosql/types/JsonType.cpp‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,7 +1119,9 @@ struct CastFromJsonTypedImpl {
11191119
for (column_index_t numFields = rowType.size(), i = 0; i < numFields;
11201120
++i) {
11211121
rowKey = rowType.nameOf(i);
1122-
boost::algorithm::to_lower(rowKey);
1122+
if (!isFromJson) {
1123+
boost::algorithm::to_lower(rowKey);
1124+
}
11231125
fieldKey2Idx[rowKey] = i;
11241126
appearedIdxs.insert(i);
11251127
}
@@ -1134,12 +1136,22 @@ struct CastFromJsonTypedImpl {
11341136
for (auto fieldResult : object) {
11351137
SIMDJSON_ASSIGN_OR_RAISE(auto field, fieldResult);
11361138
SIMDJSON_ASSIGN_OR_RAISE(jsonKey, field.unescaped_key(true));
1137-
boost::algorithm::to_lower(jsonKey);
1139+
if (!isFromJson) {
1140+
boost::algorithm::to_lower(jsonKey);
1141+
}
11381142
auto it = fieldKey2Idx.find(jsonKey);
11391143
if (it == fieldKey2Idx.end()) {
11401144
continue;
11411145
}
11421146
column_index_t jsonKeyIdx = it->second;
1147+
const bool isFirstOccurrence = appearedIdxs.erase(jsonKeyIdx) > 0;
1148+
if (!isFirstOccurrence) {
1149+
if (isFromJson) {
1150+
continue;
1151+
}
1152+
BOLT_USER_FAIL("Duplicate field: {}", jsonKey);
1153+
}
1154+
11431155
if (!field.value().is_null()) {
11441156
auto err = BOLT_DYNAMIC_TYPE_DISPATCH(
11451157
CastFromJsonTypedImpl<simdjson::ondemand::value>::apply,
@@ -1157,7 +1169,6 @@ struct CastFromJsonTypedImpl {
11571169
} else {
11581170
writerTyped.set_null_at(jsonKeyIdx);
11591171
}
1160-
appearedIdxs.erase(jsonKeyIdx);
11611172
}
11621173

11631174
// set all non appearing columns to null
@@ -1509,7 +1520,7 @@ void JsonCastOperator::castFromJson(
15091520
auto flinkCompatible =
15101521
context.execCtx()->queryCtx()->queryConfig().enableFlinkCompatible();
15111522

1512-
rows.applyToSelected([&](auto row) {
1523+
context.applyToSelectedNoThrow(rows, [&](auto row) {
15131524
writer.setOffset(row);
15141525
if (inputVector->isNullAt(row)) {
15151526
if (flinkCompatible &&

‎bolt/functions/sparksql/specialforms/FromJson.cpp‎

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -224,12 +224,7 @@ struct ExtractJsonTypeImpl {
224224
SIMDJSON_ASSIGN_OR_RAISE(auto object, value.get_object());
225225

226226
const auto& names = rowType.names();
227-
bool allFieldsAreAscii =
228-
std::all_of(names.begin(), names.end(), [](const auto& name) {
229-
return functions::stringCore::isAscii(name.data(), name.size());
230-
});
231-
232-
auto fieldIndices = makeFieldIndicesMap(rowType, allFieldsAreAscii);
227+
auto fieldIndices = makeFieldIndicesMap(rowType);
233228

234229
std::string key;
235230
for (const auto& fieldResult : object) {
@@ -240,11 +235,6 @@ struct ExtractJsonTypeImpl {
240235
if (!field.value().is_null()) {
241236
SIMDJSON_ASSIGN_OR_RAISE(key, field.unescaped_key(true));
242237

243-
if (allFieldsAreAscii) {
244-
folly::toLowerAscii(key);
245-
} else {
246-
boost::algorithm::to_lower(key);
247-
}
248238
auto it = fieldIndices.find(key);
249239
if (it != fieldIndices.end() && it->second >= 0) {
250240
const auto index = it->second;
@@ -354,20 +344,13 @@ struct ExtractJsonTypeImpl {
354344
return simdjson::SUCCESS;
355345
}
356346

357-
// Creates a map of lower case field names to their indices in the row type.
347+
// Creates a map of field names to their indices in the row type.
358348
static folly::F14FastMap<std::string, int32_t> makeFieldIndicesMap(
359-
const RowType& rowType,
360-
bool allFieldsAreAscii) {
349+
const RowType& rowType) {
361350
folly::F14FastMap<std::string, int32_t> fieldIndices;
362351
const auto size = rowType.size();
363352
for (auto i = 0; i < size; ++i) {
364353
std::string key = rowType.nameOf(i);
365-
if (allFieldsAreAscii) {
366-
folly::toLowerAscii(key);
367-
} else {
368-
boost::algorithm::to_lower(key);
369-
}
370-
371354
fieldIndices[key] = i;
372355
}
373356

‎bolt/functions/sparksql/tests/FromJsonTest.cpp‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,5 +283,45 @@ TEST_F(FromJsonTest, scalarRootDocumentDoesNotThrow) {
283283
testFromJson(scalarInput, expectedRow);
284284
}
285285

286+
TEST_F(FromJsonTest, DuplicateCaseInsensitiveFields) {
287+
// Regression test: when a JSON object contains two keys that differ only in
288+
// case (e.g. "FieldA" and "fieldA"), from_json must honor only the first
289+
// match (case-insensitive) and NOT aggregate the two values. A buggy
290+
// implementation reuses the same child writer without resetting it, which
291+
// causes array fields to grow by appending and string fields to be
292+
// concatenated.
293+
{
294+
// json string -> struct<FieldA:array<string>,Foo:string>
295+
const std::string inputJson = R"JSON(
296+
{
297+
"FieldA": ["READ", "BLUE"],
298+
"Foo": "Bar",
299+
"fieldA": ["red", "blue"],
300+
"foo": "bar"
301+
}
302+
)JSON";
303+
auto values =
304+
makeFlatVector<std::string>({inputJson, inputJson, inputJson});
305+
306+
auto fieldAVector = makeArrayVector<std::string>(
307+
{{"READ", "BLUE"}, {"READ", "BLUE"}, {"READ", "BLUE"}});
308+
auto fooVector = makeFlatVector<StringView>({"Bar", "Bar", "Bar"});
309+
auto result = makeRowVector({"FieldA", "Foo"}, {fieldAVector, fooVector});
310+
311+
testFromJson(values, result);
312+
}
313+
}
314+
315+
TEST_F(FromJsonTest, caseSensitiveFields) {
316+
const std::string inputJson = R"({"NAME":"first","age":"second"})";
317+
auto input = makeFlatVector<std::string>({inputJson, inputJson, inputJson});
318+
auto expected = makeRowVector(
319+
{"name"},
320+
{makeNullableFlatVector<StringView>(
321+
{std::nullopt, std::nullopt, std::nullopt})});
322+
323+
testFromJson(input, expected);
324+
}
325+
286326
} // namespace
287327
} // namespace bytedance::bolt::functions::sparksql::test

0 commit comments

Comments
 (0)