Skip to content

Add ShowObjects statement #2

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

Merged
merged 5 commits into from
Feb 3, 2025

Conversation

DanCodedThis
Copy link

No description provided.

@DanCodedThis DanCodedThis changed the title Added ShowObjects statement Add ShowObjects statement Jan 31, 2025
@DanCodedThis
Copy link
Author

DanCodedThis commented Jan 31, 2025

We can merge for now.

  • It is possible to get SHOW TERSE OBJECTS WHERE ... ..., instead of SHOW TERSE OBJECTS LIKE '%test%' ... and we will keep going.
  • For now we only support the happy path. I could add failing here, by rewriting fn parse_show_stmt_options as a separate function and removing wrong cases for us.
  • If so, should we fail in parser/AST (here) or in logical plan (control-plane)?

Snowflake spec https://docs.snowflake.com/en/sql-reference/sql/show-objects

eadgbear
eadgbear previously approved these changes Feb 3, 2025
osipovartem
osipovartem previously approved these changes Feb 3, 2025
Copy link

@Vedin Vedin left a comment

Choose a reason for hiding this comment

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

In general, I'm fine with this approach. However, I think we can move parse_show_objects to parser/mod.rs itself. Even if it's really limited to Snowflake for now I think it can be found in other databases in the future. At least, Materialize supports SHOW OBJECTS statement.
We can try upstreaming the current solution as is and we'll see suggestions on external review.

@DanCodedThis DanCodedThis dismissed stale reviews from osipovartem and eadgbear via 40321ce February 3, 2025 12:52
@DanCodedThis DanCodedThis requested a review from Vedin February 3, 2025 12:53
@@ -13670,8 +13670,8 @@ impl<'a> Parser<'a> {
}
false
}

fn parse_show_stmt_options(&mut self) -> Result<ShowStatementOptions, ParserError> {

Copy link

Choose a reason for hiding this comment

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

nit: remove redundant spaces here

Copy link
Author

Choose a reason for hiding this comment

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

They had the space before me. All their functions actually have spaces before them for some reason :)

@DanCodedThis DanCodedThis merged commit 177c37a into icehut Feb 3, 2025
Vedin pushed a commit that referenced this pull request Feb 4, 2025
* Added ShowObjects statement

* Parsing, next tests

* Small comment

* Tests done according to other SHOW statements

* Removed unnecessary comments
Vedin pushed a commit that referenced this pull request Feb 5, 2025
* Added ShowObjects statement

* Parsing, next tests

* Small comment

* Tests done according to other SHOW statements

* Removed unnecessary comments
Vedin added a commit that referenced this pull request Feb 5, 2025
* Add ShowObjects statement (#2)

* Added ShowObjects statement

* Parsing, next tests

* Small comment

* Tests done according to other SHOW statements

* Removed unnecessary comments

* Fix test in other that object cases. Cargo fmt.

---------

Co-authored-by: DanCodedThis <94703934+DanCodedThis@users.noreply.github.com>
Vedin pushed a commit that referenced this pull request Apr 23, 2025
* Added ShowObjects statement

* Parsing, next tests

* Small comment

* Tests done according to other SHOW statements

* Removed unnecessary comments
Vedin pushed a commit that referenced this pull request Apr 23, 2025
* Added ShowObjects statement

* Parsing, next tests

* Small comment

* Tests done according to other SHOW statements

* Removed unnecessary comments
Vedin pushed a commit that referenced this pull request Apr 28, 2025
* Added ShowObjects statement

* Parsing, next tests

* Small comment

* Tests done according to other SHOW statements

* Removed unnecessary comments
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.

4 participants