Skip to content

Snowflake dialect - #259

Merged
nickolay merged 1 commit into
apache:mainfrom
eyalleshem:snowflake_dialect
Aug 12, 2020
Merged

Snowflake dialect#259
nickolay merged 1 commit into
apache:mainfrom
eyalleshem:snowflake_dialect

Conversation

@eyalleshem

@eyalleshemeyalleshem commented Aug 10, 2020

Copy link
Copy Markdown
Contributor

No description provided.

@eyalleshem

eyalleshem commented Aug 10, 2020

Copy link
Copy Markdown
ContributorAuthor

Added a pull request for snowflake dialect .
As i didn't added any new functionally for the dialect yet , i don't sure what exactly expected to be on the test ...

@coveralls

coveralls commented Aug 10, 2020•
edited
Loading

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 204251590

  • 15 of 15(100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 91.944%
TotalsCoverage Status
Change from base Build 202618535:0.02%
Covered Lines:4554
Relevant Lines:4953
💛 - Coveralls

Dandandan
Dandandan reviewed Aug 10, 2020
View reviewed changes
Comment threadsrc/dialect/mod.rs Outdated
nickolay
nickolay suggested changes Aug 10, 2020
View reviewed changes

@nickolaynickolay 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.

Note to self: this is the result of splitting #244 into separate PRs at my request. We needed a new dialect to make the parsing quirk dealing with FROM ((table_name)) alias Snowflake-specific.

Please don't forget to add the new dialect to cli.rs and to all_dialects(): https://github.com/ballista-compute/sqlparser-rs/search?q=MsSqlDialect&unscoped_q=MsSqlDialect

Comment threadtests/sqlparser_snowflake.rs Outdated

fn snowflake_and_generic() -> TestedDialects {
TestedDialects {
// we don't have a separate SQLite dialect, so test only the generic dialect for now

@nickolaynickolayAug 10, 2020

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.

This comment doesn't belong here.

@eyalleshem
eyalleshemforce-pushed the snowflake_dialect branch 2 times, most recently from 9055ebf to f16b90aCompareAugust 11, 2020 15:01
nickolay
nickolay suggested changes Aug 11, 2020
View reviewed changes

@nickolaynickolay 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.

I don't believe you addressed my request to add the new dialect to all_dialects() as well?

@eyalleshem

eyalleshem commented Aug 11, 2020

Copy link
Copy Markdown
ContributorAuthor

Sorry , missed that.
Added now ..

nickolay
nickolay approved these changes Aug 12, 2020
View reviewed changes

@nickolaynickolay 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.

Super, thanks!

@nickolay
nickolay merged commit f500a42 into apache:mainAug 12, 2020
@eyalleshemeyalleshem mentioned this pull request Aug 13, 2020
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DandandanDandandanDandandan left review comments
+1 more reviewer
@nickolaynickolaynickolay approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@eyalleshem@coveralls@Dandandan@nickolay@eyalsatori