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/09 09:37:13 UTC
[GitHub] [doris] wangbo opened a new pull request, #10717: [refactor] remove PredicateColumn
wangbo opened a new pull request, #10717:
URL: https://github.com/apache/doris/pull/10717
# Proposed changes
remove PredicateColumn
## Problem Summary:
Describe the overview of changes.
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
2. Has unit tests been added: (Yes/No/No Need)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (Yes/No)
5. Are there any changes that cannot be rolled back: (Yes/No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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
[GitHub] [doris] wangbo commented on a diff in pull request #10717: [refactor] remove PredicateColumn
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [doris] yiguolei commented on a diff in pull request #10717: [refactor] remove PredicateColumn
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #10717:
URL: https://github.com/apache/doris/pull/10717#discussion_r917336066
##########
be/src/olap/bloom_filter_predicate.h:
##########
@@ -81,18 +80,42 @@ class BloomFilterColumnPredicate : public ColumnPredicate {
new_size += _specific_filter->find_uint32_t(dict_col->get_hash_value(idx));
}
}
+ } else if (column.is_column_string()) {
+ if constexpr (std::is_same_v<file_type, StringValue>) {
Review Comment:
file_type or fieldType?
--
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
[GitHub] [doris] github-actions[bot] closed pull request #10717: [refactor] remove PredicateColumn
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #10717: [refactor] remove PredicateColumn
URL: https://github.com/apache/doris/pull/10717
--
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
[GitHub] [doris] yiguolei commented on a diff in pull request #10717: [refactor] remove PredicateColumn
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #10717:
URL: https://github.com/apache/doris/pull/10717#discussion_r917335637
##########
be/src/olap/schema.cpp:
##########
@@ -127,48 +126,45 @@ vectorized::IColumn::MutablePtr Schema::get_predicate_column_nullable_ptr(FieldT
vectorized::IColumn::MutablePtr Schema::get_predicate_column_ptr(FieldType type) {
switch (type) {
case OLAP_FIELD_TYPE_BOOL:
- return doris::vectorized::PredicateColumnType<bool>::create();
+ return doris::vectorized::ColumnVector<doris::vectorized::UInt8>::create();
Review Comment:
move all these method to TabletColumn
--
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
[GitHub] [doris] yiguolei commented on a diff in pull request #10717: [refactor] remove PredicateColumn
Posted by GitBox <gi...@apache.org>.
yiguolei commented on code in PR #10717:
URL: https://github.com/apache/doris/pull/10717#discussion_r917335252
##########
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:
我倾向在 column 中做这种判断，比如我们可以调用insert fixed length data， 但是在这个函数里我们判断一下column的类型，做if else 判断
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #10717: [refactor] remove PredicateColumn
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10717:
URL: https://github.com/apache/doris/pull/10717#issuecomment-1374282247
We're closing this PR because it hasn't been updated in a while.
This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and feel free a maintainer to remove the Stale tag!
--
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