snowflake column identifier #520
Conversation
Pull Request Test Coverage Report for Build 2487464140
💛 - Coveralls |
|
There appear to be a few CI failures on this PR |
Signed-off-by: Pawel Leszczynski <[email protected]>
pawel-big-lebowski
force-pushed
the
snowflake-variable-support
branch
from
396ea62 to
17ac30c
Compare
There was a problem hiding this comment.
Thank you for the contribution @pawel-big-lebowski I will try and find time to review this PR tomorrow.
There was a problem hiding this comment.
Thank you @pawel-big-lebowski -- I spent some time researching this feature.
First, I think it might help this PR if you split it into 2 pieces so we can discuss each feature ($identifiers and get path) separately.
Also, 🤔 it seems as though snowflake also supports identifiers like column$FILENAME -- is this something you want to explore?
https://docs.snowflake.com/en/user-guide/querying-metadata.html#metadata-columns
| @@ -217,6 +217,18 @@ pub enum Expr { | |||
| Identifier(Ident), | |||
| /// Multi-part identifier, e.g. `table_alias.column` or `schema.table.col` | |||
| CompoundIdentifier(Vec<Ident>), | |||
| /// Multi-part identifier with a column number, e.g. [alias.]$file_col_num[.element] (snowflake) | |||
| CompoundIdentifierWithColumnNumber { | |||
There was a problem hiding this comment.
In terms of supporting things like a.$1 -- what would you think about modeling this as a a two part CompoundIdent identifier ["a", "$1"] rather than a new variant?
I think the parser would still have to be changed, but I think that approach would keep the AST and the consumption of the AST smaller with fewer dialect specific structures
| @@ -99,6 +99,12 @@ fn test_single_table_in_parenthesis() { | |||
| ); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn test_snowflake_variables() { | |||
There was a problem hiding this comment.
I think it would be good to have a test that the parsed structure is actually what is expected -- for example https://github.com/sqlparser-rs/sqlparser-rs/blob/c884fbc/tests/sqlparser_common.rs#L138-L168
|
Marking this as a draft to show it is waiting some changes so I can easily filter out which PRs need review. Please mark it as "ready for review" when it is next ready. |
|
@alamb @pawel-big-lebowski is closing this for now because it's almost a year without an update. @pawel-big-lebowski, ping me if you will work on this code specifically, if so, I open the PR again :) |
|
Thanks for cleaning this up @AugustoFKL |
Pull Request Test Coverage Report for Build 2487464140Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Signed-off-by: Pawel Leszczynski [email protected]
resolves #519
According to documentation, Snowflake allows:
[<alias>.]$<file_col_num>[.<element>]select t.$1,t.$2,t.$3 from @~/datafile.csv.gz t;select v:attr[0].name from vartab;Although the feature seems to be snowflake specific, it cannot be implemented within snowflake dialect. It's because tokens starting with
$character are tokenized intoToken::Placeholder. That's why parser's change is required.Initial OpenLineage project issue: OpenLineage/OpenLineage#814.