xinyiZzz commented on code in PR #29328:
URL: https://github.com/apache/doris/pull/29328#discussion_r1438245669


##########
be/src/olap/rowset/segment_v2/segment_iterator.cpp:
##########
@@ -2184,21 +2175,20 @@ Status 
SegmentIterator::_next_batch_internal(vectorized::Block* block) {
         _replace_version_col(_current_batch_rows_read);
     }
 
-    // If col >= block->columns(), it means col should not be filtered, there 
is a BUG.
-    // such as delete condition column was incorrectly put into 
columns_to_filter,
-    // which is usually at the end of the block. only check during the first 
next_batch.
+    // Additional deleted filter condition will be materialized column be at 
the end of the block,
+    // after _output_column_by_sel_idx  will be erase, we not need to filter 
it,
+    // so erase it from _columns_to_filter in the first next_batch.
+    // Eg:
+    //      `delete from table where a = 10;`
+    //      `select b from table;`
+    // a column only effective in segment iterator, the block from query 
engine only contain the b column,
+    // so no need to filter a column by expr.
     if (_opts.stats->blocks_load == 0) {
-        for (const auto& col : _columns_to_filter) {
-            if (col >= block->columns()) {
-                std::ostringstream ss;
-                for (const auto& i : _columns_to_filter) {
-                    ss << i << "-";
-                }
-                throw Exception(
-                        ErrorCode::INTERNAL_ERROR,
-                        "filter block column id(index) greater than 
block->columns(), "
-                        "column id={}, all columns that need filter={}, block 
columns num={}",
-                        col, ss.str().substr(0, ss.str().length() - 1), 
block->columns());
+        for (auto it = _columns_to_filter.begin(); it != 
_columns_to_filter.end();) {
+            if (*it >= block->columns()) {
+                it = _columns_to_filter.erase(it);

Review Comment:
   1. Can’t core. return value of `erase` points to the iterator of the next 
element of the currently deleted element, and assigns this return value to `it`.
   2. Traversing earse here is O(n)



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to