-
Notifications
You must be signed in to change notification settings - Fork 605
fix: only require DESCRIBE TABLE
for Snowflake and ClickHouse dialect
#1386
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
Conversation
Some(Keyword::FORMATTED) => hive_format = Some(HiveDescribeFormat::Formatted), | ||
_ => {} | ||
} | ||
let hive_format = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was a drive by cleanup to avoid using mut
_ => None, | ||
}; | ||
|
||
let has_table_keyword = if self.dialect.describe_requires_table_keyword() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the actual fix, and the rest of the PR is test updated / documentation
bf56eae
to
6ef20b0
Compare
Pull Request Test Coverage Report for Build 10411444248Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM.
@alamb Thanks for your fix, as well as thanks for @samuelcolvin finding this regression. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks for the reviews @samuelcolvin @iffyio and @git-hulk -- let's ship this release! 🚀 |
Fixes #1385
Rationale
@git-hulk added support for
DESCRIBE TABLE <..>
syntax for ClickHouse and Snowflake in #1351Unfrtunately, this seems to have caused a regression in
describe table
(aka describing a table namedtable
which apparently we do in the DataFusion tests) which we did not have test coverage forChanges
DESCRIBE TABLE
keyword for ClickHouse and Snowflake dialects