github-actions[bot] commented on code in PR #66881:
URL: https://github.com/apache/doris/pull/66881#discussion_r3842849522
##########
be/test/storage/segment/segment_iterator_expr_zonemap_test.cpp:
##########
@@ -368,4 +369,102 @@ TEST_F(SegmentIteratorExprZonemapTest,
NewIteratorPrunesCommitTsoByReadOptionVal
EXPECT_EQ(1, _stats.filtered_segment_number);
}
+TEST_F(SegmentIteratorExprZonemapTest, PointSeekRebuildsPrefixSeekSchema) {
+ std::shared_ptr<Segment> segment;
+ ASSERT_NO_FATAL_FAILURE(build_segment(&segment));
+ auto read_schema = make_read_schema(_tablet_schema);
+
+ StorageReadOptions read_options;
+ read_options.stats = &_stats;
+ read_options.tablet_schema = _tablet_schema;
+ read_options.io_ctx.reader_type = ReaderType::READER_QUERY;
+
+ SegmentIterator iter(segment, read_schema);
+ auto status = iter.init(read_options);
+ ASSERT_TRUE(status.ok()) << status;
+
+ RowCursor prefix_key;
+ std::vector<Field> prefix_fields {int_field(100)};
+ ASSERT_TRUE(prefix_key.init_scan_key(_tablet_schema,
std::move(prefix_fields)).ok());
+ StorageReadOptions::KeyRange prefix_range(&prefix_key, true, &prefix_key,
true);
+ ASSERT_TRUE(iter._prepare_seek(prefix_range).ok());
+ ASSERT_EQ(1, iter._seek_schema->num_column_ids());
+ ASSERT_EQ(1, iter._seek_block.size());
+
+ RowCursor point_key;
+ std::vector<Field> point_fields {int_field(100), int_field(0)};
+ ASSERT_TRUE(point_key.init_scan_key(_tablet_schema,
std::move(point_fields)).ok());
+ StorageReadOptions::KeyRange point_range(&point_key, true, &point_key,
true);
+ ASSERT_TRUE(iter._prepare_seek(point_range).ok());
+
+ EXPECT_EQ(2, iter._seek_schema->num_column_ids());
+ EXPECT_EQ(2, iter._seek_block.size());
+}
+
+TEST_F(SegmentIteratorExprZonemapTest, PointKeysBuildExactMonotonicRanges) {
+ std::shared_ptr<Segment> segment;
+ ASSERT_NO_FATAL_FAILURE(build_segment(&segment));
+ auto read_schema = make_read_schema(_tablet_schema);
+
+ auto key_schema = RowCursor::create_shared_schema(_tablet_schema, 2);
+ auto point_keys = std::make_shared<PointKeySet>(key_schema);
+ for (const auto& [first, second] :
+ std::vector<std::pair<int32_t, int32_t>> {{10, 0}, {4095, 0}, {5000,
0}, {5000, 1000}}) {
+ std::vector<Field> fields {int_field(first), int_field(second)};
+ RowCursor key;
+ ASSERT_TRUE(key.init(key_schema, std::move(fields)).ok());
+ point_keys->keys.emplace_back(std::move(key));
+ }
+
+ StorageReadOptions read_options;
+ read_options.stats = &_stats;
+ read_options.tablet_schema = _tablet_schema;
+ read_options.io_ctx.reader_type = ReaderType::READER_QUERY;
+ read_options.point_keys = point_keys;
+
+ SegmentIterator iter(segment, read_schema);
+ auto status = iter.init(read_options);
+ ASSERT_TRUE(status.ok()) << status;
+ iter._row_bitmap.addRange(0, segment->num_rows());
+
+ status = iter._get_row_ranges_by_point_keys();
Review Comment:
[P1] Load the segment index before calling this helper directly. Production
reaches this path through `Segment::new_iterator()`, which calls
`load_index()`, but this test constructs `SegmentIterator` itself and invokes
`_get_row_ranges_by_point_keys()` immediately. On the current head, BE UT build
1028343 aborts here because `_lookup_ordinal()` reaches
`Segment::get_short_key_index()` and its `load_index` DCHECK fails, so this
test and the rest of the binary never complete. Please load the index
explicitly (or exercise the production iterator-creation path) before the point
lookup.
##########
regression-test/suites/unique_seq_map_p0/test_seq_map_candidate_key_scan.groovy:
##########
@@ -0,0 +1,331 @@
+// 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.
+
+import java.util.regex.Pattern
+import org.apache.doris.regression.action.ProfileAction
+
+suite("test_seq_map_candidate_key_scan") {
+ sql "DROP TABLE IF EXISTS test_seq_map_candidate_key_scan"
+ sql "DROP TABLE IF EXISTS test_seq_map_candidate_key_scan_composite"
+ sql "DROP TABLE IF EXISTS test_seq_map_candidate_key_scan_multi_tablet"
+ sql "DROP TABLE IF EXISTS test_seq_map_candidate_key_scan_cost_fallback"
+ try {
+ sql """
+ CREATE TABLE test_seq_map_candidate_key_scan (
+ `id` BIGINT NOT NULL,
+ `c` INT NULL,
+ `d` INT NULL,
+ `e` INT NULL,
+ `s1` BIGINT NULL,
+ `s2` BIGINT NULL,
+ INDEX idx_c (`c`) USING INVERTED,
+ INDEX idx_d (`d`) USING INVERTED,
+ INDEX idx_e (`e`) USING INVERTED
+ ) ENGINE=OLAP
+ UNIQUE KEY(`id`)
+ DISTRIBUTED BY HASH(`id`) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1",
+ "enable_unique_key_merge_on_write" = "false",
+ "light_schema_change" = "true",
+ "disable_auto_compaction" = "true",
+ "inverted_index_storage_format" = "V3",
+ "sequence_mapping.s1" = "c,d",
+ "sequence_mapping.s2" = "e"
+ )
+ """
+
+ // Group s1 and group s2 deliberately arrive in different physical
rows.
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, c, d, s1) VALUES
(1, 20, 200, 20)"
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, e, s2) VALUES (1,
300, 30)"
+
+ // id=2 has a stale physical row matching c=20. Its latest s1 value is
c=99,
+ // so candidate collection may include it but the final residual must
remove it.
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, c, d, s1) VALUES
(2, 20, 200, 10)"
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, c, d, s1) VALUES
(2, 99, 999, 20)"
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, e, s2) VALUES (2,
300, 30)"
+
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, c, d, s1) VALUES
(3, 20, 201, 20)"
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, e, s2) VALUES (3,
300, 30)"
+ // Keep the table large enough that the weighted point-probe admission
model can still
+ // exercise selective candidate scans instead of conservatively
falling back.
+ sql """
+ INSERT INTO test_seq_map_candidate_key_scan(id, c, d, e, s1, s2)
+ SELECT number + 1000, 999, 999, 999, 1, 1 FROM numbers("number" =
"1000")
+ """
+
+ def query = "SELECT id FROM test_seq_map_candidate_key_scan WHERE c =
20 AND e = 300 ORDER BY id"
+
+ sql "SET enable_sql_cache = false"
+ sql "SET enable_seq_map_candidate_key_scan = false"
+ qt_candidate_disabled query
+
+ sql "SET enable_seq_map_candidate_key_scan = true"
+ sql "SET enable_inverted_index_query = true"
+ sql "SET enable_profile = true"
+
+ def counterValue = { String profileString, String counterName ->
+ def matcher =
Pattern.compile("${counterName}:\\s*(\\d+)").matcher(profileString)
+ assertTrue(matcher.find(), "${counterName} is absent from profile")
+ return Long.parseLong(matcher.group(1))
+ }
+
+ def profileAction = new ProfileAction(context)
+ def runWithProfile = { String tag, String statement, Closure
profileCheck ->
+ def queryId = "${tag}_${System.currentTimeMillis()}"
+ sql "/* ${queryId} */ ${statement}"
+ def profileString =
+ profileAction.getProfileBySql(queryId,
["SeqMapCandidateFallbacks"])
+ profileCheck.call(profileString)
+ }
+
+ qt_candidate_two_groups query
+ runWithProfile("seq_map_candidate_two_groups", query) { profileString
->
+ assertEquals(2L, counterValue(profileString,
"SeqMapCandidateDriverGroups"))
+ assertEquals(2L, counterValue(profileString,
"SeqMapCandidateDriverPredicates"))
+ assertEquals(3L, counterValue(profileString,
"SeqMapCandidateKeysAfterIntersect"))
+ assertTrue(counterValue(profileString, "SeqMapCandidateKeyBytes")
> 0)
+ assertTrue(counterValue(profileString, "SeqMapCandidateScanRows")
> 0)
+ assertEquals(0L, counterValue(profileString,
"SeqMapCandidateIndexDowngrades"))
+ }
+
+ // Same-group predicates must be evaluated on the same physical group
row.
+ def sameGroupQuery =
+ "SELECT id FROM test_seq_map_candidate_key_scan WHERE c = 20
AND d = 200 ORDER BY id"
+ qt_candidate_same_group sameGroupQuery
+ runWithProfile("seq_map_candidate_same_group", sameGroupQuery) {
profileString ->
+ assertEquals(1L, counterValue(profileString,
"SeqMapCandidateDriverGroups"))
+ assertEquals(2L, counterValue(profileString,
"SeqMapCandidateDriverPredicates"))
+ assertEquals(2L, counterValue(profileString,
"SeqMapCandidateKeysAfterIntersect"))
+ }
+
+ // Empty candidates can short-circuit this tablet.
+ def emptyCandidateQuery = "SELECT id FROM
test_seq_map_candidate_key_scan WHERE c = 777"
+ qt_candidate_empty emptyCandidateQuery
+ runWithProfile("seq_map_candidate_empty", emptyCandidateQuery) {
profileString ->
+ assertEquals(0L, counterValue(profileString,
"SeqMapCandidateKeysAfterIntersect"))
+ assertEquals(1L, counterValue(profileString,
"SeqMapCandidatePrunedTablets"))
+ assertEquals(0L, counterValue(profileString,
"SeqMapCandidateFallbacks"))
+ }
+
+ // Force fallback and prove that the candidate-key limit branch was
taken.
+ sql "SET seq_map_candidate_key_max_count = 1"
+ try {
+ def fallbackQuery =
+ "SELECT id FROM test_seq_map_candidate_key_scan WHERE c IN
(20, 99) ORDER BY id"
+ qt_candidate_limit_fallback fallbackQuery
+ runWithProfile("seq_map_candidate_limit", fallbackQuery) {
profileString ->
+ assertTrue(counterValue(profileString,
"SeqMapCandidateFallbacks") > 0)
+ assertTrue(profileString.contains("candidate_key_limit"),
+ "candidate_key_limit fallback reason is absent from
profile")
+ }
+ } finally {
+ sql "SET seq_map_candidate_key_max_count = 100000"
+ }
+
+ // Existing FE key ranges use the normal scan because their exact
baseline is already
+ // selective and cannot be conservatively estimated from whole-rowset
counts.
+ def keyRangeQuery = """
+ SELECT id FROM test_seq_map_candidate_key_scan
+ WHERE id BETWEEN 2 AND 3 AND c = 20 AND e = 300
+ ORDER BY id
+ """
+ qt_candidate_key_range_fallback keyRangeQuery
+ runWithProfile("seq_map_candidate_key_range", keyRangeQuery) {
profileString ->
+ assertTrue(counterValue(profileString, "SeqMapCandidateFallbacks")
> 0)
+ assertTrue(profileString.contains("key_range_present"),
+ "key_range_present fallback reason is absent from profile")
+ }
+
+ def keyInQuery = """
+ SELECT id FROM test_seq_map_candidate_key_scan
+ WHERE id IN (1, 3) AND c = 20 AND e = 300
+ ORDER BY id
+ """
+ qt_candidate_key_in_fallback keyInQuery
+ runWithProfile("seq_map_candidate_key_in", keyInQuery) { profileString
->
+ assertTrue(counterValue(profileString, "SeqMapCandidateFallbacks")
> 0)
+ assertTrue(profileString.contains("key_range_present"),
+ "key_range_present fallback reason is absent from profile")
+ }
+
+ sql "SET enable_inverted_index_query = false"
+ try {
+ qt_candidate_index_disabled query
+ runWithProfile("seq_map_candidate_index_disabled", query) {
profileString ->
+ assertTrue(counterValue(profileString,
"SeqMapCandidateFallbacks") > 0)
+
assertTrue(profileString.contains("inverted_index_query_disabled"),
+ "inverted_index_query_disabled fallback reason is
absent from profile")
+ }
+ } finally {
+ sql "SET enable_inverted_index_query = true"
+ }
+
+ // Range predicates are intentionally residual-only in the first
version.
+ def noDriverQuery = "SELECT id FROM test_seq_map_candidate_key_scan
WHERE c > 20 ORDER BY id"
+ qt_candidate_no_driver noDriverQuery
Review Comment:
[P1] Keep the filler rows out of this expected result, or update the result
to include them. Lines 66-69 insert IDs 1000 through 1999 with `c = 999`, so
this `c > 20` query returns those 1,000 rows in addition to ID 2, while the
committed `.out` contains only `2`. The QT therefore fails before
`runWithProfile` can verify the no-driver fallback. Please use a filler `c`
value that does not satisfy this predicate (for example `0`) or regenerate the
expected output, then rerun the suite.
##########
regression-test/suites/unique_seq_map_p0/test_seq_map_candidate_key_scan.groovy:
##########
@@ -0,0 +1,331 @@
+// 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.
+
+import java.util.regex.Pattern
+import org.apache.doris.regression.action.ProfileAction
+
+suite("test_seq_map_candidate_key_scan") {
+ sql "DROP TABLE IF EXISTS test_seq_map_candidate_key_scan"
+ sql "DROP TABLE IF EXISTS test_seq_map_candidate_key_scan_composite"
+ sql "DROP TABLE IF EXISTS test_seq_map_candidate_key_scan_multi_tablet"
+ sql "DROP TABLE IF EXISTS test_seq_map_candidate_key_scan_cost_fallback"
+ try {
+ sql """
+ CREATE TABLE test_seq_map_candidate_key_scan (
+ `id` BIGINT NOT NULL,
+ `c` INT NULL,
+ `d` INT NULL,
+ `e` INT NULL,
+ `s1` BIGINT NULL,
+ `s2` BIGINT NULL,
+ INDEX idx_c (`c`) USING INVERTED,
+ INDEX idx_d (`d`) USING INVERTED,
+ INDEX idx_e (`e`) USING INVERTED
+ ) ENGINE=OLAP
+ UNIQUE KEY(`id`)
+ DISTRIBUTED BY HASH(`id`) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1",
+ "enable_unique_key_merge_on_write" = "false",
+ "light_schema_change" = "true",
+ "disable_auto_compaction" = "true",
+ "inverted_index_storage_format" = "V3",
+ "sequence_mapping.s1" = "c,d",
+ "sequence_mapping.s2" = "e"
+ )
+ """
+
+ // Group s1 and group s2 deliberately arrive in different physical
rows.
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, c, d, s1) VALUES
(1, 20, 200, 20)"
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, e, s2) VALUES (1,
300, 30)"
+
+ // id=2 has a stale physical row matching c=20. Its latest s1 value is
c=99,
+ // so candidate collection may include it but the final residual must
remove it.
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, c, d, s1) VALUES
(2, 20, 200, 10)"
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, c, d, s1) VALUES
(2, 99, 999, 20)"
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, e, s2) VALUES (2,
300, 30)"
+
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, c, d, s1) VALUES
(3, 20, 201, 20)"
+ sql "INSERT INTO test_seq_map_candidate_key_scan(id, e, s2) VALUES (3,
300, 30)"
+ // Keep the table large enough that the weighted point-probe admission
model can still
+ // exercise selective candidate scans instead of conservatively
falling back.
+ sql """
+ INSERT INTO test_seq_map_candidate_key_scan(id, c, d, e, s1, s2)
+ SELECT number + 1000, 999, 999, 999, 1, 1 FROM numbers("number" =
"1000")
+ """
+
+ def query = "SELECT id FROM test_seq_map_candidate_key_scan WHERE c =
20 AND e = 300 ORDER BY id"
+
+ sql "SET enable_sql_cache = false"
+ sql "SET enable_seq_map_candidate_key_scan = false"
+ qt_candidate_disabled query
+
+ sql "SET enable_seq_map_candidate_key_scan = true"
+ sql "SET enable_inverted_index_query = true"
+ sql "SET enable_profile = true"
+
+ def counterValue = { String profileString, String counterName ->
+ def matcher =
Pattern.compile("${counterName}:\\s*(\\d+)").matcher(profileString)
+ assertTrue(matcher.find(), "${counterName} is absent from profile")
+ return Long.parseLong(matcher.group(1))
+ }
+
+ def profileAction = new ProfileAction(context)
+ def runWithProfile = { String tag, String statement, Closure
profileCheck ->
+ def queryId = "${tag}_${System.currentTimeMillis()}"
+ sql "/* ${queryId} */ ${statement}"
+ def profileString =
+ profileAction.getProfileBySql(queryId,
["SeqMapCandidateFallbacks"])
+ profileCheck.call(profileString)
+ }
+
+ qt_candidate_two_groups query
+ runWithProfile("seq_map_candidate_two_groups", query) { profileString
->
+ assertEquals(2L, counterValue(profileString,
"SeqMapCandidateDriverGroups"))
+ assertEquals(2L, counterValue(profileString,
"SeqMapCandidateDriverPredicates"))
+ assertEquals(3L, counterValue(profileString,
"SeqMapCandidateKeysAfterIntersect"))
Review Comment:
[P1] Make this positive-path fixture prove candidate admission in cloud
mode. The preceding QT passes for the same SQL, but on the current head
cloud_p0 build 1028413 reaches this line with
`SeqMapCandidateKeysAfterIntersect: 0`, so the suite fails and the profiled
execution has not established that any point keys were installed. Please also
validate that execution's rows, assert `SeqMapCandidateFallbacks == 0`
(surfacing any reason), and shape the fixture so both local and cloud runs
install the three keys; otherwise this test neither passes the gate nor proves
the optimization ran.
--
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]
