-
Notifications
You must be signed in to change notification settings - Fork 590
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
feat(iceberg): Optimization iceberg source count(*) query #18067
Conversation
0d32982
to
2552e1f
Compare
2552e1f
to
ae95e1e
Compare
enum SourceType { | ||
UNSPECIFIED = 0; | ||
SCAN = 1; | ||
COUNT_STAR = 2; | ||
} | ||
uint32 source_id = 1; | ||
repeated plan_common.ColumnCatalog columns = 2; | ||
map<string, string> with_properties = 3; | ||
repeated bytes split = 4; | ||
catalog.StreamSourceInfo info = 5; | ||
map<string, secret.SecretRef> secret_refs = 6; | ||
SourceType source_type = 7; |
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.
I think this new field is too special to be a part of the SourceNode
proto because it only meaningful for iceberg. Considering we will try to support predicate pushdown for iceberg scan, I think we can separate iceberg scan from source node just like file scan.
NodeBody::FileScan(FileScanNode { |
@@ -196,49 +199,81 @@ pub enum IcebergTimeTravelInfo { | |||
} | |||
|
|||
impl IcebergSplitEnumerator { | |||
pub async fn list_splits_batch( | |||
pub async fn list_splits_batch_count_star( |
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.
If it is a count star query, we don't need to return multiple splits, i.e. we don't need to run it in a distributed way.
use crate::error::BatchError; | ||
use crate::executor::{DataChunk, Executor}; | ||
|
||
pub struct IcebergCountStarExecutor { |
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.
It seems we don't even need a special iceberg count start executor, ValuesExecutor
can be used instead.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
#17958
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.