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/12/01 06:38:19 UTC
[GitHub] [doris] nextdreamblue opened a new pull request, #14722: Add key same label v merge iterator
nextdreamblue opened a new pull request, #14722:
URL: https://github.com/apache/doris/pull/14722
Signed-off-by: nextdreamblue <zx...@163.com>
# Proposed changes
Issue Number: close #xxx
## Problem summary
Describe your changes.
## Checklist(Required)
1. Does it affect the original behavior:
- [ ] Yes
- [x] No
- [ ] I don't know
2. Has unit tests been added:
- [ ] Yes
- [ ] No
- [x] No Need
3. Has document been added or modified:
- [ ] Yes
- [ ] No
- [x] No Need
4. Does it need to update dependencies:
- [ ] Yes
- [x] No
5. Are there any changes that cannot be rolled back:
- [ ] Yes (If Yes, please explain WHY)
- [x] 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] dataroaring merged pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
dataroaring merged PR #14722:
URL: https://github.com/apache/doris/pull/14722
--
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 #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14722:
URL: https://github.com/apache/doris/pull/14722#issuecomment-1354151341
clang-tidy review says "All clean, LGTM! :+1:"
--
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] nextdreamblue commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
nextdreamblue commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1057516755
##########
be/src/vec/core/block.cpp:
##########
@@ -353,6 +360,10 @@ void Block::skip_num_rows(int64_t& length) {
elem.column = elem.column->cut(length, origin_rows - length);
}
}
+ if (length < row_same_bit.size()) {
+ std::vector<bool> tmp_row_same_bit(row_same_bit.begin() + length, row_same_bit.end());
Review Comment:
> ditto
i fix it
--
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 #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14722:
URL: https://github.com/apache/doris/pull/14722#issuecomment-1364995077
PR approved by anyone and no changes requested.
--
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] liaoxin01 commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
liaoxin01 commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1039311762
##########
be/src/vec/olap/vgeneric_iterators.cpp:
##########
@@ -123,6 +123,7 @@ bool VMergeIteratorContext::compare(const VMergeIteratorContext& rhs) const {
if (_is_unique) {
Review Comment:
Can we use `_is_unique` to skip the same key in the agg model?
--
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] nextdreamblue commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
nextdreamblue commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1057516896
##########
be/src/vec/olap/vgeneric_iterators.h:
##########
@@ -120,6 +120,21 @@ class VMergeIteratorContext {
void set_skip(bool skip) const { _skip = skip; }
+ bool is_same() const { return _same; }
+
+ void set_same(bool same) const { _same = same; }
+
+ std::vector<bool> get_pre_ctx_same() const { return _pre_ctx_same_bit; }
Review Comment:
ok, i fix it
--
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] hello-stephen commented on pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
hello-stephen commented on PR #14722:
URL: https://github.com/apache/doris/pull/14722#issuecomment-1333338910
TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 34.87 seconds
load time: 436 seconds
storage size: 17123343172 Bytes
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20221201074534_clickbench_pr_56072.html
--
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] nextdreamblue commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
nextdreamblue commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1049502073
##########
be/src/vec/core/block.h:
##########
@@ -63,6 +63,7 @@ class Block {
Container data;
IndexByName index_by_name;
+ std::vector<uint64_t> row_same_bit;
Review Comment:
我试试改造下代码
--
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] nextdreamblue commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
nextdreamblue commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1049592521
##########
be/src/vec/core/block.h:
##########
@@ -63,6 +63,7 @@ class Block {
Container data;
IndexByName index_by_name;
+ std::vector<uint64_t> row_same_bit;
Review Comment:
`std::vector<bool>`，感觉用这个特殊的类型就可以实现位图效果了，我改造下
https://en.cppreference.com/w/cpp/container/vector_bool
--
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] zhannngchen commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
zhannngchen commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1057405813
##########
be/src/vec/core/block.cpp:
##########
@@ -339,6 +342,10 @@ void Block::set_num_rows(size_t length) {
elem.column = elem.column->cut(0, length);
}
}
+ if (length < row_same_bit.size()) {
+ std::vector<bool> tmp_row_same_bit(row_same_bit.begin(), row_same_bit.begin() + length);
Review Comment:
We can use `resize` here directly?
--
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] zhannngchen commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
zhannngchen commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1057413424
##########
be/src/vec/olap/vgeneric_iterators.h:
##########
@@ -120,6 +120,21 @@ class VMergeIteratorContext {
void set_skip(bool skip) const { _skip = skip; }
+ bool is_same() const { return _same; }
+
+ void set_same(bool same) const { _same = same; }
+
+ std::vector<bool> get_pre_ctx_same() const { return _pre_ctx_same_bit; }
Review Comment:
return a const reference to avoid copy of the whole vector.
--
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] zhannngchen commented on pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
zhannngchen commented on PR #14722:
URL: https://github.com/apache/doris/pull/14722#issuecomment-1352629948
Can't we just compare 2 rows to check if they are same? Just like what the rowwise interface did.
--
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 #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14722:
URL: https://github.com/apache/doris/pull/14722#issuecomment-1354163830
clang-tidy review says "All clean, LGTM! :+1:"
--
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] nextdreamblue commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
nextdreamblue commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1057516634
##########
be/src/vec/core/block.cpp:
##########
@@ -339,6 +342,10 @@ void Block::set_num_rows(size_t length) {
elem.column = elem.column->cut(0, length);
}
}
+ if (length < row_same_bit.size()) {
+ std::vector<bool> tmp_row_same_bit(row_same_bit.begin(), row_same_bit.begin() + length);
Review Comment:
it's better, i fix it
--
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] freemandealer commented on pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
freemandealer commented on PR #14722:
URL: https://github.com/apache/doris/pull/14722#issuecomment-1366538727
What if we let MergeIterator calculate Agg and provide CollectIterator with aggragated result?
--
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] nextdreamblue commented on pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
nextdreamblue commented on PR #14722:
URL: https://github.com/apache/doris/pull/14722#issuecomment-1352895192
> Can't we just compare 2 rows in `_agg_key_next_block` to check if they are same? Just like what the rowwise implementation did.
like in
`_agg_key_next_row` ,
```
if (!equal_row(_key_cids, *row_cursor, *_next_key)) {
break;
}
```
总感觉这样整个查询效率会低好多，多一次重复比较。我试试用roaringbitmap代替vector吧
--
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] freemandealer commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
freemandealer commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1057121438
##########
be/test/olap/segcompaction_test.cpp:
##########
@@ -419,6 +419,7 @@ TEST_F(SegCompactionTest, SegCompactionInterleaveWithBig_ooooOOoOooooooooO) {
}
s = rowset_writer->flush();
EXPECT_EQ(Status::OK(), s);
+ sleep(1);
Review Comment:
why we need sleep here?
--
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] nextdreamblue commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
nextdreamblue commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1057515742
##########
be/test/olap/segcompaction_test.cpp:
##########
@@ -419,6 +419,7 @@ TEST_F(SegCompactionTest, SegCompactionInterleaveWithBig_ooooOOoOooooooooO) {
}
s = rowset_writer->flush();
EXPECT_EQ(Status::OK(), s);
+ sleep(1);
Review Comment:
> why we need sleep here?
这个当时有一定几率还没来得及触发两次segcompaction，任务就执行完了，然后就加了一个这个sleep，我再试试
--
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] zhannngchen commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
zhannngchen commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1057405886
##########
be/src/vec/core/block.cpp:
##########
@@ -353,6 +360,10 @@ void Block::skip_num_rows(int64_t& length) {
elem.column = elem.column->cut(length, origin_rows - length);
}
}
+ if (length < row_same_bit.size()) {
+ std::vector<bool> tmp_row_same_bit(row_same_bit.begin() + length, row_same_bit.end());
Review Comment:
ditto
--
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] nextdreamblue commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
nextdreamblue commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1049592521
##########
be/src/vec/core/block.h:
##########
@@ -63,6 +63,7 @@ class Block {
Container data;
IndexByName index_by_name;
+ std::vector<uint64_t> row_same_bit;
Review Comment:
std::vector<bool>，感觉用这个特殊的类型就可以实现位图效果了，我改造下
https://en.cppreference.com/w/cpp/container/vector_bool
--
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 #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14722:
URL: https://github.com/apache/doris/pull/14722#issuecomment-1365844821
clang-tidy review says "All clean, LGTM! :+1:"
--
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 #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14722:
URL: https://github.com/apache/doris/pull/14722#issuecomment-1365885240
clang-tidy review says "All clean, LGTM! :+1:"
--
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] nextdreamblue commented on pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
nextdreamblue commented on PR #14722:
URL: https://github.com/apache/doris/pull/14722#issuecomment-1367155194
> What if we let MergeIterator calculate Agg and provide CollectIterator with aggragated result?
feel it more complicated than this pr
--
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] liaoxin01 commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
liaoxin01 commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1039311762
##########
be/src/vec/olap/vgeneric_iterators.cpp:
##########
@@ -123,6 +123,7 @@ bool VMergeIteratorContext::compare(const VMergeIteratorContext& rhs) const {
if (_is_unique) {
Review Comment:
Can we use `_is_unique` to skip the same key in the agg model?
--
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 #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14722:
URL: https://github.com/apache/doris/pull/14722#issuecomment-1365839536
clang-tidy review says "All clean, LGTM! :+1:"
--
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] freemandealer commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
freemandealer commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1058238457
##########
be/test/olap/segcompaction_test.cpp:
##########
@@ -419,6 +419,7 @@ TEST_F(SegCompactionTest, SegCompactionInterleaveWithBig_ooooOOoOooooooooO) {
}
s = rowset_writer->flush();
EXPECT_EQ(Status::OK(), s);
+ sleep(1);
Review Comment:
任务执行过快确实会有这个问题，导致目录跟预期不一样。这个我统一加一些延迟吧。谢谢！
--
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] zhannngchen commented on a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
zhannngchen commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1049269315
##########
be/src/vec/core/block.h:
##########
@@ -63,6 +63,7 @@ class Block {
Container data;
IndexByName index_by_name;
+ std::vector<uint64_t> row_same_bit;
Review Comment:
Using a vector of uint64_t is kind of waste of space, can we use a roaringbitmap instead?
--
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 a diff in pull request #14722: [fix](vec) VMergeIterator add key same label for agg table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on code in PR #14722:
URL: https://github.com/apache/doris/pull/14722#discussion_r1050322382
##########
be/src/vec/olap/vgeneric_iterators.h:
##########
@@ -120,6 +120,21 @@ class VMergeIteratorContext {
void set_skip(bool skip) const { _skip = skip; }
+ bool is_same() const { return _same; }
+
+ void set_same(bool same) const { _same = same; }
+
+ std::vector<bool> get_pre_ctx_same() const { return _pre_ctx_same_bit; }
+
+ void set_pre_ctx_same(VMergeIteratorContext* ctx) const {
+ int64_t index = ctx->get_cur_batch() - 1;
+ DCHECK(index >= 0);
+ DCHECK_LT(index, _pre_ctx_same_bit.size());
+ _pre_ctx_same_bit[index] = ctx->is_same();
+ }
+
+ size_t get_cur_batch() { return _cur_batch_num; }
Review Comment:
warning: method 'get_cur_batch' can be made const [readability-make-member-function-const]
```suggestion
size_t get_cur_batch() const { return _cur_batch_num; }
```
##########
be/src/vec/core/block.cpp:
##########
@@ -198,6 +198,7 @@ void Block::erase_impl(size_t position) {
++it;
}
}
+ if (position < row_same_bit.size()) row_same_bit.erase(row_same_bit.begin() + position);
Review Comment:
warning: statement should be inside braces [readability-braces-around-statements]
```suggestion
if (position < row_same_bit.size()) { row_same_bit.erase(row_same_bit.begin() + position);
}
```
##########
be/src/vec/olap/block_reader.cpp:
##########
@@ -407,6 +407,15 @@ void BlockReader::_update_agg_value(MutableColumns& columns, int begin, int end,
}
}
+bool BlockReader::_get_next_row_same() {
+ if (_next_row.is_same)
Review Comment:
warning: statement should be inside braces [readability-braces-around-statements]
```suggestion
if (_next_row.is_same) {
```
be/src/vec/olap/block_reader.cpp:412:
```diff
- else {
+ } 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