Add support for arbitrary map access expr #1179
Conversation
Pull Request Test Coverage Report for Build 8407761282Details
💛 - Coveralls |
Unlike array access, the current logic for accessing maps had custom handling and didn't support cases like negative numbers or function calls. This fixes the issue by allowing the key access to be parsed as an arbitrary expression. Also includes support on BigQuery where both dot and bracket notations are allowed in the same expression https://cloud.google.com/bigquery/docs/nested-repeated#query_nested_and_repeated_columns
There was a problem hiding this comment.
Thank you so much @iffyio -- this looks really nice. I had a small suggestion but it may not make sense.
Also, in general thank you for your work on this (and the other related PRs) and the reviews. Is there any chance you are interested in helping be a maintainer of this crate? As is probably obvious I do not have a huge amount of time and I think it is suffering because of that.
| // Access on BigQuery nested and repeated expressions can | ||
| // mix notations in the same expression. | ||
| // https://cloud.google.com/bigquery/docs/nested-repeated#query_nested_and_repeated_columns | ||
| Token::Period if dialect_of!(self is BigQueryDialect) => { |
There was a problem hiding this comment.
What do you think about adding GenericDialect here too?
There was a problem hiding this comment.
Yeah I suspect it would be valid to include Generic here without conflicting (from an entry point pov I remember with the current setup Generic doesn't fall into this function today because together with Postgres it parses the expression as an Array instead - but that could change in the future)
|
@alamb Thanks a lot for the reviews 🙏 Regarding maintainership, sure I'm happy to help out in this regard and can allocate more time to reviews and tracking issues etc. Do let me know how to effectively help out here |
Thank you @iffyio -- that is great news. Basically the work is reviewing other PRs and getting them ready for merge (which means doing what you already do well -- make sure they are tested, documented, and don't cause overly painful changes for downstream crates) There is more documentation on #940 Let me know if you want to learn more or have a video call to discuss the possibility |
|
That sounds good to me! Will look to help out further in that case 👍 |
BTW if you see a PR that is ready to go in your opinion please just mention (@alamb) me and I'll take a look. Otherwise I likely won't see it as I don't monitor all alerts from this repo |
Ah good to know, will do! |
Unlike array access, the current logic for
accessing maps had custom handling and didn't
support cases like negative numbers or function
calls.
This fixes the issue by allowing the key access to be parsed as an arbitrary expression.
Also includes support on BigQuery where both dot and
bracket notations are allowed in the same expression
https://cloud.google.com/bigquery/docs/nested-repeated#query_nested_and_repeated_columns