-
Notifications
You must be signed in to change notification settings - Fork 594
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: support parsing map type: map(k,v)
#17860
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
94f7632
to
bfbb254
Compare
bfbb254
to
61d2cbe
Compare
61d2cbe
to
aa33f2f
Compare
aa33f2f
to
d757a66
Compare
a563afa
to
519cd3d
Compare
map(k,v)
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! Nice work.
519cd3d
to
83e6410
Compare
Shall we rebase this PR? |
support cast improve check
83e6410
to
34e9829
Compare
let new_ctx = Context { | ||
arg_types: vec![ctx.arg_types[0].clone().as_map().clone().into_list()], | ||
return_type: ctx.return_type.as_map().clone().into_list(), | ||
variadic: ctx.variadic, | ||
}; |
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.
Though implemented ingeniously, does it mean that we need to construct (and allocate) the new context per row? 😕
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.
The real reason was that I didn't fully understand what list_cast
is doing, so I just chose to invoke it directly instead of imitating. 🤡
risingwave/src/expr/impl/src/scalar/cast.rs
Lines 200 to 205 in 34e9829
let cast = build_func( | |
PbType::Cast, | |
ctx.return_type.as_list().clone(), | |
vec![InputRefExpression::new(ctx.arg_types[0].as_list().clone(), 0).boxed()], | |
) | |
.unwrap(); |
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.
Well, list_cast
also clones datatypes and allocates vec every time..
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
part of #17743
map(k,v)
type.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.