Skip to content

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

Merged
merged 2 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/dialect/clickhouse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,8 @@ impl Dialect for ClickHouseDialect {
fn supports_select_wildcard_except(&self) -> bool {
true
}

fn describe_requires_table_keyword(&self) -> bool {
true
}
}
11 changes: 11 additions & 0 deletions src/dialect/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,9 +485,20 @@ pub trait Dialect: Debug + Any {
}
}

/// Returns the precedence when the precedence is otherwise unknown
fn prec_unknown(&self) -> u8 {
0
}

/// Returns true if this dialect requires the `TABLE` keyword after `DESCRIBE`
///
/// Defaults to false.
///
/// If true, the following statement is valid: `DESCRIBE TABLE my_table`
/// If false, the following statements are valid: `DESCRIBE my_table` and `DESCRIBE table`
fn describe_requires_table_keyword(&self) -> bool {
false
}
}

/// This represents the operators for which precedence must be defined
Expand Down
4 changes: 4 additions & 0 deletions src/dialect/snowflake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ impl Dialect for SnowflakeDialect {
_ => None,
}
}

fn describe_requires_table_keyword(&self) -> bool {
true
}
}

/// Parse snowflake create table statement.
Expand Down
21 changes: 13 additions & 8 deletions src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8185,15 +8185,20 @@ impl<'a> Parser<'a> {
format,
}),
_ => {
let mut hive_format = None;
match self.parse_one_of_keywords(&[Keyword::EXTENDED, Keyword::FORMATTED]) {
Some(Keyword::EXTENDED) => hive_format = Some(HiveDescribeFormat::Extended),
Some(Keyword::FORMATTED) => hive_format = Some(HiveDescribeFormat::Formatted),
_ => {}
}
let hive_format =
Copy link
Contributor Author

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

match self.parse_one_of_keywords(&[Keyword::EXTENDED, Keyword::FORMATTED]) {
Some(Keyword::EXTENDED) => Some(HiveDescribeFormat::Extended),
Some(Keyword::FORMATTED) => Some(HiveDescribeFormat::Formatted),
_ => None,
};

let has_table_keyword = if self.dialect.describe_requires_table_keyword() {
Copy link
Contributor Author

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

// only allow to use TABLE keyword for DESC|DESCRIBE statement
self.parse_keyword(Keyword::TABLE)
} else {
false
};

// only allow to use TABLE keyword for DESC|DESCRIBE statement
let has_table_keyword = self.parse_keyword(Keyword::TABLE);
let table_name = self.parse_object_name(false)?;
Ok(Statement::ExplainTable {
describe_alias,
Expand Down
30 changes: 30 additions & 0 deletions tests/sqlparser_clickhouse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1376,6 +1376,36 @@ fn parse_select_table_function_settings() {
}
}

#[test]
fn explain_describe() {
clickhouse().verified_stmt("DESCRIBE test.table");
clickhouse().verified_stmt("DESCRIBE TABLE test.table");
}

#[test]
fn explain_desc() {
clickhouse().verified_stmt("DESC test.table");
clickhouse().verified_stmt("DESC TABLE test.table");
}

#[test]
fn parse_explain_table() {
match clickhouse().verified_stmt("EXPLAIN TABLE test_identifier") {
Statement::ExplainTable {
describe_alias,
hive_format,
has_table_keyword,
table_name,
} => {
pretty_assertions::assert_eq!(describe_alias, DescribeAlias::Explain);
pretty_assertions::assert_eq!(hive_format, None);
pretty_assertions::assert_eq!(has_table_keyword, true);
pretty_assertions::assert_eq!("test_identifier", table_name.to_string());
}
_ => panic!("Unexpected Statement, must be ExplainTable"),
}
}

fn clickhouse() -> TestedDialects {
TestedDialects {
dialects: vec![Box::new(ClickHouseDialect {})],
Expand Down
13 changes: 0 additions & 13 deletions tests/sqlparser_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4301,29 +4301,16 @@ fn parse_explain_table() {
validate_explain("EXPLAIN test_identifier", DescribeAlias::Explain, false);
validate_explain("DESCRIBE test_identifier", DescribeAlias::Describe, false);
validate_explain("DESC test_identifier", DescribeAlias::Desc, false);
validate_explain(
"EXPLAIN TABLE test_identifier",
DescribeAlias::Explain,
true,
);
validate_explain(
"DESCRIBE TABLE test_identifier",
DescribeAlias::Describe,
true,
);
validate_explain("DESC TABLE test_identifier", DescribeAlias::Desc, true);
}

#[test]
fn explain_describe() {
verified_stmt("DESCRIBE test.table");
verified_stmt("DESCRIBE TABLE test.table");
}

#[test]
fn explain_desc() {
verified_stmt("DESC test.table");
verified_stmt("DESC TABLE test.table");
}

#[test]
Expand Down
22 changes: 11 additions & 11 deletions tests/sqlparser_hive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use sqlparser::ast::{
UnaryOperator, Value,
};
use sqlparser::dialect::{GenericDialect, HiveDialect, MsSqlDialect};
use sqlparser::parser::{ParserError, ParserOptions};
use sqlparser::parser::ParserError;
use sqlparser::test_utils::*;

#[test]
Expand All @@ -35,18 +35,11 @@ fn parse_table_create() {
hive().verified_stmt(serdeproperties);
}

fn generic(options: Option<ParserOptions>) -> TestedDialects {
TestedDialects {
dialects: vec![Box::new(GenericDialect {})],
options,
}
}

#[test]
fn parse_describe() {
let describe = r#"DESCRIBE namespace.`table`"#;
hive().verified_stmt(describe);
generic(None).verified_stmt(describe);
hive_and_generic().verified_stmt(r#"DESCRIBE namespace.`table`"#);
hive_and_generic().verified_stmt(r#"DESCRIBE namespace.table"#);
hive_and_generic().verified_stmt(r#"DESCRIBE table"#);
}

#[test]
Expand Down Expand Up @@ -414,3 +407,10 @@ fn hive() -> TestedDialects {
options: None,
}
}

fn hive_and_generic() -> TestedDialects {
TestedDialects {
dialects: vec![Box::new(HiveDialect {}), Box::new(GenericDialect {})],
options: None,
}
}
30 changes: 30 additions & 0 deletions tests/sqlparser_snowflake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2292,3 +2292,33 @@ fn test_parse_position() {
snowflake().verified_query("SELECT position('an', 'banana', 1)");
snowflake().verified_query("SELECT n, h, POSITION(n IN h) FROM pos");
}

#[test]
fn explain_describe() {
snowflake().verified_stmt("DESCRIBE test.table");
snowflake().verified_stmt("DESCRIBE TABLE test.table");
}

#[test]
fn explain_desc() {
snowflake().verified_stmt("DESC test.table");
snowflake().verified_stmt("DESC TABLE test.table");
}

#[test]
fn parse_explain_table() {
match snowflake().verified_stmt("EXPLAIN TABLE test_identifier") {
Statement::ExplainTable {
describe_alias,
hive_format,
has_table_keyword,
table_name,
} => {
assert_eq!(describe_alias, DescribeAlias::Explain);
assert_eq!(hive_format, None);
assert_eq!(has_table_keyword, true);
assert_eq!("test_identifier", table_name.to_string());
}
_ => panic!("Unexpected Statement, must be ExplainTable"),
}
}
Loading