zhannngchen commented on code in PR #16938:
URL: https://github.com/apache/doris/pull/16938#discussion_r1112505282


##########
be/src/vec/olap/vgeneric_iterators.cpp:
##########
@@ -374,6 +374,11 @@ Status VUnionIterator::init(const StorageReadOptions& 
opts) {
         return Status::OK();
     }
 
+    // we use back() and pop_back() of std::vector to handle each iterator,

Review Comment:
   we'd better to make iterator's order right outside of the UnionIterator?
   It's tricky to do this kind of operation inside UnionIterator, if some other 
developers changed the iterator's order outside of UnionIterator for some 
reason, the result will be wrong.
   Consider to do this operation at `beta_rowset_reader.cpp:L218`, just before 
`  _iterator = vectorized::new_union_iterator(std::move(iterators));`, and we 
should add a DCHECK on `iter.data_id()` to check that the iterators order right.



-- 
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