zuochunwei commented on a change in pull request #7613:
URL: https://github.com/apache/incubator-doris/pull/7613#discussion_r781783797



##########
File path: be/src/olap/column_predicate.h
##########
@@ -69,6 +69,8 @@ class ColumnPredicate {
     virtual void evaluate_vec(vectorized::IColumn& column, uint16_t size, 
bool* flags) const {};
     uint32_t column_id() const { return _column_id; }
 
+    virtual bool is_in_predicate() { return false; }

Review comment:
       is_in_predicate should be declared as const because it seems read-only

##########
File path: be/src/olap/rowset/segment_v2/bitshuffle_page.h
##########
@@ -359,17 +361,28 @@ class BitShufflePageDecoder : public PageDecoder {
  
         int begin = _cur_index;
         int end = _cur_index + max_fetch;
- 
+
+        auto* dst_col_ptr = dst.get();
+        if (dst->is_nullable()) {
+            auto nullable_column = 
assert_cast<vectorized::ColumnNullable*>(dst.get());
+            dst_col_ptr = nullable_column->get_nested_column_ptr().get();
+
+            // fill null bitmap here, not null;
+            for (int j = begin; j < end; j++) {

Review comment:
       consider using add_num_element instead

##########
File path: be/src/vec/columns/column_nullable.h
##########
@@ -130,7 +130,13 @@ class ColumnNullable final : public COWHelper<IColumn, 
ColumnNullable> {
         return false;
     }
 
+    bool is_date_type() override { return get_nested_column().is_date_type(); }

Review comment:
       same problem with above

##########
File path: be/src/vec/columns/column.h
##########
@@ -411,10 +413,15 @@ class IColumn : public COW<IColumn> {
     // only used in ColumnNullable replace_column_data
     virtual void replace_column_data_default(size_t self_row = 0) = 0;
 
-    bool is_date_type() { return is_date; }
+    virtual bool is_date_type() { return is_date; }

Review comment:
       is_xxx should be marked as const

##########
File path: be/src/olap/schema.cpp
##########
@@ -147,4 +155,57 @@ vectorized::DataTypePtr 
Schema::get_data_type_ptr(FieldType type) {
     return nullptr;
 }
 
+vectorized::IColumn::MutablePtr 
Schema::get_predicate_column_nullable_ptr(FieldType type, bool is_null) {
+    vectorized::IColumn::MutablePtr ptr = 
Schema::get_predicate_column_ptr(type);
+    if (is_null) {
+        return doris::vectorized::ColumnNullable::create(std::move(ptr), 
doris::vectorized::ColumnUInt8::create());
+    }
+    return ptr;
+}
+
+vectorized::IColumn::MutablePtr Schema::get_predicate_column_ptr(FieldType 
type) {
+    switch (type) {
+        case OLAP_FIELD_TYPE_TINYINT:
+            return 
doris::vectorized::PredicateColumnType<doris::vectorized::Int8>::create();

Review comment:
       too much switch type case xxx, i have seen many times such functions, 
building a mapping may a good choice.
   template technology may be useful to satisfied this need




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