-
Notifications
You must be signed in to change notification settings - Fork 265
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: nested list access #2966
feat: nested list access #2966
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2966 +/- ##
==========================================
- Coverage 78.97% 78.93% -0.05%
==========================================
Files 234 234
Lines 72916 72990 +74
Branches 72916 72990 +74
==========================================
+ Hits 57589 57614 +25
- Misses 12354 12383 +29
- Partials 2973 2993 +20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
let key = Box::new(self.parse_sql_expr(key)?); | ||
GetFieldAccess::ListIndex { key } |
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.
Do we want to verify this is an integer at some point?
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 in theory it could be an expression that resolves to an integer. I'm kind of trusting that it will be verified somewhere down the chain.
|
||
Ok(expr) | ||
} | ||
SQLExpr::Subscript { expr, subscript } => { |
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.
Somewhat annoying that both Subscript
and MapAccess
exist 😕
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.
Yeah and which path is triggered depends on the dialect. Implemented both for now.
Adds support for accessing list items and struct fields with subscripting:
x[0]
andx['field_name']
. These can be chained.Unfortunately there is an issue with our SQL parser related to using
.
I wasn't able to figure out. Follow up issue here: #2967