Skip to content

Commit de8587f

Browse files
committed
fix pr notes
1 parent 5f3de31 commit de8587f

File tree

5 files changed

+38
-43
lines changed

5 files changed

+38
-43
lines changed

src/dialect/mod.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,8 @@ pub trait Dialect: Debug + Any {
399399
self.supports_trailing_commas()
400400
}
401401

402-
/// Does the dialect support trailing commas in the FROM clause list?
402+
/// Returns true if the dialect supports trailing commas in the `FROM` clause of a `SELECT` statement.
403+
/// /// Example: `SELECT 1 FROM T, U, LIMIT 1`
403404
fn supports_from_trailing_commas(&self) -> bool {
404405
false
405406
}
@@ -763,9 +764,10 @@ pub trait Dialect: Debug + Any {
763764
keywords::RESERVED_FOR_IDENTIFIER.contains(&kw)
764765
}
765766

766-
// Returns list of keyword allowed after from
767-
fn get_reserved_keyword_after_from(&self) -> &[Keyword] {
768-
keywords::ALLOWED_KEYWORD_AFTER_FROM
767+
// Returns reserved keywords when looking to parse a [TableFactor].
768+
/// See [Self::supports_from_trailing_commas]
769+
fn get_reserved_keywords_for_table_factor(&self) -> &[Keyword] {
770+
keywords::RESERVED_FOR_TABLE_FACTOR
769771
}
770772

771773
/// Returns true if this dialect supports the `TABLESAMPLE` option

src/keywords.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -978,7 +978,7 @@ pub const RESERVED_FOR_COLUMN_ALIAS: &[Keyword] = &[
978978
// Global list of reserved keywords alloweed after FROM.
979979
// Parser should call Dialect::get_reserved_keyword_after_from
980980
// to allow for each dialect to customize the list.
981-
pub const ALLOWED_KEYWORD_AFTER_FROM: &[Keyword] = &[
981+
pub const RESERVED_FOR_TABLE_FACTOR: &[Keyword] = &[
982982
Keyword::INTO,
983983
Keyword::LIMIT,
984984
Keyword::HAVING,

src/parser/mod.rs

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3924,7 +3924,7 @@ impl<'a> Parser<'a> {
39243924
self.parse_comma_separated_with_trailing_commas(
39253925
|p| p.parse_select_item(),
39263926
trailing_commas,
3927-
self.default_reserved_keywords(),
3927+
None,
39283928
)
39293929
}
39303930

@@ -3951,14 +3951,14 @@ impl<'a> Parser<'a> {
39513951
Ok(values)
39523952
}
39533953

3954-
//Parse a 'FROM' statment. support trailing comma
3955-
pub fn parse_from(&mut self) -> Result<Vec<TableWithJoins>, ParserError> {
3954+
//Parse a list of [TableWithJoins]
3955+
fn parse_table_with_joins(&mut self) -> Result<Vec<TableWithJoins>, ParserError> {
39563956
let trailing_commas = self.dialect.supports_from_trailing_commas();
39573957

39583958
self.parse_comma_separated_with_trailing_commas(
39593959
Parser::parse_table_and_joins,
39603960
trailing_commas,
3961-
self.dialect.get_reserved_keyword_after_from(),
3961+
Some(self.dialect.get_reserved_keywords_for_table_factor()),
39623962
)
39633963
}
39643964

@@ -3968,8 +3968,9 @@ impl<'a> Parser<'a> {
39683968
fn is_parse_comma_separated_end_with_trailing_commas(
39693969
&mut self,
39703970
trailing_commas: bool,
3971-
reserved_keywords: &[Keyword],
3971+
reserved_keywords: Option<&[Keyword]>,
39723972
) -> bool {
3973+
let reserved_keywords = reserved_keywords.unwrap_or(keywords::RESERVED_FOR_COLUMN_ALIAS);
39733974
if !self.consume_token(&Token::Comma) {
39743975
true
39753976
} else if trailing_commas {
@@ -3989,26 +3990,15 @@ impl<'a> Parser<'a> {
39893990
/// Parse the comma of a comma-separated syntax element.
39903991
/// Returns true if there is a next element
39913992
fn is_parse_comma_separated_end(&mut self) -> bool {
3992-
self.is_parse_comma_separated_end_with_trailing_commas(
3993-
self.options.trailing_commas,
3994-
self.default_reserved_keywords(),
3995-
)
3996-
}
3997-
3998-
fn default_reserved_keywords(&self) -> &'static [Keyword] {
3999-
keywords::RESERVED_FOR_COLUMN_ALIAS
3993+
self.is_parse_comma_separated_end_with_trailing_commas(self.options.trailing_commas, None)
40003994
}
40013995

40023996
/// Parse a comma-separated list of 1+ items accepted by `F`
40033997
pub fn parse_comma_separated<T, F>(&mut self, f: F) -> Result<Vec<T>, ParserError>
40043998
where
40053999
F: FnMut(&mut Parser<'a>) -> Result<T, ParserError>,
40064000
{
4007-
self.parse_comma_separated_with_trailing_commas(
4008-
f,
4009-
self.options.trailing_commas,
4010-
self.default_reserved_keywords(),
4011-
)
4001+
self.parse_comma_separated_with_trailing_commas(f, self.options.trailing_commas, None)
40124002
}
40134003

40144004
/// Parse a comma-separated list of 1+ items accepted by `F`
@@ -4017,7 +4007,7 @@ impl<'a> Parser<'a> {
40174007
&mut self,
40184008
mut f: F,
40194009
trailing_commas: bool,
4020-
reserved_keywords: &[Keyword],
4010+
reserved_keywords: Option<&[Keyword]>,
40214011
) -> Result<Vec<T>, ParserError>
40224012
where
40234013
F: FnMut(&mut Parser<'a>) -> Result<T, ParserError>,
@@ -9981,7 +9971,7 @@ impl<'a> Parser<'a> {
99819971
// or `from`.
99829972

99839973
let from = if self.parse_keyword(Keyword::FROM) {
9984-
self.parse_from()?
9974+
self.parse_table_with_joins()?
99859975
} else {
99869976
vec![]
99879977
};

tests/sqlparser_common.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12796,3 +12796,9 @@ fn parse_update_from_before_select() {
1279612796
parse_sql_statements(query).unwrap_err()
1279712797
);
1279812798
}
12799+
12800+
#[test]
12801+
fn test_trailing_commas_in_from() {
12802+
let dialects = all_dialects_where(|d| d.supports_from_trailing_commas());
12803+
dialects.verified_only_select_with_canonical("SELECT 1, 2 FROM t,", "SELECT 1, 2 FROM t");
12804+
}

tests/sqlparser_snowflake.rs

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2484,36 +2484,33 @@ fn test_sf_trailing_commas() {
24842484
}
24852485

24862486
#[test]
2487-
fn test_sf_trailing_commas_in_from() {
2488-
snowflake().verified_only_select_with_canonical("SELECT 1, 2 FROM t,", "SELECT 1, 2 FROM t");
2489-
}
2490-
2491-
#[test]
2492-
fn test_sf_trailing_commas_multiple_sources() {
2487+
fn test_sf_from_trailing_commas() {
24932488
snowflake()
24942489
.verified_only_select_with_canonical("SELECT 1, 2 FROM t1, t2,", "SELECT 1, 2 FROM t1, t2");
2495-
}
24962490

2497-
#[test]
2498-
fn test_from_trailing_commas_with_limit() {
24992491
let sql = "SELECT a, FROM b, LIMIT 1";
25002492
let _ = snowflake().parse_sql_statements(sql).unwrap();
2501-
}
25022493

2503-
#[test]
2504-
fn test_sf_trailing_commas_multiple_subqueries() {
2494+
let sql = "INSERT INTO a SELECT b FROM c,";
2495+
let _ = snowflake().parse_sql_statements(sql).unwrap();
2496+
2497+
let sql = "SELECT a FROM b, HAVING COUNT(*) > 1";
2498+
let _ = snowflake().parse_sql_statements(sql).unwrap();
2499+
2500+
let sql = "SELECT a FROM b, WHERE c = 1";
2501+
let _ = snowflake().parse_sql_statements(sql).unwrap();
2502+
2503+
// nasted
2504+
let sql = "SELECT 1, 2 FROM (SELECT * FROM t,),";
2505+
let _ = snowflake().parse_sql_statements(sql).unwrap();
2506+
2507+
// multiple_subqueries
25052508
snowflake().verified_only_select_with_canonical(
25062509
"SELECT 1, 2 FROM (SELECT * FROM t1), (SELECT * FROM t2),",
25072510
"SELECT 1, 2 FROM (SELECT * FROM t1), (SELECT * FROM t2)",
25082511
);
25092512
}
25102513

2511-
#[test]
2512-
fn test_from_with_nested_trailing_commas() {
2513-
let sql = "SELECT 1, 2 FROM (SELECT * FROM t,),";
2514-
let _ = snowflake().parse_sql_statements(sql).unwrap();
2515-
}
2516-
25172514
#[test]
25182515
fn test_select_wildcard_with_ilike() {
25192516
let select = snowflake_and_generic().verified_only_select(r#"SELECT * ILIKE '%id%' FROM tbl"#);

0 commit comments

Comments
 (0)