This is an automated email from the ASF dual-hosted git repository.
eldenmoon pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new f5bb482e2f9 [improvement](variant) Remove BE configs that change
Variant JSON parse errors (#68447)
f5bb482e2f9 is described below
commit f5bb482e2f9d27c548bcc3824a0e401314ca87d2
Author: lihangyu <[email protected]>
AuthorDate: Mon Oct 19 10:47:02 2026 +0800
[improvement](variant) Remove BE configs that change Variant JSON parse
errors (#68447)
---
be/src/common/config.cpp | 2 -
be/src/common/config.h | 4 -
.../data_type_serde/data_type_variant_v2_serde.cpp | 12 +-
.../exprs/function/parse/variant_string_parse.cpp | 48 +------
be/src/exprs/function/parse/variant_string_parse.h | 6 +-
.../segment/variant/v2/variant_shredder.cpp | 15 +-
.../storage/segment/variant/v2/variant_shredder.h | 1 -
.../segment/variant/variant_writer_helpers.cpp | 2 -
be/test/core/column/column_variant_v2_test.cpp | 14 +-
...type_variant_v2_serde_binary_roundtrip_test.cpp | 4 +-
.../data_type_variant_v2_serde_input_test.cpp | 50 +++----
.../data_type_variant_v2_serde_output_test.cpp | 5 +-
.../function_variant_element_v2_encoded_test.cpp | 5 +-
.../exprs/function/function_variant_parse_test.cpp | 69 +++++----
.../variant/variant_column_writer_reader_test.cpp | 80 +++--------
be/test/testutil/index_storage_test_util.cpp | 4 +-
be/test/testutil/index_storage_test_util.h | 1 -
be/test/util/variant/variant_field_test.cpp | 4 +-
be/test/util/variant/variant_string_parse_test.cpp | 75 ++--------
.../data/variant_p0/duplicate_json_path.json | 3 +-
.../data/variant_p0/duplicate_json_path.out | 12 ++
.../data/variant_p0/load_invalid_json.out | 16 +++
.../data/variant_p0/v2/variant_parse_functions.out | 8 +-
.../suites/variant_p0/column_name.groovy | 15 +-
.../suites/variant_p0/duplicate_json_path.groovy | 157 ++++++++-------------
.../suites/variant_p0/load_invalid_json.groovy | 109 ++++++++++++++
.../suites/variant_p0/v2/variant_compute_v2.groovy | 32 ++---
.../variant_p0/v2/variant_parse_functions.groovy | 85 +++++------
28 files changed, 367 insertions(+), 471 deletions(-)
diff --git a/be/src/common/config.cpp b/be/src/common/config.cpp
index af6755e7f4d..1361137bae7 100644
--- a/be/src/common/config.cpp
+++ b/be/src/common/config.cpp
@@ -1208,8 +1208,6 @@ DEFINE_Bool(enable_debug_points, "false");
DEFINE_Int32(pipeline_executor_size, "0");
DEFINE_Int32(blocking_pipeline_executor_size, "0");
DEFINE_mInt32(variant_max_json_key_length, "255");
-DEFINE_mBool(variant_throw_exeception_on_invalid_json, "false");
-DEFINE_mBool(variant_enable_duplicate_json_path_check, "false");
// Controls storage-layer parse target for plain non-doc VARIANT columns:
// 0 = auto, 1 = force parse-time subcolumns, 2 = force doc-value KV staging.
// NestedGroup, deprecated flatten-nested, and persistent doc mode keep their
required paths.
diff --git a/be/src/common/config.h b/be/src/common/config.h
index c32496630b8..5abf4867249 100644
--- a/be/src/common/config.h
+++ b/be/src/common/config.h
@@ -1628,10 +1628,6 @@ size_t get_lookup_connection_cache_block_pool_size();
DECLARE_mInt64(LZ4_HC_compression_level);
// Max json key length in bytes when parsing json into variant
subcolumns/jsonb.
DECLARE_mInt32(variant_max_json_key_length);
-// Treat invalid json format str as string, instead of throwing exception if
false
-DECLARE_mBool(variant_throw_exeception_on_invalid_json);
-// Enable duplicate path check when parsing json into variant subcolumns/jsonb.
-DECLARE_mBool(variant_enable_duplicate_json_path_check);
// Controls storage-layer parse target for plain non-doc VARIANT columns:
// 0 = auto, 1 = force parse-time subcolumns, 2 = force doc-value KV staging.
DECLARE_mInt32(variant_storage_parse_mode);
diff --git a/be/src/core/data_type_serde/data_type_variant_v2_serde.cpp
b/be/src/core/data_type_serde/data_type_variant_v2_serde.cpp
index 6c183a8eb68..ef2d3a1536c 100644
--- a/be/src/core/data_type_serde/data_type_variant_v2_serde.cpp
+++ b/be/src/core/data_type_serde/data_type_variant_v2_serde.cpp
@@ -586,12 +586,20 @@ void require_jsonb_write(bool written, const char*
operation) {
}
}
+// Most JSON text read here was written by a Variant serde, for example into
an aggregate state, so
+// it must read back every value a Variant can hold, including keys longer
than the parse limit. A
+// CAST from a JSON string to ARRAY<VARIANT> or STRUCT also passes the
elements it splits out.
+JsonToVariantOptions serde_json_options() {
+ return {.max_json_key_length = std::numeric_limits<uint32_t>::max(),
+ .throw_on_invalid_json = false};
+}
+
} // namespace
Status DataTypeVariantV2SerDe::deserialize_one_cell_from_json(IColumn& column,
Slice& slice,
const
FormatOptions&) const {
RETURN_IF_CATCH_EXCEPTION({
- JsonStringToVariantEncoder encoder;
+ JsonStringToVariantEncoder encoder(serde_json_options());
encoder.add_json({slice.data, slice.size});
VariantBatchBuilder block = encoder.finish_batch();
destination(column).insert_encoded_batch(block);
@@ -634,7 +642,7 @@ Status
DataTypeVariantV2SerDe::deserialize_column_from_json_vector(IColumn& colu
if (slices.empty()) {
return Status::OK();
}
- JsonStringToVariantEncoder encoder;
+ JsonStringToVariantEncoder encoder(serde_json_options());
for (const Slice& slice : slices) {
encoder.add_json({slice.data, slice.size});
}
diff --git a/be/src/exprs/function/parse/variant_string_parse.cpp
b/be/src/exprs/function/parse/variant_string_parse.cpp
index 1c24037747b..289b607fdc5 100644
--- a/be/src/exprs/function/parse/variant_string_parse.cpp
+++ b/be/src/exprs/function/parse/variant_string_parse.cpp
@@ -27,7 +27,6 @@
#include <cstdint>
#include <limits>
#include <string_view>
-#include <unordered_set>
#include <utility>
#include "common/config.h"
@@ -171,44 +170,14 @@ private:
void collect_object(const SimdJSONParser::Object& object, uint32_t depth) {
auto scope = _builder.start_object();
- if (!_options.check_duplicate_json_path) {
- for (const auto& [key, child] : object) {
- require_json_key_length(key, _options.max_json_key_length);
- scope.add_key(to_string_ref(key));
- collect(child, depth + 1);
- }
- scope.finish();
- return;
- }
-
- std::unordered_set<std::string_view> seen_keys;
- seen_keys.reserve(object.size());
for (const auto& [key, child] : object) {
require_json_key_length(key, _options.max_json_key_length);
- if (seen_keys.emplace(key).second) {
- scope.add_key(to_string_ref(key));
- collect(child, depth + 1);
- } else {
- validate_ignored(child, depth + 1);
- }
+ scope.add_key(to_string_ref(key));
+ collect(child, depth + 1);
}
scope.finish();
}
- void validate_ignored(SimdJSONParser::Element element, uint32_t depth)
const {
- variant_json::require_json_depth(depth);
- if (element.isArray()) {
- for (SimdJSONParser::Element child : element.getArray()) {
- validate_ignored(child, depth + 1);
- }
- } else if (element.isObject()) {
- for (const auto& [key, child] : element.getObject()) {
- require_json_key_length(key, _options.max_json_key_length);
- validate_ignored(child, depth + 1);
- }
- }
- }
-
VariantBatchBuilder::Row& _builder;
const JsonToVariantOptions& _options;
};
@@ -385,9 +354,7 @@ FormattedScalar format_json_uuid(const std::array<uint8_t,
16>& value) {
} // namespace variant_json
JsonToVariantOptions JsonToVariantOptions::current_config() {
- return {.max_json_key_length =
static_cast<uint32_t>(config::variant_max_json_key_length),
- .throw_on_invalid_json =
config::variant_throw_exeception_on_invalid_json,
- .check_duplicate_json_path =
config::variant_enable_duplicate_json_path_check};
+ return {.max_json_key_length =
static_cast<uint32_t>(config::variant_max_json_key_length)};
}
struct JsonStringToVariantEncoder::Impl {
@@ -417,15 +384,8 @@ struct JsonStringToVariantEncoder::Impl {
throw Exception(ErrorCode::INVALID_ARGUMENT,
"Variant JSON input has a null data pointer");
}
- if (json.size == 0) {
- auto object = row.start_object();
- object.finish();
- row.finish();
- return;
- }
-
SimdJSONParser::Element root;
- if (!parser.parse(json.data, json.size, root)) {
+ if (json.size == 0 || !parser.parse(json.data, json.size, root)) {
if (options.throw_on_invalid_json) {
throw Exception(ErrorCode::INVALID_ARGUMENT, "Failed to parse
JSON as Variant");
}
diff --git a/be/src/exprs/function/parse/variant_string_parse.h
b/be/src/exprs/function/parse/variant_string_parse.h
index 8fe10f73b64..a2e8fc75bde 100644
--- a/be/src/exprs/function/parse/variant_string_parse.h
+++ b/be/src/exprs/function/parse/variant_string_parse.h
@@ -41,8 +41,10 @@ struct VariantJsonFormatOptions {
struct JsonToVariantOptions {
uint32_t max_json_key_length = 255;
- bool throw_on_invalid_json = false;
- bool check_duplicate_json_path = false;
+ // Text that is not a JSON document, including an empty string, fails the
row. The Variant
+ // serde turns this off because it mostly re-reads JSON that a Variant
serde wrote: a value the
+ // parser cannot read back, such as an integer beyond 64 bits, then keeps
its text.
+ bool throw_on_invalid_json = true;
// Takes a value snapshot. Later changes to mutable config do not affect
an encoder.
static JsonToVariantOptions current_config();
diff --git a/be/src/storage/segment/variant/v2/variant_shredder.cpp
b/be/src/storage/segment/variant/v2/variant_shredder.cpp
index fcb4e95aa41..e9164a513d7 100644
--- a/be/src/storage/segment/variant/v2/variant_shredder.cpp
+++ b/be/src/storage/segment/variant/v2/variant_shredder.cpp
@@ -157,16 +157,11 @@ struct VariantShredder::Impl {
PathState& path_state = paths[path_index];
const size_t row_marker = row + 1;
if (path_state.last_row_marker == row_marker) {
- if (!options.check_duplicate_json_path) {
- return Status::InvalidArgument("may contains duplicated entry
: {}",
- path_state.path.get_path());
- }
- return Status::OK();
+ return Status::InvalidArgument("may contains duplicated entry :
{}",
+ path_state.path.get_path());
}
path_state.last_row_marker = row_marker;
- if (value.is_null()) {
- return Status::OK();
- }
+ DCHECK(!value.is_null());
return get_or_create_builder(path_index)->append(value, row);
}
@@ -209,9 +204,9 @@ struct VariantShredder::Impl {
Status visit(VariantRef value, MetadataPathCache& metadata_cache,
PathIndex path_index,
size_t row) {
+ // A null member occupies no path, so {"a":{"b":null},"a.b":3} does
not collide.
if (value.is_null()) {
- return options.check_duplicate_json_path ? append_leaf(value,
path_index, row)
- : Status::OK();
+ return Status::OK();
}
if (value.basic_type() != VariantBasicType::OBJECT) {
return append_leaf(value, path_index, row);
diff --git a/be/src/storage/segment/variant/v2/variant_shredder.h
b/be/src/storage/segment/variant/v2/variant_shredder.h
index 91b8c8b4944..b9bbb14daf9 100644
--- a/be/src/storage/segment/variant/v2/variant_shredder.h
+++ b/be/src/storage/segment/variant/v2/variant_shredder.h
@@ -54,7 +54,6 @@ struct VariantShredderOptions {
BeConsts::DEFAULT_VARIANT_MAX_SPARSE_COLUMN_STATS_SIZE;
uint32_t doc_bucket_count = 1;
size_t doc_materialization_min_rows = 0;
- bool check_duplicate_json_path = false;
};
struct VariantShreddedColumns {
diff --git a/be/src/storage/segment/variant/variant_writer_helpers.cpp
b/be/src/storage/segment/variant/variant_writer_helpers.cpp
index ef7710bc1cc..f5cf7d9ab3b 100644
--- a/be/src/storage/segment/variant/variant_writer_helpers.cpp
+++ b/be/src/storage/segment/variant/variant_writer_helpers.cpp
@@ -25,7 +25,6 @@
#include <utility>
#include "common/cast_set.h"
-#include "common/config.h"
#include "common/exception.h"
#include "core/assert_cast.h"
#include "core/column/column_nullable.h"
@@ -86,7 +85,6 @@ Status make_variant_shredder_options(const TabletSchema&
tablet_schema,
cast_set<uint32_t>(std::max(1,
parent_column.variant_doc_hash_shard_count())),
.doc_materialization_min_rows = cast_set<size_t>(
std::max<int64_t>(0,
parent_column.variant_doc_materialization_min_rows())),
- .check_duplicate_json_path =
config::variant_enable_duplicate_json_path_check,
};
*options = std::move(result);
return Status::OK();
diff --git a/be/test/core/column/column_variant_v2_test.cpp
b/be/test/core/column/column_variant_v2_test.cpp
index 0a20eafca37..fa1cb3dc289 100644
--- a/be/test/core/column/column_variant_v2_test.cpp
+++ b/be/test/core/column/column_variant_v2_test.cpp
@@ -81,9 +81,7 @@ namespace {
using MetaIdsColumn = ColumnVector<TYPE_UINT32>;
VariantField encode_json(std::string_view json) {
- JsonStringToVariantEncoder encoder({.max_json_key_length = 255,
- .throw_on_invalid_json = true,
- .check_duplicate_json_path = false});
+ JsonStringToVariantEncoder encoder({.max_json_key_length = 255,
.throw_on_invalid_json = true});
encoder.add_json({json.data(), json.size()});
VariantBatchBuilder block = encoder.finish_batch();
return VariantField::from_ref(block.value_at(0));
@@ -1481,9 +1479,8 @@ TEST(ColumnVariantV2Test,
InsertsCodecOwnedBatchesDirectly) {
}
{
- JsonStringToVariantEncoder encoder({.max_json_key_length = 255,
- .throw_on_invalid_json = true,
- .check_duplicate_json_path =
false});
+ JsonStringToVariantEncoder encoder(
+ {.max_json_key_length = 255, .throw_on_invalid_json = true});
encoder.add_json(StringRef("7", 1));
VariantBatchBuilder block = encoder.finish_batch();
auto column = ColumnVariantV2::create();
@@ -1493,9 +1490,8 @@ TEST(ColumnVariantV2Test,
InsertsCodecOwnedBatchesDirectly) {
}
{
- JsonStringToVariantEncoder encoder({.max_json_key_length = 255,
- .throw_on_invalid_json = true,
- .check_duplicate_json_path =
false});
+ JsonStringToVariantEncoder encoder(
+ {.max_json_key_length = 255, .throw_on_invalid_json = true});
constexpr std::string_view FIRST = R"({"a":[1,{"b":true}]})";
constexpr std::string_view SECOND = R"({"b":2,"a":[]})";
encoder.add_json({FIRST.data(), FIRST.size()});
diff --git
a/be/test/core/data_type_serde/data_type_variant_v2_serde_binary_roundtrip_test.cpp
b/be/test/core/data_type_serde/data_type_variant_v2_serde_binary_roundtrip_test.cpp
index 006007f3f90..4474b62969b 100644
---
a/be/test/core/data_type_serde/data_type_variant_v2_serde_binary_roundtrip_test.cpp
+++
b/be/test/core/data_type_serde/data_type_variant_v2_serde_binary_roundtrip_test.cpp
@@ -62,9 +62,7 @@ namespace doris {
namespace {
VariantField encode_json(std::string_view json) {
- JsonStringToVariantEncoder encoder({.max_json_key_length = 255,
- .throw_on_invalid_json = true,
- .check_duplicate_json_path = false});
+ JsonStringToVariantEncoder encoder({.max_json_key_length = 255,
.throw_on_invalid_json = true});
encoder.add_json({json.data(), json.size()});
VariantBatchBuilder block = encoder.finish_batch();
return VariantField::from_ref(block.value_at(0));
diff --git
a/be/test/core/data_type_serde/data_type_variant_v2_serde_input_test.cpp
b/be/test/core/data_type_serde/data_type_variant_v2_serde_input_test.cpp
index 2bb8b74691c..9d126b93231 100644
--- a/be/test/core/data_type_serde/data_type_variant_v2_serde_input_test.cpp
+++ b/be/test/core/data_type_serde/data_type_variant_v2_serde_input_test.cpp
@@ -22,7 +22,6 @@
#include <string_view>
#include <vector>
-#include "common/config.h"
#include "core/column/column_nullable.h"
#include "core/column/column_string.h"
#include "core/column/column_vector.h"
@@ -37,18 +36,6 @@
namespace doris {
namespace {
-class ScopedInvalidJsonMode {
-public:
- explicit ScopedInvalidJsonMode(bool value)
- : _old(config::variant_throw_exeception_on_invalid_json) {
- config::variant_throw_exeception_on_invalid_json = value;
- }
- ~ScopedInvalidJsonMode() {
config::variant_throw_exeception_on_invalid_json = _old; }
-
-private:
- bool _old;
-};
-
Status deserialize_json(const DataTypeVariantV2SerDe& serde, ColumnVariantV2&
column,
std::string_view text) {
Slice slice(text.data(), text.size());
@@ -91,30 +78,26 @@ ColumnVariantV2::MutablePtr typed_int(int32_t value) {
TEST(DataTypeVariantV2SerdeInputTest, JsonUsesT15ValidEmptyAndInvalidPolicies)
{
DataTypeVariantV2SerDe serde;
auto column = ColumnVariantV2::create();
- {
- ScopedInvalidJsonMode mode(false);
- EXPECT_TRUE(deserialize_json(serde, *column,
R"({"b":[1,null]})").ok());
- EXPECT_TRUE(deserialize_json(serde, *column, {}).ok());
- EXPECT_TRUE(deserialize_json(serde, *column, "not-json").ok());
- }
- ASSERT_EQ(column->size(), 3);
+ // The serde re-reads JSON that it wrote, so text the parser rejects keeps
its text instead of
+ // failing, for example an integer beyond 64 bits in a serialized
aggregate state.
+ EXPECT_TRUE(deserialize_json(serde, *column, R"({"b":[1,null]})").ok());
+ EXPECT_TRUE(deserialize_json(serde, *column, {}).ok());
+ EXPECT_TRUE(deserialize_json(serde, *column, "not-json").ok());
+ EXPECT_TRUE(deserialize_json(serde, *column, "18446744073709551616").ok());
+ ASSERT_EQ(column->size(), 4);
EXPECT_EQ(json_at(serde, *column, 0), R"({"b":[1,null]})");
- EXPECT_EQ(json_at(serde, *column, 1), "{}");
+ EXPECT_EQ(json_at(serde, *column, 1), R"("")");
EXPECT_EQ(json_at(serde, *column, 2), R"("not-json")");
+ EXPECT_EQ(json_at(serde, *column, 3), R"("18446744073709551616")");
- const size_t before = column->size();
- {
- ScopedInvalidJsonMode mode(true);
- EXPECT_EQ(deserialize_json(serde, *column, "not-json").code(),
ErrorCode::INVALID_ARGUMENT);
- }
- EXPECT_EQ(column->size(), before);
+ const std::string long_key(300, 'k');
+ const std::string long_key_json = R"({")" + long_key + R"(":1})";
+ ASSERT_TRUE(deserialize_json(serde, *column, long_key_json).ok());
+ EXPECT_EQ(json_at(serde, *column, 4), long_key_json);
+ const size_t before = column->size();
const std::string invalid_utf8("\xC3\x28", 2);
- {
- ScopedInvalidJsonMode mode(false);
- EXPECT_EQ(deserialize_json(serde, *column, invalid_utf8).code(),
- ErrorCode::INVALID_ARGUMENT);
- }
+ EXPECT_EQ(deserialize_json(serde, *column, invalid_utf8).code(),
ErrorCode::INVALID_ARGUMENT);
EXPECT_EQ(column->size(), before);
}
@@ -125,11 +108,10 @@ TEST(DataTypeVariantV2SerdeInputTest,
VectorFailureIsAtomicAndCounterIsNotAdvanc
const size_t before = column->size();
std::string valid = R"({"a":1})";
- std::string invalid = "{";
+ std::string invalid("\xC3\x28", 2);
std::vector<Slice> slices {{valid.data(), valid.size()}, {invalid.data(),
invalid.size()}};
uint64_t num_deserialized = 0;
DataTypeSerDe::FormatOptions options;
- ScopedInvalidJsonMode mode(true);
const Status status =
serde.deserialize_column_from_json_vector(*column, slices,
&num_deserialized, options);
EXPECT_EQ(status.code(), ErrorCode::INVALID_ARGUMENT);
diff --git
a/be/test/core/data_type_serde/data_type_variant_v2_serde_output_test.cpp
b/be/test/core/data_type_serde/data_type_variant_v2_serde_output_test.cpp
index 10697c9645c..bf9be8ac2f2 100644
--- a/be/test/core/data_type_serde/data_type_variant_v2_serde_output_test.cpp
+++ b/be/test/core/data_type_serde/data_type_variant_v2_serde_output_test.cpp
@@ -99,9 +99,8 @@ ColumnPtr encoded_copy(const ColumnVariantV2& typed) {
}
ColumnVariantV2::MutablePtr
encoded_json(std::initializer_list<std::string_view> rows) {
- JsonStringToVariantEncoder encoder({.max_json_key_length = 1024,
- .throw_on_invalid_json = true,
- .check_duplicate_json_path = false});
+ JsonStringToVariantEncoder encoder(
+ {.max_json_key_length = 1024, .throw_on_invalid_json = true});
for (std::string_view row : rows) {
encoder.add_json({row.data(), row.size()});
}
diff --git
a/be/test/exprs/function/function_variant_element_v2_encoded_test.cpp
b/be/test/exprs/function/function_variant_element_v2_encoded_test.cpp
index cd00f159121..39cba07078f 100644
--- a/be/test/exprs/function/function_variant_element_v2_encoded_test.cpp
+++ b/be/test/exprs/function/function_variant_element_v2_encoded_test.cpp
@@ -35,9 +35,8 @@ namespace {
using Segment = VariantElementV2PathSegment;
VariantField encode_json(std::string_view json) {
- JsonStringToVariantEncoder encoder({.max_json_key_length = 1024,
- .throw_on_invalid_json = true,
- .check_duplicate_json_path = false});
+ JsonStringToVariantEncoder encoder(
+ {.max_json_key_length = 1024, .throw_on_invalid_json = true});
encoder.add_json({json.data(), json.size()});
VariantBatchBuilder block = encoder.finish_batch();
return VariantField::from_ref(block.value_at(0));
diff --git a/be/test/exprs/function/function_variant_parse_test.cpp
b/be/test/exprs/function/function_variant_parse_test.cpp
index 9cdfcbccc7c..7481dd17b43 100644
--- a/be/test/exprs/function/function_variant_parse_test.cpp
+++ b/be/test/exprs/function/function_variant_parse_test.cpp
@@ -177,7 +177,6 @@ TEST(FunctionVariantParseTest,
PreservesSqlNullAndErrorToNull) {
const DataTypePtr string_type = std::make_shared<DataTypeString>();
const std::string invalid_utf8(1, static_cast<char>(0xFF));
- ScopedValue strict(config::variant_throw_exeception_on_invalid_json, true);
ExecutionResult failure = execute_parse(
"parse_to_variant", make_strings({R"({"before":1})", invalid_utf8,
R"({"after":2})"}),
string_type, 3);
@@ -248,15 +247,22 @@ TEST(FunctionVariantParseTest,
ConfiguredVariantReturnTypeBuildsAndExecutes) {
TEST(FunctionVariantParseTest, DistinguishesSqlNullJsonNullEmptyAndConst) {
const DataTypePtr string_type = std::make_shared<DataTypeString>();
- ExecutionResult values = execute_parse(
- "parse_to_variant", make_strings({R"({"a":1})", "null", ""}),
string_type, 3);
+ ExecutionResult values =
+ execute_parse("parse_to_variant", make_strings({R"({"a":1})",
"null"}), string_type, 2);
ASSERT_TRUE(values.status.ok()) << values.status.to_string();
- ASSERT_EQ(values.output->size(), 3);
+ ASSERT_EQ(values.output->size(), 2);
EXPECT_FALSE(is_sql_null_at(values.output, 0));
EXPECT_FALSE(is_sql_null_at(values.output, 1));
EXPECT_EQ(variant_json_at(values.output, 0), R"({"a":1})");
EXPECT_EQ(variant_json_at(values.output, 1), "null");
- EXPECT_EQ(variant_json_at(values.output, 2), "{}");
+
+ // An empty string is not a JSON document.
+ ExecutionResult empty = execute_parse("parse_to_variant",
make_strings({""}), string_type, 1);
+ EXPECT_EQ(empty.status.code(), ErrorCode::INVALID_ARGUMENT) <<
empty.status.to_string();
+ ExecutionResult empty_to_null =
+ execute_parse("try_parse_to_variant", make_strings({""}),
string_type, 1);
+ ASSERT_TRUE(empty_to_null.status.ok()) << empty_to_null.status.to_string();
+ EXPECT_TRUE(is_sql_null_at(empty_to_null.output, 0));
const DataTypePtr nullable_string_type =
make_nullable(std::make_shared<DataTypeString>());
ExecutionResult nullable = execute_parse(
@@ -279,7 +285,6 @@ TEST(FunctionVariantParseTest,
DistinguishesSqlNullJsonNullEmptyAndConst) {
}
TEST(FunctionVariantParseTest, StrictFailureDoesNotPublishPartialBatch) {
- ScopedValue strict(config::variant_throw_exeception_on_invalid_json, true);
const DataTypePtr string_type = std::make_shared<DataTypeString>();
ExecutionResult result =
execute_parse("parse_to_variant",
@@ -297,26 +302,17 @@ TEST(FunctionVariantParseTest,
StrictFailureDoesNotPublishPartialBatch) {
TEST(FunctionVariantParseTest, ErrorToNullOnlyNullsRecoverableFailures) {
const DataTypePtr string_type = std::make_shared<DataTypeString>();
- {
- ScopedValue strict(config::variant_throw_exeception_on_invalid_json,
true);
- ExecutionResult result = execute_parse(
- "try_parse_to_variant",
- make_strings({R"({"before":1})", "{", "null",
R"({"after":2})"}), string_type, 4);
- ASSERT_TRUE(result.status.ok()) << result.status.to_string();
- EXPECT_FALSE(is_sql_null_at(result.output, 0));
- EXPECT_TRUE(is_sql_null_at(result.output, 1));
- EXPECT_FALSE(is_sql_null_at(result.output, 2));
- EXPECT_EQ(variant_json_at(result.output, 2), "null");
- EXPECT_EQ(variant_json_at(result.output, 3), R"({"after":2})");
- }
- {
- ScopedValue
permissive(config::variant_throw_exeception_on_invalid_json, false);
- ExecutionResult result =
- execute_parse("try_parse_to_variant", make_strings({"{"}),
string_type, 1);
- ASSERT_TRUE(result.status.ok()) << result.status.to_string();
- EXPECT_FALSE(is_sql_null_at(result.output, 0));
- EXPECT_EQ(variant_json_at(result.output, 0), R"("{")");
- }
+ ExecutionResult result =
+ execute_parse("try_parse_to_variant",
+ make_strings({R"({"before":1})", "{", "null",
"hello", R"({"after":2})"}),
+ string_type, 5);
+ ASSERT_TRUE(result.status.ok()) << result.status.to_string();
+ EXPECT_FALSE(is_sql_null_at(result.output, 0));
+ EXPECT_TRUE(is_sql_null_at(result.output, 1));
+ EXPECT_FALSE(is_sql_null_at(result.output, 2));
+ EXPECT_EQ(variant_json_at(result.output, 2), "null");
+ EXPECT_TRUE(is_sql_null_at(result.output, 3));
+ EXPECT_EQ(variant_json_at(result.output, 4), R"({"after":2})");
}
TEST(FunctionVariantParseTest, ConfiguredInputValidationUsesFailOrOuterNull) {
@@ -333,7 +329,6 @@ TEST(FunctionVariantParseTest,
ConfiguredInputValidationUsesFailOrOuterNull) {
EXPECT_TRUE(is_sql_null_at(null.output, 0));
}
{
- ScopedValue
reject_duplicates(config::variant_enable_duplicate_json_path_check, false);
ExecutionResult fail = execute_parse("parse_to_variant",
make_strings({R"({"a":1,"a":2})"}),
string_type, 1);
EXPECT_FALSE(fail.status.ok());
@@ -343,15 +338,6 @@ TEST(FunctionVariantParseTest,
ConfiguredInputValidationUsesFailOrOuterNull) {
EXPECT_TRUE(is_sql_null_at(null.output, 0));
}
{
- ScopedValue
keep_first(config::variant_enable_duplicate_json_path_check, true);
- ExecutionResult result = execute_parse("try_parse_to_variant",
-
make_strings({R"({"a":1,"a":2})"}), string_type, 1);
- ASSERT_TRUE(result.status.ok()) << result.status.to_string();
- EXPECT_FALSE(is_sql_null_at(result.output, 0));
- EXPECT_EQ(variant_json_at(result.output, 0), R"({"a":1})");
- }
- {
- ScopedValue
permissive(config::variant_throw_exeception_on_invalid_json, false);
const std::string invalid_utf8(1, static_cast<char>(0xFF));
ExecutionResult fail =
execute_parse("parse_to_variant",
make_strings({invalid_utf8}), string_type, 1);
@@ -361,6 +347,17 @@ TEST(FunctionVariantParseTest,
ConfiguredInputValidationUsesFailOrOuterNull) {
ASSERT_TRUE(null.status.ok()) << null.status.to_string();
EXPECT_TRUE(is_sql_null_at(null.output, 0));
}
+ {
+ // The DOM parser cannot read an integer beyond 64 bits, so the
document is invalid JSON.
+ ExecutionResult fail = execute_parse(
+ "parse_to_variant", make_strings({"18446744073709551616"}),
string_type, 1);
+ EXPECT_FALSE(fail.status.ok());
+ ExecutionResult null =
+ execute_parse("try_parse_to_variant",
+ make_strings({R"({"n":18446744073709551616})"}),
string_type, 1);
+ ASSERT_TRUE(null.status.ok()) << null.status.to_string();
+ EXPECT_TRUE(is_sql_null_at(null.output, 0));
+ }
{
const std::string too_deep =
nested_array_json(VARIANT_MAX_NESTING_DEPTH + 1);
ExecutionResult fail =
diff --git a/be/test/storage/variant/variant_column_writer_reader_test.cpp
b/be/test/storage/variant/variant_column_writer_reader_test.cpp
index a5a011f84b4..423152955ed 100644
--- a/be/test/storage/variant/variant_column_writer_reader_test.cpp
+++ b/be/test/storage/variant/variant_column_writer_reader_test.cpp
@@ -121,20 +121,6 @@ static std::string variant_writer_test_name(
return variant_writer_input_name(info.param);
}
-class ScopedDuplicateJsonPathCheck {
-public:
- explicit ScopedDuplicateJsonPathCheck(bool enabled)
- : _old_value(config::variant_enable_duplicate_json_path_check) {
- config::variant_enable_duplicate_json_path_check = enabled;
- }
- ~ScopedDuplicateJsonPathCheck() {
- config::variant_enable_duplicate_json_path_check = _old_value;
- }
-
-private:
- bool _old_value;
-};
-
static Status create_variant_writer_source(VariantWriterInput input,
const std::vector<std::string>&
jsons,
int max_subcolumns_count, bool
enable_doc_mode,
@@ -2168,7 +2154,6 @@ TEST_P(VariantWriterCompatibilityTest,
materialized_array_preserves_middle_row_g
}
TEST_P(VariantWriterCompatibilityTest,
dotted_and_nested_paths_follow_writer_semantics) {
- ScopedDuplicateJsonPathCheck duplicate_path_check(false);
init_variant_tablet(10015 + static_cast<int>(GetParam()), 2);
const std::vector<std::string> jsons {
@@ -2201,11 +2186,15 @@ TEST_P(VariantWriterCompatibilityTest,
dotted_and_nested_paths_follow_writer_sem
}));
}
-TEST_P(VariantWriterCompatibilityTest,
dotted_null_path_is_first_when_duplicate_check_enabled) {
- ScopedDuplicateJsonPathCheck duplicate_path_check(true);
+TEST_P(VariantWriterCompatibilityTest,
dotted_and_nested_paths_without_collision_are_written) {
init_variant_tablet(10019 + static_cast<int>(GetParam()), 2);
- const std::vector<std::string> jsons {R"({"a":{"b":null},"a.b":5})"};
+ // A null member occupies no path, and a dotted key whose children differ
from the nested
+ // object's children reaches different paths, so neither row collides.
+ const std::vector<std::string> jsons {
+ R"({"a.b":null,"a":{"b":5}})",
+ R"({"a.b":{"c":8},"a":{"b":{"d":9}}})",
+ };
ColumnPtr source;
DataTypePtr source_type;
ASSERT_TRUE(create_variant_writer_source(GetParam(), jsons, 2, false, {},
&source, &source_type)
@@ -2214,12 +2203,17 @@ TEST_P(VariantWriterCompatibilityTest,
dotted_null_path_is_first_when_duplicate_
SegmentFooterPB footer;
std::string file_path;
const std::string rowset_id =
- "shared_dotted_null_first_" +
variant_writer_input_name(GetParam());
- ASSERT_TRUE(write_variant_segment(source, source_type, rowset_id, &footer,
&file_path).ok());
+ "shared_dotted_without_collision_" +
variant_writer_input_name(GetParam());
+ const Status write_status =
+ write_variant_segment(source, source_type, rowset_id, &footer,
&file_path);
+ ASSERT_TRUE(write_status.ok()) << write_status;
std::vector<std::optional<std::string>> actual;
ASSERT_TRUE(read_variant_root_rows(footer, file_path, &actual).ok());
- EXPECT_EQ(actual, (std::vector<std::optional<std::string>> {"{}"}));
+ EXPECT_EQ(actual, (std::vector<std::optional<std::string>> {
+ R"({"a":{"b":5}})",
+ R"({"a":{"b":{"c":8,"d":9}}})",
+ }));
}
TEST_P(VariantWriterCompatibilityTest, nullable_round_trip) {
@@ -2301,47 +2295,7 @@ TEST_P(VariantWriterCompatibilityTest,
doc_mode_round_trip) {
}
}
-TEST_F(VariantColumnWriterReaderTest,
test_write_column_variant_v2_duplicate_dotted_paths) {
- ScopedDuplicateJsonPathCheck duplicate_path_check(true);
- init_variant_tablet(10003, 2);
-
- const std::vector<std::string> jsons {
- R"({"a.b":1,"a":{"b":2}})",
- R"({"a":{"b":3},"a.b":4})",
- R"({"a.b":null,"a":{"b":5}})",
- R"({"a.b":6})",
- R"({"a":{"b":7}})",
- R"({"a.b":{"c":8},"a":{"b":{"d":9}}})",
- };
- auto source = ColumnVariantV2::create();
- DataTypeVariantV2SerDe serde;
- DataTypeSerDe::FormatOptions serde_options;
- for (const auto& json : jsons) {
- Slice slice(json.data(), json.size());
- ASSERT_TRUE(serde.deserialize_one_cell_from_json(*source, slice,
serde_options).ok());
- }
-
- SegmentFooterPB footer;
- std::string file_path;
- const auto source_type = std::make_shared<DataTypeVariantV2>(2, false);
- const Status write_status = write_variant_segment(source->get_ptr(),
source_type,
- "v2_duplicate_paths",
&footer, &file_path, 3);
- ASSERT_TRUE(write_status.ok()) << write_status;
-
- std::vector<std::optional<std::string>> actual;
- ASSERT_TRUE(read_variant_root_rows(footer, file_path, &actual).ok());
- // ColumnVariantV2 stores object entries in metadata field-id order, so
the nested form is the
- // first canonical leaf for a dotted-path collision regardless of the
source JSON key order.
- const std::vector<std::optional<std::string>> expected {
- R"({"a":{"b":2}})", R"({"a":{"b":3}})", R"({"a":{"b":5}})",
- R"({"a":{"b":6}})", R"({"a":{"b":7}})",
R"({"a":{"b":{"c":8,"d":9}}})",
- };
- EXPECT_EQ(actual, expected);
-}
-
-TEST_F(VariantColumnWriterReaderTest,
-
test_write_column_variant_v2_rejects_duplicate_dotted_paths_without_check) {
- ScopedDuplicateJsonPathCheck duplicate_path_check(false);
+TEST_F(VariantColumnWriterReaderTest,
test_write_column_variant_v2_rejects_duplicate_dotted_paths) {
init_variant_tablet(10008, 2);
auto source = ColumnVariantV2::create();
@@ -3339,7 +3293,6 @@ TEST_F(VariantColumnWriterReaderTest,
v2_shredder_uses_only_rows_in_requested_ra
segment_v2::VariantShredder shredder({
.max_subcolumns_count = 1,
.sparse_bucket_count = 1,
- .check_duplicate_json_path =
config::variant_enable_duplicate_json_path_check,
});
ASSERT_TRUE(shredder.append(combined->read_view(), 1, 1).ok());
segment_v2::VariantShreddedColumns shredded;
@@ -3416,7 +3369,6 @@ TEST_F(VariantColumnWriterReaderTest,
v2_shredder_drops_typed_cast_null_from_spa
.max_subcolumns_count = 1,
.typed_paths_to_sparse = true,
.sparse_bucket_count = 1,
- .check_duplicate_json_path =
config::variant_enable_duplicate_json_path_check,
});
ASSERT_TRUE(shredder.append(source->read_view(), 0, source->size()).ok());
segment_v2::VariantShreddedColumns shredded;
diff --git a/be/test/testutil/index_storage_test_util.cpp
b/be/test/testutil/index_storage_test_util.cpp
index 944dd568b33..ee7fa90ee04 100644
--- a/be/test/testutil/index_storage_test_util.cpp
+++ b/be/test/testutil/index_storage_test_util.cpp
@@ -406,9 +406,7 @@ Status fill_variant_column(const VariantColumnSpec&
column_spec, const IndexBatc
size_t column_pos, MutableColumnPtr* output) {
ensure_variant_json_shape(batch, column_pos);
auto variant_column = ColumnVariantV2::create();
- JsonStringToVariantEncoder encoder(
- {.throw_on_invalid_json = true,
- .check_duplicate_json_path = batch.check_duplicate_json_path});
+ JsonStringToVariantEncoder encoder({.throw_on_invalid_json = true});
for (const auto& json : batch.variant_jsons_by_column[column_pos]) {
encoder.add_json({json.data(), json.size()});
}
diff --git a/be/test/testutil/index_storage_test_util.h
b/be/test/testutil/index_storage_test_util.h
index 873ce5db4fc..4715fa897d4 100644
--- a/be/test/testutil/index_storage_test_util.h
+++ b/be/test/testutil/index_storage_test_util.h
@@ -115,7 +115,6 @@ struct IndexBatch {
std::vector<std::vector<std::string>> variant_jsons_by_column;
std::vector<ColumnPtr> variant_columns_by_column;
bool deprecated_enable_flatten_nested = false;
- bool check_duplicate_json_path = false;
static IndexBatch single_text(std::vector<std::string> values, int32_t
first_key = 0);
static IndexBatch single_variant(std::vector<std::string> jsons, int32_t
first_key = 0);
diff --git a/be/test/util/variant/variant_field_test.cpp
b/be/test/util/variant/variant_field_test.cpp
index 9e932f784f2..ef90da9d01a 100644
--- a/be/test/util/variant/variant_field_test.cpp
+++ b/be/test/util/variant/variant_field_test.cpp
@@ -213,9 +213,7 @@ void expect_encode_and_decode_preservation(const
std::string& metadata_bytes,
}
VariantField encode_json(std::string_view json) {
- JsonStringToVariantEncoder encoder({.max_json_key_length = 255,
- .throw_on_invalid_json = true,
- .check_duplicate_json_path = false});
+ JsonStringToVariantEncoder encoder({.max_json_key_length = 255,
.throw_on_invalid_json = true});
encoder.add_json({json.data(), json.size()});
VariantBatchBuilder block = encoder.finish_batch();
return VariantField::from_ref(block.value_at(0));
diff --git a/be/test/util/variant/variant_string_parse_test.cpp
b/be/test/util/variant/variant_string_parse_test.cpp
index d34f5057395..b3cb5db74bb 100644
--- a/be/test/util/variant/variant_string_parse_test.cpp
+++ b/be/test/util/variant/variant_string_parse_test.cpp
@@ -280,64 +280,16 @@ TEST(VariantJsonTest,
EncoderCopiesRowsBeforeParserReuseAndProducesCanonicalValu
EXPECT_TRUE(canonical_equals(block.value_at(0), reparsed.value_at(0)));
}
-TEST(VariantJsonTest, DuplicateKeysKeepFirstCompleteSubtreeWhenEnabled) {
- JsonToVariantOptions options;
- options.check_duplicate_json_path = true;
- VariantBatchBuilder block = encode_jsons(
- {R"({"a":1,"a":[2]})", R"({"a":[1],"a":2})",
R"({"a":{"b":1},"a":{"c":2}})"}, options);
- ASSERT_EQ(block.num_rows(), 3);
- EXPECT_EQ(print_json(block.value_at(0)), R"({"a":1})");
- EXPECT_EQ(print_json(block.value_at(1)), R"({"a":[1]})");
- EXPECT_EQ(print_json(block.value_at(2)), R"({"a":{"b":1}})");
-
- // This is member-level first-wins semantics. It deliberately removes the
old PathInData
- // flatten/merge artifact; it is not evidence that the broader T0.2
comparison is complete.
- VariantBatchBuilder dotted = encode_jsons({R"({"a.b":1,"a":{"b":2}})"},
options);
- EXPECT_EQ(print_json(dotted.value_at(0)), R"({"a":{"b":2},"a.b":1})");
-}
-
-TEST(VariantJsonTest, DuplicateKeysFailWithoutPerObjectHashWhenDisabled) {
- JsonToVariantOptions options;
- options.check_duplicate_json_path = false;
- expect_exception_code(ErrorCode::INVALID_ARGUMENT,
- [&] {
static_cast<void>(encode_jsons({R"({"a":1,"a":2})"}, options)); });
-}
-
-TEST(VariantJsonTest,
RepeatedDuplicateSchemasStayStableAcrossSchemaAndScalarRows) {
- JsonToVariantOptions options;
- options.check_duplicate_json_path = true;
- VariantBatchBuilder block =
- encode_jsons({R"({"a":1,"a":2})", R"({"a":3,"a":4})", "null",
R"({"a":5,"a":6})",
- R"({"b":7,"b":8})", R"({"b":9,"b":10})"},
- options);
- ASSERT_EQ(block.num_rows(), 6);
- EXPECT_EQ(print_json(block.value_at(0)), R"({"a":1})");
- EXPECT_EQ(print_json(block.value_at(1)), R"({"a":3})");
- EXPECT_TRUE(block.value_at(2).is_null());
- EXPECT_EQ(print_json(block.value_at(3)), R"({"a":5})");
- EXPECT_EQ(print_json(block.value_at(4)), R"({"b":7})");
- EXPECT_EQ(print_json(block.value_at(5)), R"({"b":9})");
- std::vector<VariantRef> rows;
- rows.reserve(block.num_rows());
- for (size_t index = 0; index < block.num_rows(); ++index) {
- rows.push_back(block.value_at(index));
+TEST(VariantJsonTest, DuplicateKeysFail) {
+ for (const std::string_view json :
+ {R"({"a":1,"a":2})", R"({"a":1,"a":[2]})",
R"({"a":{"b":1},"a":{"c":2}})"}) {
+ expect_exception_code(ErrorCode::INVALID_ARGUMENT,
+ [&] { static_cast<void>(encode_jsons({json}));
});
}
- validate_canonical(block.metadata_ref(), rows);
-}
-
-TEST(VariantJsonTest, IgnoredDuplicateSubtreeStillValidatesKeysAndDepth) {
- JsonToVariantOptions options;
- options.max_json_key_length = 255;
- options.check_duplicate_json_path = true;
- const std::string oversized_key(256, 'k');
- const std::string oversized = R"({"a":1,"a":{")" + oversized_key +
R"(":2}})";
- expect_exception_code(ErrorCode::INVALID_ARGUMENT,
- [&] { static_cast<void>(encode_jsons({oversized},
options)); });
- const std::string too_deep =
- R"({"a":1,"a":)" + nested_array_json(VARIANT_MAX_NESTING_DEPTH) +
"}";
- expect_exception_code(ErrorCode::INVALID_ARGUMENT,
- [&] { static_cast<void>(encode_jsons({too_deep},
options)); });
+ // A dotted key and a nested key are different members of a parsed value.
+ VariantBatchBuilder dotted = encode_jsons({R"({"a.b":1,"a":{"b":2}})"});
+ EXPECT_EQ(print_json(dotted.value_at(0)), R"({"a":{"b":2},"a.b":1})");
}
TEST(VariantJsonTest, KeyLengthBoundaryIsMeasuredInUtf8Bytes) {
@@ -357,13 +309,15 @@ TEST(VariantJsonTest,
EmptyAndInvalidInputFollowExplicitPolicy) {
permissive.throw_on_invalid_json = false;
VariantBatchBuilder block = encode_jsons({std::string_view {}, "{"},
permissive);
ASSERT_EQ(block.num_rows(), 2);
- EXPECT_EQ(print_json(block.value_at(0)), "{}");
+ EXPECT_EQ(print_json(block.value_at(0)), R"("")");
EXPECT_EQ(print_json(block.value_at(1)), R"("{")");
- JsonToVariantOptions strict;
- strict.throw_on_invalid_json = true;
+ const JsonToVariantOptions strict;
+ EXPECT_TRUE(strict.throw_on_invalid_json);
expect_exception_code(ErrorCode::INVALID_ARGUMENT,
[&] { static_cast<void>(encode_jsons({"{"},
strict)); });
+ expect_exception_code(ErrorCode::INVALID_ARGUMENT,
+ [&] {
static_cast<void>(encode_jsons({std::string_view {}}, strict)); });
const std::string invalid_utf8(1, static_cast<char>(0xFF));
expect_exception_code(ErrorCode::INVALID_ARGUMENT, [&] {
@@ -518,8 +472,7 @@ TEST(VariantJsonTest,
CurrentConfigIsSnapshottedAndExplicitOptionsAreValidated)
const JsonToVariantOptions options =
JsonToVariantOptions::current_config();
EXPECT_EQ(options.max_json_key_length,
static_cast<uint32_t>(config::variant_max_json_key_length));
- EXPECT_EQ(options.throw_on_invalid_json,
config::variant_throw_exeception_on_invalid_json);
- EXPECT_EQ(options.check_duplicate_json_path,
config::variant_enable_duplicate_json_path_check);
+ EXPECT_TRUE(options.throw_on_invalid_json);
JsonToVariantOptions invalid;
invalid.max_json_key_length = 0;
diff --git a/regression-test/data/variant_p0/duplicate_json_path.json
b/regression-test/data/variant_p0/duplicate_json_path.json
index e065c9b2331..378e90371fd 100644
--- a/regression-test/data/variant_p0/duplicate_json_path.json
+++ b/regression-test/data/variant_p0/duplicate_json_path.json
@@ -1,7 +1,6 @@
{"k":8,"v":{"a":42,"a":{"b":42}}}
{"k":9,"v":{"a":123,"a":"123"}}
-{"k":10,"v":{"a.b":8,"a":{"b":9}}}
-{"k":11,"v":{"a":{"b":10},"a.b":11}}
{"k":12,"v":{"a":{"b":11},"a":{"c":12}}}
{"k":13,"v":{"a":[13],"a":14}}
{"k":14,"v":{"a":14,"a":[13]}}
+{"k":15,"v":{"a":{"b":15},"c":16}}
diff --git a/regression-test/data/variant_p0/duplicate_json_path.out
b/regression-test/data/variant_p0/duplicate_json_path.out
new file mode 100644
index 00000000000..fd763ded85d
--- /dev/null
+++ b/regression-test/data/variant_p0/duplicate_json_path.out
@@ -0,0 +1,12 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !rows --
+12 \N true
+13 \N true
+14 \N true
+15 {"a":{"b":15},"c":16} false
+2 \N true
+3 \N true
+4 {"a":{"b":5},"c":6} false
+8 \N true
+9 \N true
+
diff --git a/regression-test/data/variant_p0/load_invalid_json.out
b/regression-test/data/variant_p0/load_invalid_json.out
new file mode 100644
index 00000000000..fe61c68613d
--- /dev/null
+++ b/regression-test/data/variant_p0/load_invalid_json.out
@@ -0,0 +1,16 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !rows --
+1 {"a":1} object
+11 {"a":11} object
+12 \N \N
+13 \N \N
+14 \N \N
+2 {"a":2} object
+3 123 tinyint
+4 \N \N
+5 \N \N
+6 \N \N
+
+-- !not_null --
+1 {"a":1}
+
diff --git a/regression-test/data/variant_p0/v2/variant_parse_functions.out
b/regression-test/data/variant_p0/v2/variant_parse_functions.out
index c99fe822150..7ee80984502 100644
--- a/regression-test/data/variant_p0/v2/variant_parse_functions.out
+++ b/regression-test/data/variant_p0/v2/variant_parse_functions.out
@@ -8,10 +8,7 @@
3 false false false 1 1
-- !strict_error_to_null --
-true false
-
--- !permissive_invalid_json_is_string --
-{ false { false
+true true true
-- !key_too_long_error_to_null --
true
@@ -19,6 +16,3 @@ true
-- !duplicate_key_error_to_null --
true
--- !duplicate_key_first_wins --
-{"dup":1} {"dup":1}
-
diff --git a/regression-test/suites/variant_p0/column_name.groovy
b/regression-test/suites/variant_p0/column_name.groovy
index 549184adcdc..cc097cc40cc 100644
--- a/regression-test/suites/variant_p0/column_name.groovy
+++ b/regression-test/suites/variant_p0/column_name.groovy
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-suite("regression_test_variant_column_name", "p0, nonConcurrent"){
+suite("regression_test_variant_column_name", "p0"){
def variantV2Function = "parse_to_variant"
def table_name = "var_column_name"
sql "DROP TABLE IF EXISTS ${table_name}"
@@ -73,14 +73,11 @@ suite("regression_test_variant_column_name", "p0,
nonConcurrent"){
"""
// name with `.`
- // When parser-side duplicate path deduplication is disabled, dotted keys
and nested paths
- // collide during Variant materialization and should report duplicated
entry.
- setBeConfigTemporary([variant_enable_duplicate_json_path_check: false]) {
- sql "truncate table var_column_name"
- test {
- sql """insert into var_column_name values (7,
${variantV2Function}('{"a.b": "UPPER CASE", "a" : {"b" : 123}}'))"""
- exception "may contains duplicated entry"
- }
+ // A dotted key and a nested key collide when the value is stored and fail
the write.
+ sql "truncate table var_column_name"
+ test {
+ sql """insert into var_column_name values (7,
${variantV2Function}('{"a.b": "UPPER CASE", "a" : {"b" : 123}}'))"""
+ exception "may contains duplicated entry"
}
for (int i = 0; i < 7; i++) {
sql """insert into var_column_name select * from var_column_name"""
diff --git a/regression-test/suites/variant_p0/duplicate_json_path.groovy
b/regression-test/suites/variant_p0/duplicate_json_path.groovy
index bba9caf67f5..3c6aac3e8bc 100644
--- a/regression-test/suites/variant_p0/duplicate_json_path.groovy
+++ b/regression-test/suites/variant_p0/duplicate_json_path.groovy
@@ -16,110 +16,75 @@
// under the License.
suite("duplicate_json_path", "p0") {
- def variantV2Function = "parse_to_variant"
- def customBeConfig = [
- variant_enable_duplicate_json_path_check: true
- ]
- setBeConfigTemporary(customBeConfig) {
- sql "DROP TABLE IF EXISTS duplicate_json_path"
- sql """
- CREATE TABLE duplicate_json_path (
- k int,
- v variant
- )
- DUPLICATE KEY(k)
- DISTRIBUTED BY HASH(k) BUCKETS 1
- PROPERTIES (
- "replication_num" = "1",
- "group_commit_interval_ms" = "2000",
- "disable_auto_compaction" = "true"
- );
- """
+ sql "DROP TABLE IF EXISTS duplicate_json_path"
+ sql """
+ CREATE TABLE duplicate_json_path (
+ k int,
+ v variant
+ )
+ DUPLICATE KEY(k)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1"
+ );
+ """
- sql """insert into duplicate_json_path values (1,
${variantV2Function}('{"a":42,"a":{"b":42}}'))"""
- sql """insert into duplicate_json_path values (2,
${variantV2Function}('{"a" : 123, "a" : "123"}'))"""
- sql """insert into duplicate_json_path values (3,
${variantV2Function}('{"a.b":1,"a":{"b":2}}'))"""
- sql """insert into duplicate_json_path values (4,
${variantV2Function}('{"a":{"b":3},"a.b":4}'))"""
- sql """insert into duplicate_json_path values (5,
${variantV2Function}('{"a":{"b":5},"a":{"c":6}}'))"""
- sql """insert into duplicate_json_path values (6,
${variantV2Function}('{"a":[1],"a":2}'))"""
- sql """insert into duplicate_json_path values (7,
${variantV2Function}('{"a":2,"a":[1]}'))"""
+ // A key that repeats in one object is a parse error: parse_to_variant
fails and
+ // try_parse_to_variant returns NULL.
+ test {
+ sql """insert into duplicate_json_path values (1,
parse_to_variant('{"a":42,"a":{"b":42}}'))"""
+ exception "Duplicate Variant object key"
+ }
+ sql """insert into duplicate_json_path values
+ (2, try_parse_to_variant('{"a" : 123, "a" : "123"}')),
+ (3, try_parse_to_variant('{"a":{"b":5},"a":{"c":6}}')),
+ (4, try_parse_to_variant('{"a":{"b":5},"c":6}'))"""
- streamLoad {
- table "duplicate_json_path"
- set 'read_json_by_line', 'true'
- set 'format', 'json'
- set 'group_commit', 'async_mode'
- unset 'label'
- file 'duplicate_json_path.json'
- time 10000
+ // A dotted key and a nested key are different keys of the parsed value,
but they are the
+ // same path when the value is stored, so the write fails.
+ test {
+ sql """insert into duplicate_json_path values (5,
parse_to_variant('{"a.b":1,"a":{"b":2}}'))"""
+ exception "may contains duplicated entry"
+ }
- check { result, exception, startTime, endTime ->
- if (exception != null) {
- throw exception
- }
- def json = parseJson(result)
- assertEquals("success", json.Status.toLowerCase())
- assertEquals(7, json.NumberTotalRows)
- assertEquals(7, json.NumberLoadedRows)
- }
- }
+ // Load jobs parse like try_parse_to_variant, so a row with duplicate keys
loads NULL.
+ streamLoad {
+ table "duplicate_json_path"
+ set 'read_json_by_line', 'true'
+ set 'format', 'json'
+ file 'duplicate_json_path.json'
+ time 10000
- for (int i = 0; i < 30; i++) {
- def count = sql "select count(*) from duplicate_json_path"
- if (count[0][0] == 14) {
- break
+ check { result, exception, startTime, endTime ->
+ if (exception != null) {
+ throw exception
}
- sleep(1000)
+ def json = parseJson(result)
+ assertEquals("success", json.Status.toLowerCase())
+ assertEquals(6, json.NumberTotalRows)
+ assertEquals(6, json.NumberLoadedRows)
}
- def totalRows = sql "select count(*) from duplicate_json_path"
- assertEquals(14, totalRows[0][0])
+ }
- // When duplicate path check is enabled, duplicate Variant paths keep
the first value.
- def expectedResultV1 = [
- [1, "{\"b\":42}", "42", null],
- [2, "123", null, null],
- [3, "{\"b\":1}", "1", null],
- [4, "{\"b\":3}", "3", null],
- [5, "{\"b\":5,\"c\":6}", "5", "6"],
- [6, "[1]", null, null],
- [7, "2", null, null],
- [8, "{\"b\":42}", "42", null],
- [9, "123", null, null],
- [10, "{\"b\":8}", "8", null],
- [11, "{\"b\":10}", "10", null],
- [12, "{\"b\":11,\"c\":12}", "11", "12"],
- [13, "[13]", null, null],
- [14, "14", null, null]
- ]
- def expectedResultV2 = [
- [1, "42", null, null],
- [2, "123", null, null],
- [3, "{\"b\":2}", "2", null],
- [4, "{\"b\":3}", "3", null],
- [5, "{\"b\":5}", "5", null],
- [6, "[1]", null, null],
- [7, "2", null, null],
- [8, "42", null, null],
- [9, "123", null, null],
- [10, "{\"b\":9}", "9", null],
- [11, "{\"b\":10}", "10", null],
- [12, "{\"b\":11}", "11", null],
- [13, "[13]", null, null],
- [14, "14", null, null]
- ]
- def expectedResult = true
- ? expectedResultV2 : expectedResultV1
+ // A dotted-key collision is found when the value is stored, not when it
is parsed, so it
+ // fails the whole load instead of loading NULL.
+ streamLoad {
+ table "duplicate_json_path"
+ set 'read_json_by_line', 'true'
+ set 'format', 'json'
+ inputText '''{"k":16,"v":{"a":1}}
+{"k":17,"v":{"a.b":8,"a":{"b":9}}}'''
+ time 10000
- def queryResult = {
- sql """
- select k, cast(v['a'] as string), cast(v['a']['b'] as string),
cast(v['a']['c'] as string)
- from duplicate_json_path
- order by k
- """
+ check { result, exception, startTime, endTime ->
+ if (exception != null) {
+ throw exception
+ }
+ def json = parseJson(result)
+ assertEquals("fail", json.Status.toLowerCase())
+ assertTrue(json.Message.contains("may contains duplicated entry"),
json.Message)
}
- assertEquals(expectedResult, queryResult())
-
- trigger_and_wait_compaction("duplicate_json_path", "full")
- assertEquals(expectedResult, queryResult())
}
+
+ order_qt_rows """select k, v, v is null from duplicate_json_path order by
k"""
}
diff --git a/regression-test/suites/variant_p0/load_invalid_json.groovy
b/regression-test/suites/variant_p0/load_invalid_json.groovy
new file mode 100644
index 00000000000..78e5f04f908
--- /dev/null
+++ b/regression-test/suites/variant_p0/load_invalid_json.groovy
@@ -0,0 +1,109 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// A load job parses a string field into VARIANT like try_parse_to_variant:
text that is not a
+// JSON document loads SQL NULL. A JSON-format load passes a string value
without its quotes, so a
+// string that holds JSON text is parsed, while a plain string such as "hello"
loads NULL.
+suite("variant_load_invalid_json", "p0") {
+ sql "DROP TABLE IF EXISTS variant_load_invalid_json"
+ sql """
+ CREATE TABLE variant_load_invalid_json (
+ k int,
+ v variant
+ )
+ DUPLICATE KEY(k)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES ("replication_num" = "1");
+ """
+
+ streamLoad {
+ table "variant_load_invalid_json"
+ set 'format', 'json'
+ set 'read_json_by_line', 'true'
+ inputText '''{"k":1,"v":{"a":1}}
+{"k":2,"v":"{\\"a\\":2}"}
+{"k":3,"v":"123"}
+{"k":4,"v":"hello"}
+{"k":5,"v":""}
+{"k":6,"v":null}'''
+ time 10000
+
+ check { result, exception, startTime, endTime ->
+ if (exception != null) {
+ throw exception
+ }
+ def json = parseJson(result)
+ assertEquals("success", json.Status.toLowerCase())
+ assertEquals(6, json.NumberLoadedRows)
+ }
+ }
+
+ streamLoad {
+ table "variant_load_invalid_json"
+ set 'column_separator', '|'
+ inputText '''11|{"a":11}
+12|not-json
+13|
+14|\\N'''
+ time 10000
+
+ check { result, exception, startTime, endTime ->
+ if (exception != null) {
+ throw exception
+ }
+ def json = parseJson(result)
+ assertEquals("success", json.Status.toLowerCase())
+ assertEquals(4, json.NumberLoadedRows)
+ }
+ }
+
+ order_qt_rows """select k, v, variant_type(v) from
variant_load_invalid_json order by k"""
+
+ // A NOT NULL column filters the rows that parse to NULL.
+ sql "DROP TABLE IF EXISTS variant_load_invalid_json_not_null"
+ sql """
+ CREATE TABLE variant_load_invalid_json_not_null (
+ k int,
+ v variant NOT NULL
+ )
+ DUPLICATE KEY(k)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES ("replication_num" = "1");
+ """
+
+ streamLoad {
+ table "variant_load_invalid_json_not_null"
+ set 'format', 'json'
+ set 'read_json_by_line', 'true'
+ set 'max_filter_ratio', '0.5'
+ inputText '''{"k":1,"v":{"a":1}}
+{"k":2,"v":"hello"}'''
+ time 10000
+
+ check { result, exception, startTime, endTime ->
+ if (exception != null) {
+ throw exception
+ }
+ def json = parseJson(result)
+ assertEquals("success", json.Status.toLowerCase())
+ assertEquals(1, json.NumberLoadedRows)
+ assertEquals(1, json.NumberFilteredRows)
+ }
+ }
+
+ order_qt_not_null """select k, v from variant_load_invalid_json_not_null
order by k"""
+}
diff --git a/regression-test/suites/variant_p0/v2/variant_compute_v2.groovy
b/regression-test/suites/variant_p0/v2/variant_compute_v2.groovy
index 7c702931e60..d88469c573a 100644
--- a/regression-test/suites/variant_p0/v2/variant_compute_v2.groovy
+++ b/regression-test/suites/variant_p0/v2/variant_compute_v2.groovy
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-suite("variant_compute_v2", "p0,nonConcurrent") {
+suite("variant_compute_v2", "p0") {
def variantV2Function = "parse_to_variant"
sql "SET enable_nereids_planner = true"
sql "SET enable_fallback_to_original_planner = false"
@@ -288,23 +288,21 @@ suite("variant_compute_v2", "p0,nonConcurrent") {
FROM numbers("number" = "1")
"""
- setBeConfigTemporary([variant_throw_exeception_on_invalid_json: true]) {
- order_qt_parse_error_to_null """
+ order_qt_parse_error_to_null """
+ SELECT number,
+ CAST(try_parse_to_variant(payload) AS STRING),
+ try_parse_to_variant(payload) IS NULL
+ FROM (
SELECT number,
- CAST(try_parse_to_variant(payload) AS STRING),
- try_parse_to_variant(payload) IS NULL
- FROM (
- SELECT number,
- CASE number
- WHEN 0 THEN '{"ok":1}'
- WHEN 1 THEN 'not-json'
- ELSE NULL
- END AS payload
- FROM numbers("number" = "3")
- ) t
- ORDER BY number
- """
- }
+ CASE number
+ WHEN 0 THEN '{"ok":1}'
+ WHEN 1 THEN 'not-json'
+ ELSE NULL
+ END AS payload
+ FROM numbers("number" = "3")
+ ) t
+ ORDER BY number
+ """
order_qt_direct_result """
SELECT ${variantV2Function}(CONCAT('{"id":', number, '}'))
diff --git
a/regression-test/suites/variant_p0/v2/variant_parse_functions.groovy
b/regression-test/suites/variant_p0/v2/variant_parse_functions.groovy
index e20dd7e685d..433d2bf68df 100644
--- a/regression-test/suites/variant_p0/v2/variant_parse_functions.groovy
+++ b/regression-test/suites/variant_p0/v2/variant_parse_functions.groovy
@@ -36,81 +36,60 @@ suite("variant_parse_functions", "p0,nonConcurrent") {
FROM (${nullableInput}) t ORDER BY id
"""
- setBeConfigTemporary([variant_throw_exeception_on_invalid_json: true]) {
- test {
- sql """
- SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/
- parse_to_variant('{')
- """
- exception "Parse json document failed at row 0, error:"
- }
- test {
- sql """
- SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/
- json_parse('{')
- """
- exception "Parse json document failed at row 0, error:"
- }
- order_qt_strict_error_to_null """
+ // Text that is not a JSON document, including an empty string, is an
error; the TRY form
+ // returns NULL instead.
+ test {
+ sql """
SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/
- try_parse_to_variant('{') IS NULL,
- try_parse_to_variant('') IS NULL
+ parse_to_variant('{')
"""
+ exception "Parse json document failed at row 0, error:"
}
-
- setBeConfigTemporary([variant_throw_exeception_on_invalid_json: false]) {
- order_qt_permissive_invalid_json_is_string """
+ test {
+ sql """
SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/
- CAST(parse_to_variant('{') AS STRING),
- parse_to_variant('{') IS NULL,
- CAST(try_parse_to_variant('{') AS STRING),
- try_parse_to_variant('{') IS NULL
+ parse_to_variant('')
"""
+ exception "Parse json document failed at row 0, error:"
}
-
- setBeConfigTemporary([
- variant_throw_exeception_on_invalid_json: true,
- variant_max_json_key_length: 3
- ]) {
- test {
- sql """
- SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/
- parse_to_variant('{"long":1}')
- """
- exception "Parse json document failed at row 0, error:"
- }
- order_qt_key_too_long_error_to_null """
+ test {
+ sql """
SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/
- try_parse_to_variant('{"long":1}') IS NULL
+ json_parse('{')
"""
+ exception "Parse json document failed at row 0, error:"
}
+ order_qt_strict_error_to_null """
+ SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/
+ try_parse_to_variant('{') IS NULL,
+ try_parse_to_variant('') IS NULL,
+ try_parse_to_variant('not-json') IS NULL
+ """
- setBeConfigTemporary([
- variant_throw_exeception_on_invalid_json: true,
- variant_enable_duplicate_json_path_check: false
- ]) {
+ setBeConfigTemporary([variant_max_json_key_length: 3]) {
test {
sql """
SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/
- parse_to_variant('{"dup":1,"dup":2}')
+ parse_to_variant('{"long":1}')
"""
exception "Parse json document failed at row 0, error:"
}
- order_qt_duplicate_key_error_to_null """
+ order_qt_key_too_long_error_to_null """
SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/
- try_parse_to_variant('{"dup":1,"dup":2}') IS NULL
+ try_parse_to_variant('{"long":1}') IS NULL
"""
}
- setBeConfigTemporary([
- variant_throw_exeception_on_invalid_json: true,
- variant_enable_duplicate_json_path_check: true
- ]) {
- order_qt_duplicate_key_first_wins """
+ test {
+ sql """
SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/
- CAST(parse_to_variant('{"dup":1,"dup":2}') AS STRING),
- CAST(try_parse_to_variant('{"dup":1,"dup":2}') AS STRING)
+ parse_to_variant('{"dup":1,"dup":2}')
"""
+ exception "Parse json document failed at row 0, error:"
}
+ order_qt_duplicate_key_error_to_null """
+ SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/
+ try_parse_to_variant('{"dup":1,"dup":2}') IS NULL
+ """
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
