This is an automated email from the ASF dual-hosted git repository. eldenmoon pushed a commit to branch branch-pick-68447-to-4.2 in repository https://gitbox.apache.org/repos/asf/doris.git
commit 8dd16917ea8eadeecf2973bfe6f6786bea356fa8 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) (cherry picked from commit f5bb482e2f9d27c548bcc3824a0e401314ca87d2) --- be/src/common/config.cpp | 2 - be/src/common/config.h | 4 - .../data_type_serde/data_type_variant_serde.cpp | 4 +- .../data_type_serde/data_type_variant_v2_serde.cpp | 12 +- be/src/exec/common/variant_util.cpp | 3 +- be/src/exprs/function/function_variant_parse.cpp | 2 - .../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/src/util/json/json_parser.h | 5 +- be/test/core/column/column_variant_v2_test.cpp | 14 +- .../data_type_serde/data_type_serde_arrow_test.cpp | 6 +- ...type_variant_v2_serde_binary_roundtrip_test.cpp | 4 +- .../data_type_variant_v2_serde_input_test.cpp | 50 ++---- .../data_type_variant_v2_serde_jsonb_test.cpp | 3 +- .../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 ++++---- be/test/exprs/variant_v2_execution_test.cpp | 3 +- .../format/transformer/vparquet_writer_test.cpp | 6 +- .../variant/variant_column_writer_reader_test.cpp | 81 ++------- be/test/storage/variant/variant_util_test.cpp | 32 ---- be/test/testutil/index_storage_test_util.cpp | 1 - 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 | 13 +- .../suites/variant_p0/duplicate_json_path.groovy | 185 ++++++++++----------- .../suites/variant_p0/load_invalid_json.groovy | 109 ++++++++++++ .../suites/variant_p0/v2/variant_compute_v2.groovy | 30 ++-- .../variant_p0/v2/variant_parse_functions.groovy | 85 ++++------ 37 files changed, 407 insertions(+), 517 deletions(-) diff --git a/be/src/common/config.cpp b/be/src/common/config.cpp index bd9eff1e435..3722740c281 100644 --- a/be/src/common/config.cpp +++ b/be/src/common/config.cpp @@ -1198,8 +1198,6 @@ DEFINE_mInt64(workload_group_scan_task_wait_timeout_ms, "10000"); DEFINE_mBool(variant_use_cloud_schema_dict_cache, "true"); DEFINE_mInt64(variant_threshold_rows_to_estimate_sparse_column, "2048"); 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 70c6519efef..677f10b1975 100644 --- a/be/src/common/config.h +++ b/be/src/common/config.h @@ -1558,10 +1558,6 @@ DECLARE_mBool(variant_use_cloud_schema_dict_cache); DECLARE_mInt64(variant_threshold_rows_to_estimate_sparse_column); // 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_serde.cpp b/be/src/core/data_type_serde/data_type_variant_serde.cpp index e0ddd20b7d6..ebfccd6f445 100644 --- a/be/src/core/data_type_serde/data_type_variant_serde.cpp +++ b/be/src/core/data_type_serde/data_type_variant_serde.cpp @@ -138,7 +138,9 @@ Status DataTypeVariantSerDe::serialize_one_cell_to_json(const IColumn& column, i Status DataTypeVariantSerDe::deserialize_one_cell_from_json(IColumn& column, Slice& slice, const FormatOptions& options) const { ParseConfig parse_config; - parse_config.check_duplicate_json_path = config::variant_enable_duplicate_json_path_check; + // The serde mostly re-reads JSON written by a Variant serde, so a value the parser + // cannot read back, such as an integer beyond 64 bits, keeps its text. + parse_config.throw_on_invalid_json = false; StringRef json_ref(slice.data, slice.size); RETURN_IF_CATCH_EXCEPTION( variant_util::parse_json_to_variant(column, json_ref, nullptr, parse_config)); 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 20c7ba92936..121ba607d53 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 @@ -388,12 +388,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); @@ -436,7 +444,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/exec/common/variant_util.cpp b/be/src/exec/common/variant_util.cpp index 274691304f1..4ebb549e076 100644 --- a/be/src/exec/common/variant_util.cpp +++ b/be/src/exec/common/variant_util.cpp @@ -1868,7 +1868,7 @@ void parse_json_to_variant_impl(IColumn& column, const char* src, size_t length, } if (!result) { VLOG_DEBUG << "failed to parse " << std::string_view(src, length) << ", length= " << length; - if (config::variant_throw_exeception_on_invalid_json) { + if (config.throw_on_invalid_json) { throw doris::Exception(ErrorCode::INVALID_ARGUMENT, "Failed to parse object {}", std::string_view(src, length)); } @@ -2266,7 +2266,6 @@ Status parse_and_materialize_variant_columns(Block& block, const TabletSchema& t // Deprecated legacy flatten-nested switch. Distinct from variant_enable_nested_group. configs[i].deprecated_enable_flatten_nested = tablet_schema.deprecated_variant_flatten_nested(); - configs[i].check_duplicate_json_path = config::variant_enable_duplicate_json_path_check; const auto& column = tablet_schema.column(variant_schema_pos[i]); if (!column.is_variant_type()) { return Status::InternalError("column is not variant type, column name: {}", diff --git a/be/src/exprs/function/function_variant_parse.cpp b/be/src/exprs/function/function_variant_parse.cpp index c41b2316afb..f7bcc124983 100644 --- a/be/src/exprs/function/function_variant_parse.cpp +++ b/be/src/exprs/function/function_variant_parse.cpp @@ -80,8 +80,6 @@ public: const bool enable_doc_mode = variant_type->enable_doc_mode(); auto values = ColumnVariant::create(max_subcolumns_count, enable_doc_mode); ParseConfig parse_config; - parse_config.check_duplicate_json_path = - config::variant_enable_duplicate_json_path_check; parse_config.parse_to = enable_doc_mode ? ParseConfig::ParseTo::OnlyDocValueColumn : ParseConfig::ParseTo::OnlySubcolumns; JsonParser parser; 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 f197d96d30a..cc4c7ebd8d8 100644 --- a/be/src/storage/segment/variant/v2/variant_shredder.cpp +++ b/be/src/storage/segment/variant/v2/variant_shredder.cpp @@ -161,16 +161,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); } @@ -213,9 +208,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 4f8c57c3ae7..d8e53e405b2 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/src/util/json/json_parser.h b/be/src/util/json/json_parser.h index c4a165e8995..b4c50d787bf 100644 --- a/be/src/util/json/json_parser.h +++ b/be/src/util/json/json_parser.h @@ -101,7 +101,10 @@ void writeValueAsJsonb(const Element& element, JsonbWriter& writer) { struct ParseConfig { bool deprecated_enable_flatten_nested = false; - bool check_duplicate_json_path = false; + // Duplicate paths in one JSON document are always rejected. + bool check_duplicate_json_path = true; + // Text that is not a JSON document fails the row. Only the Variant serde turns this off. + bool throw_on_invalid_json = true; enum class ParseTo { OnlySubcolumns = 0, OnlyDocValueColumn = 1, diff --git a/be/test/core/column/column_variant_v2_test.cpp b/be/test/core/column/column_variant_v2_test.cpp index fa74c056386..c774a4d7570 100644 --- a/be/test/core/column/column_variant_v2_test.cpp +++ b/be/test/core/column/column_variant_v2_test.cpp @@ -80,9 +80,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)); @@ -1521,9 +1519,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(); @@ -1533,9 +1530,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_serde_arrow_test.cpp b/be/test/core/data_type_serde/data_type_serde_arrow_test.cpp index f2551229706..d8d4708acce 100644 --- a/be/test/core/data_type_serde/data_type_serde_arrow_test.cpp +++ b/be/test/core/data_type_serde/data_type_serde_arrow_test.cpp @@ -941,8 +941,7 @@ TEST(DataTypeSerDeArrowTest, PlainArrowConverterDoesNotInferIcebergUuid) { TEST(DataTypeSerDeArrowTest, IcebergVariantExtensionAndParquetSchema) { auto make_variant_column = []() { JsonStringToVariantEncoder encoder({.max_json_key_length = 1024, - .throw_on_invalid_json = true, - .check_duplicate_json_path = false}); + .throw_on_invalid_json = true}); for (std::string_view json : {std::string_view {R"({"name":"doris","n":1})"}, std::string_view {"null"}, std::string_view {R"([1,true,"x"])"}}) { @@ -1038,8 +1037,7 @@ TEST(DataTypeSerDeArrowTest, IcebergVariantExtensionAndParquetSchema) { TEST(DataTypeSerDeArrowTest, NestedIcebergVariantExtensionsAndParquetSchema) { auto make_variant_column = [](std::initializer_list<std::string_view> json_values) { JsonStringToVariantEncoder encoder({.max_json_key_length = 1024, - .throw_on_invalid_json = true, - .check_duplicate_json_path = false}); + .throw_on_invalid_json = true}); for (std::string_view json : json_values) { encoder.add_json({json.data(), json.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 fbe40886df2..f7dc4e171ed 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 @@ -63,9 +63,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 e9d3d669a17..6e8d4718a70 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_jsonb_test.cpp b/be/test/core/data_type_serde/data_type_variant_v2_serde_jsonb_test.cpp index 5cecf9d9cef..be9b8b951e6 100644 --- a/be/test/core/data_type_serde/data_type_variant_v2_serde_jsonb_test.cpp +++ b/be/test/core/data_type_serde/data_type_variant_v2_serde_jsonb_test.cpp @@ -113,8 +113,7 @@ std::string json_at(const DataTypeVariantV2SerDe& serde, const IColumn& column, 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}); + .throw_on_invalid_json = true}); for (std::string_view row : rows) { encoder.add_json({row.data(), row.size()}); } 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 25c3ca7abd9..fa5691fc629 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 585847de4ad..84b1b71bf7d 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 383329a1b0a..05eb0443a70 100644 --- a/be/test/exprs/function/function_variant_parse_test.cpp +++ b/be/test/exprs/function/function_variant_parse_test.cpp @@ -187,7 +187,6 @@ TEST(FunctionVariantParseTest, LegacyPathPreservesSqlNullAndErrorToNull) { 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, false); @@ -270,15 +269,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( @@ -301,7 +307,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", @@ -319,26 +324,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) { @@ -355,7 +351,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()); @@ -365,15 +360,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); @@ -383,6 +369,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/exprs/variant_v2_execution_test.cpp b/be/test/exprs/variant_v2_execution_test.cpp index 36fd2e1bec9..e94c86dfd31 100644 --- a/be/test/exprs/variant_v2_execution_test.cpp +++ b/be/test/exprs/variant_v2_execution_test.cpp @@ -51,8 +51,7 @@ TEST(VariantV2ExecutionTest, TypePropertiesDoNotAffectEquality) { TEST(VariantV2ExecutionTest, CanonicalComparison) { auto parse = [](std::initializer_list<std::string_view> rows) { JsonStringToVariantEncoder encoder({.max_json_key_length = 255, - .throw_on_invalid_json = true, - .check_duplicate_json_path = false}); + .throw_on_invalid_json = true}); for (auto json : rows) { encoder.add_json({json.data(), json.size()}); } diff --git a/be/test/format/transformer/vparquet_writer_test.cpp b/be/test/format/transformer/vparquet_writer_test.cpp index 279d0e664d0..f872057116d 100644 --- a/be/test/format/transformer/vparquet_writer_test.cpp +++ b/be/test/format/transformer/vparquet_writer_test.cpp @@ -89,8 +89,7 @@ TEST_F(VParquetWriterTest, WritesIcebergVariantAndCollectsLogicalMetrics) { ASSERT_TRUE(transformer.open().ok()); JsonStringToVariantEncoder encoder({.max_json_key_length = 1024, - .throw_on_invalid_json = true, - .check_duplicate_json_path = false}); + .throw_on_invalid_json = true}); for (std::string_view json : {std::string_view {R"({"name":"doris","n":1})"}, std::string_view {"null"}, std::string_view {R"([1,2,3])"}}) { encoder.add_json({json.data(), json.size()}); @@ -176,8 +175,7 @@ TEST_F(VParquetWriterTest, WritesNestedIcebergVariant) { ASSERT_TRUE(transformer.open().ok()); JsonStringToVariantEncoder encoder({.max_json_key_length = 1024, - .throw_on_invalid_json = true, - .check_duplicate_json_path = false}); + .throw_on_invalid_json = true}); for (std::string_view json : {std::string_view {R"({"kind":"nested"})"}, std::string_view {"null"}, std::string_view {"{}"}, std::string_view {"7"}}) { 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 183a3e077fc..ec79782db18 100644 --- a/be/test/storage/variant/variant_column_writer_reader_test.cpp +++ b/be/test/storage/variant/variant_column_writer_reader_test.cpp @@ -105,20 +105,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, @@ -138,7 +124,6 @@ static Status create_variant_writer_source(VariantWriterInput input, ParseConfig parse_config; parse_config.parse_to = enable_doc_mode ? ParseConfig::ParseTo::OnlyDocValueColumn : ParseConfig::ParseTo::OnlySubcolumns; - parse_config.check_duplicate_json_path = config::variant_enable_duplicate_json_path_check; variant_util::parse_json_to_variant(*variant, *json_column, parse_config); values = std::move(variant); type = std::make_shared<DataTypeVariant>(max_subcolumns_count, enable_doc_mode); @@ -2295,7 +2280,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 { @@ -2336,11 +2320,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) @@ -2349,12 +2337,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) { @@ -2436,47 +2429,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(); @@ -3318,7 +3271,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; @@ -3395,7 +3347,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/storage/variant/variant_util_test.cpp b/be/test/storage/variant/variant_util_test.cpp index fb729a3cfd5..5ccfdcd7da3 100644 --- a/be/test/storage/variant/variant_util_test.cpp +++ b/be/test/storage/variant/variant_util_test.cpp @@ -120,20 +120,6 @@ static std::string _serialize_variant_row(const ColumnVariant& variant, size_t r return value; } -class ScopedDuplicateJsonPathCheck { -public: - explicit ScopedDuplicateJsonPathCheck(bool value) - : _old_value(config::variant_enable_duplicate_json_path_check) { - config::variant_enable_duplicate_json_path_check = value; - } - ~ScopedDuplicateJsonPathCheck() { - config::variant_enable_duplicate_json_path_check = _old_value; - } - -private: - bool _old_value; -}; - class ScopedVariantStorageParseMode { public: explicit ScopedVariantStorageParseMode(int32_t value) @@ -365,7 +351,6 @@ TEST(VariantUtilTest, ParseOnlyDocValueColumn_SerializesMixedTypes) { } TEST(VariantUtilTest, ParseDuplicateJsonPathsKeepsFirstValue) { - ScopedDuplicateJsonPathCheck check_guard(true); const std::vector<std::string_view> jsons = { R"({"a":42,"a":{"b":42}})", R"({"a":123,"a":"123"})", R"({"a.b":1,"a":{"b":2}})", R"({"a":{"b":3},"a.b":4})", R"({"a":{"b":5},"a":{"c":6}})", @@ -417,7 +402,6 @@ TEST(VariantUtilTest, ParseDuplicateJsonPathsKeepsFirstValue) { } TEST(VariantUtilTest, ParseDuplicateJsonPathsKeepsFirstArrayOrScalarValue) { - ScopedDuplicateJsonPathCheck check_guard(true); const std::vector<std::string_view> jsons = { R"({"a":[1],"a":2})", R"({"a":2,"a":[1]})", @@ -449,7 +433,6 @@ TEST(VariantUtilTest, ParseDuplicateJsonPathsKeepsFirstArrayOrScalarValue) { } TEST(VariantUtilTest, ParseDuplicateJsonPathsInDocModeKeepsFirstValue) { - ScopedDuplicateJsonPathCheck check_guard(true); const std::vector<std::string_view> jsons = { R"({"a":42,"a":{"b":42}})", R"({"a":123,"a":"123"})", R"({"a.b":1,"a":{"b":2}})", R"({"a":{"b":3},"a.b":4})", R"({"a":{"b":5},"a":{"c":6}})", @@ -506,7 +489,6 @@ TEST(VariantUtilTest, ParseDuplicateJsonPathsInDocModeKeepsFirstValue) { } TEST(VariantUtilTest, ParseDuplicateJsonPathsInDocModeKeepsFirstArrayOrScalarValue) { - ScopedDuplicateJsonPathCheck check_guard(true); const std::vector<std::string_view> jsons = { R"({"a":[1],"a":2})", R"({"a":2,"a":[1]})", @@ -540,20 +522,6 @@ TEST(VariantUtilTest, ParseDuplicateJsonPathsInDocModeKeepsFirstArrayOrScalarVal EXPECT_EQ(JsonbToJson::jsonb_to_json_string(second.get_value(), second.get_size()), "2"); } -TEST(VariantUtilTest, ParseDuplicateJsonPathsCheckDisabledByDefault) { - ScopedDuplicateJsonPathCheck check_guard(false); - const std::vector<std::string_view> jsons = { - R"({"a":123,"a":"123"})", - }; - - auto variant = ColumnVariant::create(0, false); - auto json_col = _make_json_column(jsons); - - ParseConfig cfg; - cfg.deprecated_enable_flatten_nested = false; - EXPECT_THROW(parse_json_to_variant(*variant, *json_col, cfg), Exception); -} - TEST(VariantUtilTest, ParseVariantColumns_StorageNonDocScalarJsonToDocValueKv) { TabletSchema tablet_schema = _make_variant_schema(false); std::vector<std::string> jsons {R"({"a":1})", R"({"a":2})"}; diff --git a/be/test/testutil/index_storage_test_util.cpp b/be/test/testutil/index_storage_test_util.cpp index b94a62be2e9..8a17aff2f01 100644 --- a/be/test/testutil/index_storage_test_util.cpp +++ b/be/test/testutil/index_storage_test_util.cpp @@ -412,7 +412,6 @@ Status fill_variant_column(const VariantColumnSpec& column_spec, const IndexBatc ParseConfig config; config.deprecated_enable_flatten_nested = batch.deprecated_enable_flatten_nested; - config.check_duplicate_json_path = batch.check_duplicate_json_path; config.parse_to = column_spec.enable_doc_mode ? ParseConfig::ParseTo::OnlyDocValueColumn : batch.parse_to; variant_util::parse_json_to_variant(*variant_column, *json_column, config); diff --git a/be/test/testutil/index_storage_test_util.h b/be/test/testutil/index_storage_test_util.h index 26ea66c2686..f4f10dcc848 100644 --- a/be/test/testutil/index_storage_test_util.h +++ b/be/test/testutil/index_storage_test_util.h @@ -116,7 +116,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; ParseConfig::ParseTo parse_to = ParseConfig::ParseTo::OnlySubcolumns; static IndexBatch single_text(std::vector<std::string> values, 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 4c900dfbdd3..eabdcafb142 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 dd75979bb1b..2ebf419fecd 100644 --- a/be/test/util/variant/variant_string_parse_test.cpp +++ b/be/test/util/variant/variant_string_parse_test.cpp @@ -282,64 +282,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) { @@ -359,13 +311,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, [&] { @@ -520,8 +474,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 037edd8f10e..758924e0069 100644 --- a/regression-test/suites/variant_p0/column_name.groovy +++ b/regression-test/suites/variant_p0/column_name.groovy @@ -66,14 +66,11 @@ suite("regression_test_variant_column_name", "p0, nonConcurrent"){ qt_sql "select cast(Tags[''] as text) from var_column_name order by cast(Tags[''] as string)" // 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 7fcfeac97f7..da219dba280 100644 --- a/regression-test/suites/variant_p0/duplicate_json_path.groovy +++ b/regression-test/suites/variant_p0/duplicate_json_path.groovy @@ -17,109 +17,104 @@ suite("duplicate_json_path", "p0") { def variantV2Function = getFeConfig("enable_variant_v2").toBoolean() ? "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", + "group_commit_interval_ms" = "2000", + "disable_auto_compaction" = "true" + ); + """ - 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]}'))""" + 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]}'))""" - 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 + 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 - 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) + 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) } + } - for (int i = 0; i < 30; i++) { - def count = sql "select count(*) from duplicate_json_path" - if (count[0][0] == 14) { - break - } - sleep(1000) + for (int i = 0; i < 30; i++) { + def count = sql "select count(*) from duplicate_json_path" + if (count[0][0] == 14) { + break } - 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 = getFeConfig("enable_variant_v2").toBoolean() - ? expectedResultV2 : expectedResultV1 + sleep(1000) + } + def totalRows = sql "select count(*) from duplicate_json_path" + assertEquals(14, totalRows[0][0]) - 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 - """ - } - assertEquals(expectedResult, queryResult()) + // 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 = getFeConfig("enable_variant_v2").toBoolean() + ? expectedResultV2 : expectedResultV1 - trigger_and_wait_compaction("duplicate_json_path", "full") - assertEquals(expectedResult, queryResult()) + 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 + """ } + assertEquals(expectedResult, queryResult()) + + trigger_and_wait_compaction("duplicate_json_path", "full") + assertEquals(expectedResult, queryResult()) } 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 7eaf85c4d0c..3a288f35df7 100644 --- a/regression-test/suites/variant_p0/v2/variant_compute_v2.groovy +++ b/regression-test/suites/variant_p0/v2/variant_compute_v2.groovy @@ -290,23 +290,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 abea8b350f6..20577be43bb 100644 --- a/regression-test/suites/variant_p0/v2/variant_parse_functions.groovy +++ b/regression-test/suites/variant_p0/v2/variant_parse_functions.groovy @@ -38,42 +38,37 @@ 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)*/ + parse_to_variant('') + """ + exception "Parse json document failed at row 0, error:" + } + 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 + 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_max_json_key_length: 3 - ]) { + setBeConfigTemporary([variant_max_json_key_length: 3]) { test { sql """ SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/ @@ -87,32 +82,16 @@ suite("variant_parse_functions", "p0,nonConcurrent") { """ } - setBeConfigTemporary([ - variant_throw_exeception_on_invalid_json: true, - variant_enable_duplicate_json_path_check: false - ]) { - test { - sql """ - SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/ - parse_to_variant('{"dup":1,"dup":2}') - """ - exception "Parse json document failed at row 0, error:" - } - order_qt_duplicate_key_error_to_null """ + test { + sql """ SELECT /*+SET_VAR(enable_fold_constant_by_be=false)*/ - try_parse_to_variant('{"dup":1,"dup":2}') IS NULL + 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 + """ - setBeConfigTemporary([ - variant_throw_exeception_on_invalid_json: true, - variant_enable_duplicate_json_path_check: true - ]) { - order_qt_duplicate_key_first_wins """ - 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) - """ - } - } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
