-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[fix](variant) Prefer sparse variant fields over root value #65660
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 {} [] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This expected row does not match the inserted value. The suite inserts
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
| } |
There was a problem hiding this comment.
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 testsdoc_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 existinghas_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.There was a problem hiding this comment.
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