This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.2
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.2 by this push:
new 55a8d549577 [branch-4.2][fix](zonemap) Do not abort forced MIN/MAX on
a table with a delete predicate (#68958)
55a8d549577 is described below
commit 55a8d5495771f6a3830125510b0190db91ee5b65
Author: Chenyang Sun <[email protected]>
AuthorDate: Tue Oct 13 10:40:07 2026 +0800
[branch-4.2][fix](zonemap) Do not abort forced MIN/MAX on a table with a
delete predicate (#68958)
Introduced by #68324, the branch-4.2 pick of #67642: with
`force_pushdown_zonemap_minmax` on, a segment that has a delete
predicate is now answered by `VStatisticsIterator`.
The read schema is implemented differently on the two branches. On
master, the `ReadSchema` from #66472 keeps the delete predicate columns
out of `num_block_columns()`, so `VStatisticsIterator` only builds
iterators for the block columns. branch-4.2 has no such split: the
schema ends with those delete predicate columns, `VStatisticsIterator`
builds iterators for them too, and `DCHECK(block->columns() ==
_column_iterators.size())` aborts ASAN/debug BEs.
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
be/src/storage/iterator/vgeneric_iterators.cpp | 4 ++-
be/test/exec/scan/vgeneric_iterators_test.cpp | 27 ++++++++++++++++--
.../explain/test_pushdown_zonemap_minmax.out | 3 ++
.../explain/test_pushdown_zonemap_minmax.groovy | 33 ++++++++++++++++++++++
4 files changed, 64 insertions(+), 3 deletions(-)
diff --git a/be/src/storage/iterator/vgeneric_iterators.cpp
b/be/src/storage/iterator/vgeneric_iterators.cpp
index 941c36ed5e3..99793ecefe5 100644
--- a/be/src/storage/iterator/vgeneric_iterators.cpp
+++ b/be/src/storage/iterator/vgeneric_iterators.cpp
@@ -62,7 +62,9 @@ Status VStatisticsIterator::init(const StorageReadOptions&
opts) {
}
Status VStatisticsIterator::next_batch(Block* block) {
- DCHECK(block->columns() == _column_iterators.size());
+ // The schema can end with delete predicate columns the query does not
read. The block does
+ // not hold them, so only the block's columns are filled.
+ DCHECK_LE(block->columns(), _column_iterators.size());
if (_output_rows < _target_rows) {
block->clear_column_data();
auto columns_guard = block->mutate_columns_scoped();
diff --git a/be/test/exec/scan/vgeneric_iterators_test.cpp
b/be/test/exec/scan/vgeneric_iterators_test.cpp
index e0039a8e326..af4a5893d6e 100644
--- a/be/test/exec/scan/vgeneric_iterators_test.cpp
+++ b/be/test/exec/scan/vgeneric_iterators_test.cpp
@@ -235,10 +235,13 @@ protected:
// pushed-down `agg` would run on. `accept_cut_bound` is what statistics
collection sets:
// it takes an inexact min/max as an approximation instead of reading the
data.
// `with_delete` adds a delete predicate, which leaves the zone map
covering removed rows.
+ // `column_ids` is the schema the rowset reader builds: the columns the
query reads, then the
+ // delete predicate columns it does not read.
std::unique_ptr<RowwiseIterator> pushdown_iterator_for(
const std::string& name, const std::vector<std::string>& values,
bool accept_cut_bound = false, bool with_delete = false,
- TPushAggOp::type agg = TPushAggOp::MINMAX) {
+ TPushAggOp::type agg = TPushAggOp::MINMAX,
+ const std::vector<ColumnId>& column_ids = {0, 1}) {
auto tablet_schema = make_schema();
const std::string segment_path = std::string(kTestDir) + "/" + name +
".dat";
@@ -271,7 +274,6 @@ protected:
tablet_schema,
io::FileReaderOptions {}, &segment)
.ok());
- std::vector<ColumnId> column_ids {0, 1};
// VStatisticsIterator keeps a reference to the schema, so it has to
outlive the iterator.
auto schema = std::make_shared<Schema>(tablet_schema->columns(),
column_ids);
StorageReadOptions read_options;
@@ -386,6 +388,27 @@ TEST_F(StatisticsIteratorStringBoundsTest,
CountKeepsTheDeletePredicateGuardWhen
<< "COUNT reports the segment row count, which still counts the
deleted rows";
}
+// The query reads c2 and the delete predicate is on c1. The rowset reader
appends c1 to the
+// schema, but the block only holds c2, so only c2 gets filled.
+TEST_F(StatisticsIteratorStringBoundsTest,
DeletePredicateColumnAfterTheBlockIsNotFilled) {
+ auto iter = pushdown_iterator_for("del_column", {"aaa", "bbb"},
/*accept_cut_bound=*/true,
+ /*with_delete=*/true, TPushAggOp::MINMAX,
+ /*column_ids=*/ {1, 0});
+ ASSERT_NE(dynamic_cast<VStatisticsIterator*>(iter.get()), nullptr);
+
+ Block block;
+ const auto* read_column = iter->schema().column(1);
+ auto data_type = read_column->get_vec_type();
+ block.insert(ColumnWithTypeAndName(data_type->create_column(), data_type,
read_column->name()));
+ ASSERT_TRUE(iter->next_batch(&block).ok());
+ ASSERT_EQ(1, block.columns());
+ ASSERT_EQ(2, block.rows());
+ // The zone map gives the max first, then the min.
+ EXPECT_EQ("bbb",
block.get_by_position(0).column->get_data_at(0).to_string());
+ EXPECT_EQ("aaa",
block.get_by_position(0).column->get_data_at(1).to_string());
+ EXPECT_TRUE(iter->next_batch(&block).is<ErrorCode::END_OF_FILE>());
+}
+
TEST(VGenericIteratorsTest, Union) {
auto schema = create_schema();
auto output_schema = std::make_shared<Schema>(schema);
diff --git
a/regression-test/data/query_p0/explain/test_pushdown_zonemap_minmax.out
b/regression-test/data/query_p0/explain/test_pushdown_zonemap_minmax.out
index 1c9976143ee..59e6cb15e2b 100644
--- a/regression-test/data/query_p0/explain/test_pushdown_zonemap_minmax.out
+++ b/regression-test/data/query_p0/explain/test_pushdown_zonemap_minmax.out
@@ -17,3 +17,6 @@ aaa 600 zzzz
-- !str_on --
512 zzz{
+-- !delete_other_column --
+2022-04-27T16:00:33 2022-04-27T16:00:59
+
diff --git
a/regression-test/suites/query_p0/explain/test_pushdown_zonemap_minmax.groovy
b/regression-test/suites/query_p0/explain/test_pushdown_zonemap_minmax.groovy
index c10d180ef99..a0861cdd576 100644
---
a/regression-test/suites/query_p0/explain/test_pushdown_zonemap_minmax.groovy
+++
b/regression-test/suites/query_p0/explain/test_pushdown_zonemap_minmax.groovy
@@ -99,4 +99,37 @@ suite("test_pushdown_zonemap_minmax") {
}
qt_str_on "select length(max(v)), right(max(v), 4) from
test_string_minmax_str"
sql "set force_pushdown_zonemap_minmax = false"
+
+ // The delete predicate is on k and the query reads only v. The scan adds
k to the columns it
+ // reads, and the zone map answer must fill only v.
+ sql "DROP TABLE IF EXISTS test_minmax_delete_other_column"
+ // Auto compaction is off: it would apply the delete to the data and drop
the predicate.
+ sql """
+ CREATE TABLE test_minmax_delete_other_column (
+ `k` VARCHAR(20) NOT NULL,
+ `v` DATETIME NOT NULL
+ ) ENGINE=OLAP
+ DUPLICATE KEY(`k`)
+ DISTRIBUTED BY HASH(`k`) BUCKETS 1
+ PROPERTIES (
+ "replication_allocation" = "tag.location.default: 1",
+ "disable_auto_compaction" = "true"
+ );
+ """
+ // The zone map still covers the deleted row, so it is neither the min nor
the max.
+ sql """
+ INSERT INTO test_minmax_delete_other_column VALUES
+ ('a', '2022-04-27 16:00:33'), ('b', '2022-04-27 16:00:59'), ('1',
'2022-04-27 16:00:40')
+ """
+ sql "DELETE FROM test_minmax_delete_other_column WHERE k = '1'"
+ // Without this, FE answers MIN/MAX from its cache and the query never
reaches BE.
+ sql "set disable_nereids_rules = 'REWRITE_SIMPLE_AGG_TO_CONSTANT'"
+ sql "set force_pushdown_zonemap_minmax = true"
+ explain {
+ sql "select min(v), max(v) from test_minmax_delete_other_column"
+ contains "pushAggOp=MINMAX"
+ }
+ qt_delete_other_column "select min(v), max(v) from
test_minmax_delete_other_column"
+ sql "set force_pushdown_zonemap_minmax = false"
+ sql "set disable_nereids_rules = ''"
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
