kangkaisen commented on a change in pull request #4152:
URL: https://github.com/apache/incubator-doris/pull/4152#discussion_r459325003



##########
File path: be/src/runtime/buffered_tuple_stream2.h
##########
@@ -257,6 +257,11 @@ class BufferedTupleStream2 {
     bool has_tuple_footprint() const {
         return _fixed_tuple_row_size > 0 || !_string_slots.empty() || 
_nullable_tuple;
     }
+    /// Returns true if the row consumes any memory. If false, the stream only 
needs to
+    /// store the count of rows.
+    bool row_consumes_memory() const {

Review comment:
       **I think the implementation isn't consistent with the method comment 
and name.**
   
   Only if there has one tuple. _fixed_tuple_row_size will large than zero. 
   ```
       for (int i = 0; i < _desc.tuple_descriptors().size(); ++i) {
           const TupleDescriptor* tuple_desc = _desc.tuple_descriptors()[i];
           const int tuple_byte_size = tuple_desc->byte_size();
           _fixed_tuple_row_size += tuple_byte_size;
           if (!tuple_desc->string_slots().empty()) {
               _string_slots.push_back(make_pair(i, 
tuple_desc->string_slots()));
           }
           // if (!tuple_desc->collection_slots().empty()) {
           //     _collection_slots.push_back(make_pair(i, 
tuple_desc->collection_slots()));
           // }
       }
   ```
   




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

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