github-actions[bot] commented on code in PR #65051: URL: https://github.com/apache/doris/pull/65051#discussion_r3502746533
########## regression-test/suites/point_query_p0/test_short_circuit_rowstore_nested_complex.groovy: ########## @@ -0,0 +1,95 @@ +// 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_short_circuit_rowstore_nested_complex", "p0") { + sql "SET enable_nereids_planner=true" + sql "SET enable_sql_cache=false" + sql "SET enable_snapshot_point_query=true" + sql "SET enable_short_circuit_query_access_column_store=true" + + sql "DROP TABLE IF EXISTS short_circuit_rowstore_nested_complex" + sql """ + CREATE TABLE short_circuit_rowstore_nested_complex ( + pk INT, + deep STRUCT< + nested_str: VARCHAR(64), + inner_s: STRUCT<deep_str: VARCHAR(64), flag: BOOLEAN, deep_char: CHAR(8)>, + deep_arr: ARRAY<STRUCT<verified: BOOLEAN, txt: VARCHAR(64), char_tag: CHAR(8)>>, + deep_map: MAP<VARCHAR(32), STRUCT<leaf: VARCHAR(64), n: INT, char_leaf: CHAR(8)>> + > NULL, + s STRUCT<str: VARCHAR(64), char_leaf: CHAR(8), num: INT, sibling: VARCHAR(64)> NULL + ) ENGINE = OLAP + UNIQUE KEY(pk) + DISTRIBUTED BY HASH(pk) BUCKETS 1 + PROPERTIES ( + "replication_allocation" = "tag.location.default: 1", + "enable_unique_key_merge_on_write" = "true", + "light_schema_change" = "true", + "store_row_column" = "true", + "row_store_page_size" = "16384" + ) + """ + + sql """ + INSERT INTO short_circuit_rowstore_nested_complex VALUES + (5, + named_struct( + 'nested_str', 'b-deep-5', + 'inner_s', named_struct('deep_str', 'b-inner-5', 'flag', true, 'deep_char', 'bd5'), + 'deep_arr', array(named_struct('verified', false, 'txt', 'b-deep-arr-5', 'char_tag', 'bt5')), + 'deep_map', map('b_key', named_struct('leaf', 'b-leaf-5', 'n', -5, 'char_leaf', 'bl5'))), + named_struct('str', 'b-s-5', 'char_leaf', 'bc5', 'num', -35, 'sibling', 'b-sib-5')) + """ + sql "SYNC" + + sql "SET enable_short_circuit_query=false" + order_qt_normal_path """ + SELECT /*+ SET_VAR(enable_nereids_planner=true) */ + pk, + CHAR_LENGTH(struct_element(element_at(struct_element(deep, 'deep_map'), 'dense'), 'leaf')) AS char_len, + LENGTH(LOWER(struct_element(element_at(struct_element(deep, 'deep_map'), 'dense'), 'leaf'))) AS lower_len, Review Comment: This query uses map key `dense`, but the inserted row only contains `deep_map = map('b_key', ...)`, so both map-derived outputs are `NULL` even if reading the stored map value through the new access-path column-store path is still broken. Please query the inserted key, for example `b_key`, and regenerate the expected output with non-null `char_len`/`lower_len` values so the regression actually covers nested map payload materialization. ########## be/src/service/point_query_executor.cpp: ########## @@ -530,48 +624,9 @@ Status PointQueryExecutor::_lookup_row_data() { _reusable->get_col_default_values(), _reusable->include_col_uids())); } if (!_reusable->missing_col_uids().empty()) { - if (!_reusable->runtime_state()->enable_short_circuit_query_access_column_store()) { Review Comment: This fallback is skipped for row-cache hits, so enabling `disable_storage_row_cache=false` can still return the old broken shape on repeated point queries. The first lookup calls `BaseTablet::lookup_row_data()` and inserts only the row-store JSONB blob into `RowCache`; the second lookup can hit `_cached_row_data`, deserialize only `_include_col_uids`, and `continue` before `lookup_missing_columns_by_rowid()` runs. Since this PR intentionally leaves access-path columns in `_missing_col_uids`, those columns are never read from column store on the cached path. Please either bypass row cache when missing columns exist, or keep enough row-location state to run this fallback after a cache hit as well. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
