From fda87f9734a3f9080e2826840c666da25a6bb9f4 Mon Sep 17 00:00:00 2001 From: parkma99 Date: Tue, 9 May 2023 10:47:53 +0800 Subject: [PATCH 1/2] Improve error message for CREATE EXTERNAL TABLE --- .../test_files/create_external_table.slt | 8 +++++++ datafusion/sql/src/parser.rs | 24 ++++++++++--------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/datafusion/core/tests/sqllogictests/test_files/create_external_table.slt b/datafusion/core/tests/sqllogictests/test_files/create_external_table.slt index b67c6f20b3d8e..586ead8f60e34 100644 --- a/datafusion/core/tests/sqllogictests/test_files/create_external_table.slt +++ b/datafusion/core/tests/sqllogictests/test_files/create_external_table.slt @@ -91,3 +91,11 @@ create EXTERNAL TABLE t(c1 int, c2 int) STORED AS CSV PARTITIONED BY (c1) partit # Duplicate `OPTIONS` clause statement error DataFusion error: SQL error: ParserError\("OPTIONS specified more than once"\) CREATE EXTERNAL TABLE t STORED AS CSV OPTIONS ('k1' 'v1', 'k2' 'v2') OPTIONS ('k3' 'v3') LOCATION 'foo.csv' + +# With typo error +statement error DataFusion error: SQL error: ParserError\("WITH clause is incomplete or has typo error in CREATE EXTERNAL TABLE statement"\) +CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV WITH HEAD ROW LOCATION 'foo.csv'; + +# Missing `anything` in WITH clause +statement error DataFusion error: SQL error: ParserError\("WITH clause is incomplete or has typo error in CREATE EXTERNAL TABLE statement"\) +CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV WITH LOCATION 'foo.csv'; diff --git a/datafusion/sql/src/parser.rs b/datafusion/sql/src/parser.rs index ccd94edd0f815..76c171e4a8189 100644 --- a/datafusion/sql/src/parser.rs +++ b/datafusion/sql/src/parser.rs @@ -434,16 +434,19 @@ impl<'a> DFParser<'a> { } else if self.parser.parse_keyword(Keyword::LOCATION) { ensure_not_set(&builder.location, "LOCATION")?; builder.location = Some(self.parser.parse_literal_string()?); - } else if self - .parser - .parse_keywords(&[Keyword::WITH, Keyword::HEADER]) - { - self.parser.expect_keyword(Keyword::ROW)?; - ensure_not_set(&builder.has_header, "WITH HEADER ROW")?; - builder.has_header = Some(true); - } else if self.parser.parse_keywords(&[Keyword::WITH, Keyword::ORDER]) { - ensure_not_set(&builder.order_exprs, "WITH ORDER")?; - builder.order_exprs = Some(self.parse_order_by_exprs()?); + } else if self.parser.parse_keyword(Keyword::WITH) { + if self.parser.parse_keyword(Keyword::HEADER) { + self.parser.expect_keyword(Keyword::ROW)?; + ensure_not_set(&builder.has_header, "WITH HEADER ROW")?; + builder.has_header = Some(true); + } else if self.parser.parse_keyword(Keyword::ORDER) { + ensure_not_set(&builder.order_exprs, "WITH ORDER")?; + builder.order_exprs = Some(self.parse_order_by_exprs()?); + } else { + return Err(ParserError::ParserError( + "WITH clause is incomplete or has typo error in CREATE EXTERNAL TABLE statement".into(), + )); + } } else if self.parser.parse_keyword(Keyword::DELIMITER) { ensure_not_set(&builder.delimiter, "DELIMITER")?; builder.delimiter = Some(self.parse_delimiter()?); @@ -977,7 +980,6 @@ mod tests { expect_parse_ok(sql, expected)?; // For error cases, see: `create_external_table.slt` - Ok(()) } } From 6d151288953b48dd0513078f49e50bfb510ad3da Mon Sep 17 00:00:00 2001 From: parkma99 Date: Tue, 9 May 2023 19:08:45 +0800 Subject: [PATCH 2/2] change error message by using expect_keyword --- .../test_files/create_external_table.slt | 6 +++--- datafusion/sql/src/parser.rs | 14 ++++++-------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/datafusion/core/tests/sqllogictests/test_files/create_external_table.slt b/datafusion/core/tests/sqllogictests/test_files/create_external_table.slt index 586ead8f60e34..a0784ff96def8 100644 --- a/datafusion/core/tests/sqllogictests/test_files/create_external_table.slt +++ b/datafusion/core/tests/sqllogictests/test_files/create_external_table.slt @@ -93,9 +93,9 @@ statement error DataFusion error: SQL error: ParserError\("OPTIONS specified mor CREATE EXTERNAL TABLE t STORED AS CSV OPTIONS ('k1' 'v1', 'k2' 'v2') OPTIONS ('k3' 'v3') LOCATION 'foo.csv' # With typo error -statement error DataFusion error: SQL error: ParserError\("WITH clause is incomplete or has typo error in CREATE EXTERNAL TABLE statement"\) +statement error DataFusion error: SQL error: ParserError\("Expected HEADER, found: HEAD"\) CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV WITH HEAD ROW LOCATION 'foo.csv'; # Missing `anything` in WITH clause -statement error DataFusion error: SQL error: ParserError\("WITH clause is incomplete or has typo error in CREATE EXTERNAL TABLE statement"\) -CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV WITH LOCATION 'foo.csv'; +statement error DataFusion error: SQL error: ParserError\("Expected HEADER, found: LOCATION"\) +CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV WITH LOCATION 'foo.csv'; diff --git a/datafusion/sql/src/parser.rs b/datafusion/sql/src/parser.rs index 76c171e4a8189..b1116af3cfea0 100644 --- a/datafusion/sql/src/parser.rs +++ b/datafusion/sql/src/parser.rs @@ -435,17 +435,14 @@ impl<'a> DFParser<'a> { ensure_not_set(&builder.location, "LOCATION")?; builder.location = Some(self.parser.parse_literal_string()?); } else if self.parser.parse_keyword(Keyword::WITH) { - if self.parser.parse_keyword(Keyword::HEADER) { - self.parser.expect_keyword(Keyword::ROW)?; - ensure_not_set(&builder.has_header, "WITH HEADER ROW")?; - builder.has_header = Some(true); - } else if self.parser.parse_keyword(Keyword::ORDER) { + if self.parser.parse_keyword(Keyword::ORDER) { ensure_not_set(&builder.order_exprs, "WITH ORDER")?; builder.order_exprs = Some(self.parse_order_by_exprs()?); } else { - return Err(ParserError::ParserError( - "WITH clause is incomplete or has typo error in CREATE EXTERNAL TABLE statement".into(), - )); + self.parser.expect_keyword(Keyword::HEADER)?; + self.parser.expect_keyword(Keyword::ROW)?; + ensure_not_set(&builder.has_header, "WITH HEADER ROW")?; + builder.has_header = Some(true); } } else if self.parser.parse_keyword(Keyword::DELIMITER) { ensure_not_set(&builder.delimiter, "DELIMITER")?; @@ -980,6 +977,7 @@ mod tests { expect_parse_ok(sql, expected)?; // For error cases, see: `create_external_table.slt` + Ok(()) } }