You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/07/10 04:31:54 UTC

[GitHub] [doris] wangbo commented on a diff in pull request #10717: [refactor] remove PredicateColumn

wangbo commented on code in PR #10717:
URL: https://github.com/apache/doris/pull/10717#discussion_r917337848


##########
be/src/olap/rowset/segment_v2/bitshuffle_page.h:
##########
@@ -392,7 +392,14 @@ class BitShufflePageDecoder : public PageDecoder {
 
         size_t max_fetch = std::min(*n, static_cast<size_t>(_num_elements - _cur_index));
 
-        dst->insert_many_fix_len_data(get_data(_cur_index), max_fetch);
+        // todo(wb) remove this branch after the data format is completely unified
+        if constexpr (Type == OLAP_FIELD_TYPE_DATE) {
+            dst->insert_many_date(get_data(_cur_index), max_fetch);
+        } else if constexpr (Type == OLAP_FIELD_TYPE_DATETIME) {

Review Comment:
   1 分支放这的好处是可以用constexpr
   2 如果要放column里,如果不加模板,单纯用if判断的话,就没法用constexpr
   如果要用模板的话,那只能给column加个模板,这个改动太大了,所有用到column的地方都得加一个模板标识<>
   另外模板应该没法给insert方法加,因为那个是虚函数
   3 这个问题我觉得从最终方案考虑,如果最终我们决定对于date类型还是用columnvector保存,那么一定会出现一个columnvector保存多种类型的情况,那么就需要columnvector能够保存一个类型的模板,这个问题就解决了。
   如果我们计划加一个新的类型比如dateColumn用于保存date类型,那么我建议保持现状,暂时先用这个分支



-- 
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-unsubscribe@doris.apache.org

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


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