Skip to content
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

Add support for Snowflake column aliases that use SQL keywords #1632

Merged
merged 3 commits into from
Jan 16, 2025

Conversation

yoavcloud
Copy link
Contributor

Attempts at solving issue #1607

Snowflake is not very strict with using SQL keywords as select list item aliases. Added support for the dialect to control if a word is parsed as an alias or not. Parsing of aliases is quite centralized now, and this PR breaks that up a bit, so feedback is welcomed if this is a good approach or alternative solutions are preferred.

@yoavcloud yoavcloud force-pushed the reserved_kws_col_alias branch from e393b20 to 98a5627 Compare January 7, 2025 12:43
Copy link
Contributor

@iffyio iffyio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @yoavcloud! The approach sounds reasonable to me, left a comment

@@ -8738,6 +8738,34 @@ impl<'a> Parser<'a> {
Ok(IdentWithAlias { ident, alias })
}

// Optionally parses an alias for a select list item
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Optionally parses an alias for a select list item
/// Optionally parses an alias for a select list item

Comment on lines 8749 to 8751
if self
.dialect
.is_select_item_alias(after_as, &w.keyword, self) =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like the code is simlar to parse_optional_alias and they only differ in this clause? Thinking it would be nice to reuse the logic if so, maybe we have the shared function taking reserved_kwds as an optional argument and if None is passed, we use dialect.is_select_item_alias()

@yoavcloud yoavcloud force-pushed the reserved_kws_col_alias branch from 98a5627 to dd5be2f Compare January 15, 2025 13:13
@yoavcloud
Copy link
Contributor Author

@iffyio in this iteration I re-used the logic but also aligned parsing of table aliases to column aliases, enabling customization by dialects. LMKWYT

@yoavcloud yoavcloud requested a review from iffyio January 15, 2025 13:15
Copy link
Contributor

@iffyio iffyio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @yoavcloud!
cc @alamb

@iffyio iffyio merged commit b4b5576 into apache:main Jan 16, 2025
9 checks passed
hansott added a commit to hansott/datafusion-sqlparser-rs that referenced this pull request Jan 23, 2025
…o escape-literals

* 'main' of github.com:hansott/datafusion-sqlparser-rs:
  National strings: check if dialect supports backslash escape (apache#1672)
  Add support for Create Iceberg Table statement for Snowflake parser (apache#1664)
  Add support for Snowflake account privileges (apache#1666)
  Update rat_exclude_file.txt (apache#1670)
  Update verson to 0.54.0 and update changelog (apache#1668)
  Add support for Snowflake AT/BEFORE (apache#1667)
  Add support for qualified column names in JOIN ... USING (apache#1663)
  Add support for `IS [NOT] [form] NORMALIZED` (apache#1655)
  fix parsing of `INSERT INTO ... SELECT ... RETURNING ` (apache#1661)
  Add support for Snowflake column aliases that use SQL keywords (apache#1632)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants