feat(parser): support to parse param in parser #8113
Conversation
Codecov Report
@@ Coverage Diff @@
## main #8113 +/- ##
==========================================
- Coverage 71.78% 71.78% -0.01%
==========================================
Files 1130 1130
Lines 181489 181508 +19
==========================================
+ Hits 130277 130290 +13
- Misses 51212 51218 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
ZENOTME
requested review from
BugenZhao and
st1page
and removed request for
BugenZhao
| '$' => { | ||
| if let Some(parameter) = self.tokenize_parameter(chars) { | ||
| Ok(Some(parameter)) | ||
| } else { | ||
| Ok(Some(Token::Char('$'))) | ||
| } | ||
| } |
There was a problem hiding this comment.
FYI in PostgreSQL $ can also start a dollar-quoted string constant (upstream sqlparser-rs/sqlparser-rs#772).
But extending the support later would not break common use cases.
src/sqlparser/src/parser.rs
Outdated
| let data_type = if let Token::DoubleColon = self.peek_token() { | ||
| self.next_token(); | ||
| Some(self.parse_data_type()?) | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
Why do we include the cast as part of parameter? Is it possible to be CAST($1 AS int) rather than $1::int?
One idea is to just parse the outer part as a normal cast, which includes both CAST(AS) and ::. Then during binder type inference, just treat $1 as unknown (same as null and '12', or typed like null::date, '12'::interval, interval '12').
^ This can also be a refactor after we have build the whole code path of this paramater-with-type first.
There was a problem hiding this comment.
I think this is the right way.🤣 Let's do it now.
ZENOTME
force-pushed
the
zj/support_param_in_parse
branch
from
7c19b62
to
40f803c
Compare
|
I updated this PR because I was playing with buildkite & merge queue. Sorry if that bothered. 🤪🙏 |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
To complete #8112, we need to support to parse param syntax like ('$1','$1::int') in parser first.
Checklist For Contributors
./risedev check(or alias,./risedev c)Checklist For Reviewers
Documentation
Click here for Documentation
Types of user-facing changes
Please keep the types that apply to your changes, and remove the others.
Release note