-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feature] (vec) instead of converting line to src tuple for stream load in vectorized. #9314
Conversation
9548478
to
efbf588
Compare
RETURN_IF_ERROR(ctx->prepare(_state, *_row_desc.get(), _mem_tracker)); | ||
RETURN_IF_ERROR(ctx->open(_state)); | ||
_dest_vexpr_ctx.emplace_back(ctx); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else {}
be/src/exec/broker_scanner.cpp
Outdated
if (range.__isset.num_of_columns_from_file) { | ||
fill_slots_of_columns_from_path(range.num_of_columns_from_file, columns_from_path); | ||
fill_slots_of_columns_from_path(range.num_of_columns_from_file, range.columns_from_path); | ||
} | ||
|
||
_success = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in function line_split_to_values
have set the _success
,not need set here.
|
||
RETURN_IF_ERROR(VExprContext::filter_block(_vconjunct_ctx_ptr, block.get(), | ||
_tuple_desc->slots().size())); | ||
_mutable_block->add_rows(block.get(), begin, row_add); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here must have a copy here. should opt to skip the copy operation. if _mutable_block.size() + block.size() < batch_size
to do merge. else directly return the block, do not copy here
} | ||
auto output_block = _mutable_block->to_block(); | ||
std::shared_ptr<vectorized::Block> block_ptr(new vectorized::Block()); | ||
block_ptr->swap(std::move(output_block)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
block_ptr->swap(_mutable_block->to_block());
if (_scan_finished.load()) { | ||
return Status::OK(); | ||
} | ||
std::shared_ptr<vectorized::Block> block(new vectorized::Block()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why here is shared_ptr ? not the unique_ptr?
be/src/vec/exec/vbroker_scanner.cpp
Outdated
SCOPED_TIMER(_read_timer); | ||
|
||
const int batch_size = _state->batch_size(); | ||
std::shared_ptr<vectorized::Block> tmp_block(std::make_shared<Block>()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why define tmp_block here ? should in _fill_dest_block
.
cacbcbe
to
7190796
Compare
_queue_writer_cond.notify_all(); | ||
|
||
// All scanner has been finished, and all cached batch has been read | ||
if (scanner_block == nullptr || scanner_block.get() == nullptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scanner_block == nullptr || scanner_block.get() == nullptr
only need one
// notify one scanner | ||
_queue_writer_cond.notify_one(); | ||
|
||
if (_mutable_block.get() == nullptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unlikely, should move the line to up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR approved by at least one committer and no changes requested. |
…ad in vectorized. (apache#9314) Co-authored-by: xiepengcheng01 <xiepengcheng01@xafj-palo-rpm64.xafj.baidu.com>
…ad in vectorized. (apache#9314) Co-authored-by: xiepengcheng01 <xiepengcheng01@xafj-palo-rpm64.xafj.baidu.com>
…ad in vectorized. (apache#9314) Co-authored-by: xiepengcheng01 <xiepengcheng01@xafj-palo-rpm64.xafj.baidu.com>
…ad in vectorized. (apache#9314) Co-authored-by: xiepengcheng01 <xiepengcheng01@xafj-palo-rpm64.xafj.baidu.com>
Proposed changes
Support stream load in vectorized instead of converting line to src tuple for vbroker scanner.
Issue Number: close #xxx
Problem Summary:
Describe the overview of changes.
Checklist(Required)
Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...