feat: support explain options #1426
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
iffyio
reviewed
Sep 14, 2024
|
Update my code. Please review again @iffyio Thank you. |
iffyio
reviewed
Sep 15, 2024
iffyio
reviewed
Sep 17, 2024
src/parser/mod.rs
Outdated
| @@ -1274,6 +1274,29 @@ impl<'a> Parser<'a> { | |||
| } | |||
| } | |||
|
|
|||
| pub fn parse_utility_option_list(&mut self) -> Result<Vec<UtilityOption>, ParserError> { | |||
There was a problem hiding this comment.
Suggested change
| pub fn parse_utility_option_list(&mut self) -> Result<Vec<UtilityOption>, ParserError> { | |
| pub fn parse_utility_options(&mut self) -> Result<Vec<UtilityOption>, ParserError> { |
src/parser/mod.rs
Outdated
| // Note: DuckDB is compatible with PostgreSQL syntax for this statement, | ||
| // although not all features may be implemented. | ||
| if describe_alias == DescribeAlias::Explain | ||
| && dialect_of!(self is PostgreSqlDialect | DuckDbDialect | GenericDialect) |
There was a problem hiding this comment.
Suggested change
| && dialect_of!(self is PostgreSqlDialect | DuckDbDialect | GenericDialect) | |
| && self.dialect.supports_explain_with_utility_options() |
tests/sqlparser_common.rs
Outdated
Comment on lines
4265
to
4282
| run_explain_analyze_with_specific_dialect( | ||
| |_d| true, | ||
| query, | ||
| expected_verbose, | ||
| expected_analyze, | ||
| expected_format, | ||
| exepcted_options, | ||
| ) | ||
| } | ||
|
|
||
| fn run_explain_analyze_with_specific_dialect<T: Fn(&dyn Dialect) -> bool>( | ||
| dialect_predicate: T, | ||
| query: &str, | ||
| expected_verbose: bool, | ||
| expected_analyze: bool, | ||
| expected_format: Option<AnalyzeFormat>, | ||
| exepcted_options: Option<Vec<UtilityOption>>, | ||
| ) { |
There was a problem hiding this comment.
I think instead of having two functions, we can reuse the existing run_explain_analyze and have that one take in a dialect: TestedDialects as the argument (instead of a lambda), then the existing tests can pass in all_dialects() while the new tests pass in all_dialects_where(|d| d.supports_option_list())
src/test_utils.rs
Outdated
Comment on lines
151
to
155
| /// Ensures that `sql` parses as a single [Statement] and that the re-serialization of the parsed | ||
| /// result matches the given `canonical` SQL string. | ||
| pub fn verified_stmt_with_canonical(&self, sql: &str, canonical: &str) -> Statement { | ||
| self.one_statement_parses_to(sql, canonical) | ||
| } |
There was a problem hiding this comment.
Suggested change
| /// Ensures that `sql` parses as a single [Statement] and that the re-serialization of the parsed | |
| /// result matches the given `canonical` SQL string. | |
| pub fn verified_stmt_with_canonical(&self, sql: &str, canonical: &str) -> Statement { | |
| self.one_statement_parses_to(sql, canonical) | |
| } |
this looks unused and we can remove?
iffyio
approved these changes
Sep 18, 2024
alamb
approved these changes
Sep 19, 2024
| @@ -7116,6 +7123,47 @@ where | |||
| } | |||
| } | |||
|
|
|||
| /// Represents a single PostgreSQL utility option. | |||
| /// | |||
| /// A utility option is a key-value pair where the key is an identifier (IDENT) and the value | |||
|
Looks like there are some small CI issues to address |
Pull Request Test Coverage Report for Build 10942458377Details
💛 - Coveralls |
…dd Explain describe_alias check
kysshsy
force-pushed
the
support_explain_options
branch
from
d935ebe to
b5bb4f2
Compare
Fixes the CI issues @alamb . |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
support
EXPLAIN [ ( option [, ...] ) ] statementclose #1425