Skip to content

Support Snowflake Update-From-Select - #1604

Merged
alamb merged 9 commits into
mainfrom
unknown repository
Dec 24, 2024
Merged

Support Snowflake Update-From-Select#1604
alamb merged 9 commits into
mainfrom
unknown repository

Conversation

@yuval-illumex

@yuval-illumexyuval-illumex commented Dec 16, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

The following query works in Snowflake:

UPDATE t1 FROM ( SELECT name, id FROM t1 GROUP BY id ) AS t2 SET name = t2.name WHERE t1.id = t2.id

Today the parser only supports update-set-from

iffyio
iffyio reviewed Dec 19, 2024
View reviewed changes
Comment threadsrc/parser/mod.rs Outdated
Comment threadsrc/parser/mod.rs Outdated
Comment threadsrc/parser/mod.rs
Comment threadtests/sqlparser_snowflake.rs Outdated
@yuval-illumex

yuval-illumex commented Dec 22, 2024

Copy link
Copy Markdown
ContributorAuthor

Thank you @iffyio for your feedback, I asked your opinion in one of the comments.

@yuval-illumex

yuval-illumex commented Dec 22, 2024•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@iffyio appreciate your feedback again 🙏

iffyio
iffyio reviewed Dec 23, 2024
View reviewed changes

@iffyioiffyio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Left a couple comments, otherwise this looks good to me overall!

Comment threadtests/sqlparser_common.rs Outdated
Comment threadsrc/ast/query.rs Outdated
Comment threadsrc/ast/query.rs Outdated
Comment threadsrc/keywords.rs
// Reserved for Snowflake table sample
Keyword::SAMPLE,
Keyword::TABLESAMPLE,
Keyword::FROM,

@iffyioiffyioDec 23, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this change related to the PR (wasn't clear to me if so)?

@yuval-illumexyuval-illumexDec 23, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes.
When removing this as a keyword the following error occurs:
Error during parsing: ParserError("Expected: ), found: t1 at Line: 9, Column: 13")

iffyio
iffyio approved these changes Dec 24, 2024
View reviewed changes

@iffyioiffyio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Thanks @yuval-illumex!
cc @alamb

Comment threadsrc/ast/query.rs Outdated
Comment threadsrc/ast/spans.rs Outdated
yuval-illumexand others added 2 commits December 24, 2024 13:04
@yuval-illumex@iffyio
Update src/ast/query.rs
390f83b
Comments
Co-authored-by: Ifeanyi Ubah <ify1992@yahoo.com>
@yuval-illumex@iffyio
Update src/ast/spans.rs
0833f06
Change parameter
Co-authored-by: Ifeanyi Ubah <ify1992@yahoo.com>
@alamb
alamb merged commit 024a878 into apache:mainDec 24, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iffyioiffyioiffyio approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yuval-illumex@iffyio@alamb