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

Remove DescribeTableStmt in parser in favour of existing functionality from sqlparser-rs #8703

Merged
merged 1 commit into from
Jan 1, 2024

Conversation

Jefffrey
Copy link
Contributor

@Jefffrey Jefffrey commented Jan 1, 2024

Which issue does this PR close?

Part of #4808

Rationale for this change

Start incrementally removing Datafusion parser extensions in favour of migrating them to sqlparser-rs. In this case, sqlparser-rs already supports explain table behaviour:

https://github.com/sqlparser-rs/sqlparser-rs/blob/a75778c8c7ef48e0ceff96637146b57912d2e10b/src/ast/mod.rs#L2091-L2101

So just remove the custom Datafusion code here.

What changes are included in this PR?

Remove Datafusion parser extension code for describe table_name SQL statement as parsing of this is already supported by sqlparser-rs

Are these changes tested?

Existing sqllogictest for describe ensures no breaking changes:

https://github.com/apache/arrow-datafusion/blob/bf3bd9259aa0e93ccc2c79a606207add30d004a4/datafusion/sqllogictest/test_files/describe.slt#L29-L34

Are there any user-facing changes?

@github-actions github-actions bot added sql SQL Planner core Core DataFusion crate labels Jan 1, 2024
@Jefffrey Jefffrey changed the title Remove DescribeTableStmt in parser in favour of existing functionalit… Remove DescribeTableStmt in parser in favour of existing functionality from sqlparser-rs Jan 1, 2024
@Jefffrey Jefffrey mentioned this pull request Jan 1, 2024
Copy link
Member

@jonahgao jonahgao left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@alamb
Copy link
Contributor

alamb commented Jan 1, 2024

Thank you for the research about describe.slt 👍

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @Jefffrey and @jonahgao for the review

@alamb alamb merged commit 77c2180 into apache:main Jan 1, 2024
23 checks passed
@Jefffrey Jefffrey deleted the remove_parser_describe_statement branch January 1, 2024 20:47
roeap pushed a commit to delta-io/delta-rs that referenced this pull request Jan 30, 2024
# Description
This PR upgrades `delta-rs` to using DataFusion 35.0, which was recently
released. In order to do this, I had to fix a few breaking changes, and
also upgrade Arrow to 50 and `sqlparser` to 0.41.

# Related Issue(s)
N/A

# Documentation
See here for the list of PRs which required code change:
- apache/datafusion#8703
-
https://github.com/apache/arrow-datafusion/blob/ec6abece2dcfa68007b87c69eefa6b0d7333f628/dev/changelog/35.0.0.md?plain=1#L227

---------

Co-authored-by: Ming Ying <[email protected]>
rtyler pushed a commit to dispanser/delta-rs that referenced this pull request Jan 31, 2024
# Description
This PR upgrades `delta-rs` to using DataFusion 35.0, which was recently
released. In order to do this, I had to fix a few breaking changes, and
also upgrade Arrow to 50 and `sqlparser` to 0.41.

# Related Issue(s)
N/A

# Documentation
See here for the list of PRs which required code change:
- apache/datafusion#8703
-
https://github.com/apache/arrow-datafusion/blob/ec6abece2dcfa68007b87c69eefa6b0d7333f628/dev/changelog/35.0.0.md?plain=1#L227

---------

Co-authored-by: Ming Ying <[email protected]>
RobinLin666 pushed a commit to RobinLin666/delta-rs that referenced this pull request Feb 2, 2024
# Description
This PR upgrades `delta-rs` to using DataFusion 35.0, which was recently
released. In order to do this, I had to fix a few breaking changes, and
also upgrade Arrow to 50 and `sqlparser` to 0.41.

# Related Issue(s)
N/A

# Documentation
See here for the list of PRs which required code change:
- apache/datafusion#8703
-
https://github.com/apache/arrow-datafusion/blob/ec6abece2dcfa68007b87c69eefa6b0d7333f628/dev/changelog/35.0.0.md?plain=1#L227

---------

Co-authored-by: Ming Ying <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate sql SQL Planner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants