Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions be/src/core/column/column_variant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1835,6 +1835,11 @@ bool ColumnVariant::is_visible_root_value(size_t nrow) const {
}
}

const auto& sparse_offsets = serialized_sparse_column_offsets();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still misses sparse rows after any prior doc-snapshot row. Once this new check returns false for a row with sparse entries, serialize_one_row_to_json_format() immediately tests doc_value_column_map.get_offsets()[row_num] != 0; that is the cumulative end offset, so it remains true for later rows even when the current row has no doc-value entries. If a result column is assembled from mixed-format or compatibility rowsets where an earlier segment has doc-value snapshots and a later row has sparse entries, the sparse row can serialize as {} and return before the sparse map below is considered. Please gate the doc snapshot branch with the current-row delta, e.g. the existing has_doc_value_column(row_num) / offsets[row_num - 1] < offsets[row_num], and add a unit case with a prior doc row and a current sparse row.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to fix in new mem layout

if (sparse_offsets[nrow - 1] != sparse_offsets[nrow]) {
return false;
}

const auto& doc_value_column_map = assert_cast<const ColumnMap&>(*serialized_doc_value_column);
// doc snapshot column is not empty
if (doc_value_column_map.get_offsets()[nrow - 1] != doc_value_column_map.get_offsets()[nrow]) {
Expand Down
27 changes: 27 additions & 0 deletions be/test/core/column/column_variant_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1066,6 +1066,33 @@ TEST_F(ColumnVariantTest, test_insert_indices_from) {
}
}

TEST_F(ColumnVariantTest, visible_root_does_not_hide_sparse_fields) {
auto source = ColumnVariant::create(0, false);
VariantMap mixed;
mixed.try_emplace(PathInData(), FieldWithDataType {.field = get_jsonb_field("array_int")});
mixed.try_emplace(PathInData("n"), FieldWithDataType {.field = VariantUtil::get_field("int")});
mixed.try_emplace(PathInData("word"),
FieldWithDataType {.field = VariantUtil::get_field("string")});
source->try_insert(Field::create_field<TYPE_VARIANT>(std::move(mixed)));
source->finalize();

auto destination = ColumnVariant::create(1, false);
destination->try_insert(
VariantUtil::construct_variant_map({{"k", VariantUtil::get_field("int")}}));
destination->insert_range_from(*source, 0, 1);
destination->finalize();

EXPECT_EQ(destination->get_subcolumns().get_root()->data.get_least_common_base_type_id(),
PrimitiveType::TYPE_JSONB);
const auto& sparse_offsets = destination->serialized_sparse_column_offsets();
EXPECT_LT(sparse_offsets[0], sparse_offsets[1]);

DataTypeSerDe::FormatOptions options;
std::string json;
destination->serialize_one_row_to_string(1, &json, options);
EXPECT_EQ(json, R"({"n":20,"word":"str"})");
}

TEST_F(ColumnVariantTest, is_variable_length) {
EXPECT_TRUE(column_variant->is_variable_length());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
-- This file is automatically generated. You should know what you did if you want to edit this
-- !topn_lazy_sparse_variant --
0 1 {} {}
1 -1 {} [1,2,3]
3 42 {"k":1} {}
4 -9223372036854775808 [1,2,3] {"k":1}
6 1 [1,2,3] {}
7 0 [1,2,3] [1,2,3]
9 \N {} []

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This expected row does not match the inserted value. The suite inserts col_variant = '{}' for pk 9 (and again for pk 15), but the new oracle records []. Other rows inserted as {} in this same output remain {}, so this is not just a display convention; it locks in an object-to-array change in the regression that is supposed to protect VARIANT materialization correctness. Please fix the underlying serialization case and regenerate this as {} (or remove these rows from this regression and cover the object/array issue separately).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to fix in new mem layout

10 1 {} {"n":7,"word":"hot"}
12 1 {"word":"hot","n":7} \N
13 5777142737451291289 {} {"n":7,"word":"hot"}
15 -1 [1,2,3] []
16 -906638365906932436 {"word":"hot","n":7} {"k":1}
18 -9223372036854775808 {"word":"hot","n":7} {"n":7,"word":"hot"}
19 -9223372036854775808 {"word":"hot","n":7} [1,2,3]

Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// 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.

suite("test_topn_lazy_materialize_sparse_variant", "p0") {
sql "set default_variant_enable_doc_mode = false"
sql "set default_variant_max_subcolumns_count = 1"
sql "set default_variant_sparse_hash_shard_count = 2"
sql "set use_v3_storage_format = true"
sql "set enable_file_cache = true"
sql "set enable_segment_limit_pushdown = false"
sql "set topn_lazy_materialization_threshold = 1024"

sql "DROP TABLE IF EXISTS test_topn_lazy_materialize_sparse_variant"
sql """
CREATE TABLE test_topn_lazy_materialize_sparse_variant (
pk INT,
col_bigint BIGINT NULL,
col_json JSON NOT NULL,
col_variant VARIANT NULL
) ENGINE = OLAP
DUPLICATE KEY(pk)
PARTITION BY RANGE(pk) (
PARTITION p0 VALUES LESS THAN ('4'),
PARTITION p1 VALUES LESS THAN ('64'),
PARTITION p2 VALUES LESS THAN ('256'),
PARTITION pmax VALUES LESS THAN ('2147483647')
)
DISTRIBUTED BY HASH(pk) BUCKETS 4
PROPERTIES (
"replication_num" = "1",
"disable_auto_compaction" = "true",
"storage_format" = "V3"
)
"""

sql """
INSERT INTO test_topn_lazy_materialize_sparse_variant VALUES
(19, -9223372036854775808, '{"word":"hot","n":7}', '[1,2,3]'),
(18, -9223372036854775808, '{"word":"hot","n":7}', '{"word":"hot","n":7}'),
(17, -1, '{"k":1}', '{}'),
(16, -906638365906932436, '{"word":"hot","n":7}', '{"k":1}'),
(15, -1, '[1,2,3]', '{}'),
(14, -9223372036854775808, '{"word":"hot","n":7}', '[1,2,3]'),
(13, 5777142737451291289, '{}', '{"word":"hot","n":7}'),
(12, 1, '{"word":"hot","n":7}', NULL),
(11, 42, '[1,2,3]', NULL),
(10, 1, '{}', '{"word":"hot","n":7}'),
(9, NULL, '{}', '{}'),
(8, NULL, '{"k":1}', '{"k":1}'),
(7, 0, '[1,2,3]', '[1,2,3]'),
(6, 1, '[1,2,3]', '{}'),
(5, -3610716764638269764, '{"k":1}', '[1,2,3]'),
(4, -9223372036854775808, '[1,2,3]', '{"k":1}'),
(3, 42, '{"k":1}', '{}'),
(2, -9223372036854775808, '{}', '{}'),
(1, -1, '{}', '[1,2,3]'),
(0, 1, '{}', '{}')
"""

def topnQuery = """
SELECT pk, col_bigint, col_json, col_variant
FROM test_topn_lazy_materialize_sparse_variant
WHERE ABS(pk % 3) IN (0, 1, 3)
ORDER BY pk ASC
LIMIT 128
"""
explain {
sql topnQuery
contains("MaterializeNode")
}
qt_topn_lazy_sparse_variant topnQuery
}
Loading