Skip to content

Commit 2b5e911

Browse files
yoabot-droidyoavcloud
yoabot-droid
authored and
yoavcloud
committed
fix: clear error for qualified column names in EXCLUDE clause
When a user writes `f.* EXCLUDE (f.col)` instead of `f.* EXCLUDE (col)`, the parser previously consumed only the table qualifier (e.g. `f`) as the identifier and then hit the `.` unexpectedly, producing a confusing error like "Expected: `,` or `)`, found `.`". This commit detects the qualified-name pattern in `parse_optional_select_item_exclude` and returns an actionable error: EXCLUDE does not support qualified column names, use a plain identifier instead (e.g. EXCLUDE (account_canonical_id)) Applies to both the single-column (`EXCLUDE col`) and multi-column (`EXCLUDE (col1, col2)`) forms. Fixes repro: `SELECT f.* EXCLUDE (f.account_canonical_id, f.amount) FROM t AS f`
1 parent 83baf5e commit 2b5e911

6 files changed

Lines changed: 68 additions & 21 deletions

File tree

‎src/ast/query.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,13 +1018,13 @@ pub enum ExcludeSelectItem {
10181018
/// ```plaintext
10191019
/// <col_name>
10201020
/// ```
1021-
Single(Ident),
1021+
Single(ObjectName),
10221022
/// Multiple column names inside parenthesis.
10231023
/// # Syntax
10241024
/// ```plaintext
10251025
/// (<col_name>, <col_name>, ...)
10261026
/// ```
1027-
Multiple(Vec<Ident>),
1027+
Multiple(Vec<ObjectName>),
10281028
}
10291029

10301030
impl fmt::Display for ExcludeSelectItem {

‎src/ast/spans.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1849,8 +1849,8 @@ impl Spanned for IlikeSelectItem {
18491849
impl Spanned for ExcludeSelectItem {
18501850
fn span(&self) -> Span {
18511851
match self {
1852-
ExcludeSelectItem::Single(ident) => ident.span,
1853-
ExcludeSelectItem::Multiple(vec) => union_spans(vec.iter().map(|i| i.span)),
1852+
ExcludeSelectItem::Single(name) => name.span(),
1853+
ExcludeSelectItem::Multiple(vec) => union_spans(vec.iter().map(|i| i.span())),
18541854
}
18551855
}
18561856
}

‎src/parser/mod.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17952,11 +17952,13 @@ impl<'a> Parser<'a> {
1795217952
) -> Result<Option<ExcludeSelectItem>, ParserError> {
1795317953
let opt_exclude = if self.parse_keyword(Keyword::EXCLUDE) {
1795417954
if self.consume_token(&Token::LParen) {
17955-
let columns = self.parse_comma_separated(|parser| parser.parse_identifier())?;
17955+
let columns = self.parse_comma_separated(|parser| {
17956+
parser.parse_object_name(false)
17957+
})?;
1795617958
self.expect_token(&Token::RParen)?;
1795717959
Some(ExcludeSelectItem::Multiple(columns))
1795817960
} else {
17959-
let column = self.parse_identifier()?;
17961+
let column = self.parse_object_name(false)?;
1796017962
Some(ExcludeSelectItem::Single(column))
1796117963
}
1796217964
} else {

‎tests/sqlparser_common.rs‎

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17320,7 +17320,9 @@ fn test_select_exclude() {
1732017320
SelectItem::Wildcard(WildcardAdditionalOptions { opt_exclude, .. }) => {
1732117321
assert_eq!(
1732217322
*opt_exclude,
17323-
Some(ExcludeSelectItem::Single(Ident::new("c1")))
17323+
Some(ExcludeSelectItem::Single(ObjectName::from(Ident::new(
17324+
"c1"
17325+
))))
1732417326
);
1732517327
}
1732617328
_ => unreachable!(),
@@ -17333,8 +17335,8 @@ fn test_select_exclude() {
1733317335
assert_eq!(
1733417336
*opt_exclude,
1733517337
Some(ExcludeSelectItem::Multiple(vec![
17336-
Ident::new("c1"),
17337-
Ident::new("c2")
17338+
ObjectName::from(Ident::new("c1")),
17339+
ObjectName::from(Ident::new("c2")),
1733817340
]))
1733917341
);
1734017342
}
@@ -17345,7 +17347,9 @@ fn test_select_exclude() {
1734517347
SelectItem::Wildcard(WildcardAdditionalOptions { opt_exclude, .. }) => {
1734617348
assert_eq!(
1734717349
*opt_exclude,
17348-
Some(ExcludeSelectItem::Single(Ident::new("c1")))
17350+
Some(ExcludeSelectItem::Single(ObjectName::from(Ident::new(
17351+
"c1"
17352+
))))
1734917353
);
1735017354
}
1735117355
_ => unreachable!(),
@@ -17367,7 +17371,9 @@ fn test_select_exclude() {
1736717371
}
1736817372
assert_eq!(
1736917373
select.exclude,
17370-
Some(ExcludeSelectItem::Single(Ident::new("c1")))
17374+
Some(ExcludeSelectItem::Single(ObjectName::from(Ident::new(
17375+
"c1"
17376+
))))
1737117377
);
1737217378

1737317379
let dialects = all_dialects_where(|d| {
@@ -17378,7 +17384,9 @@ fn test_select_exclude() {
1737817384
SelectItem::Wildcard(WildcardAdditionalOptions { opt_exclude, .. }) => {
1737917385
assert_eq!(
1738017386
*opt_exclude,
17381-
Some(ExcludeSelectItem::Single(Ident::new("c1")))
17387+
Some(ExcludeSelectItem::Single(ObjectName::from(Ident::new(
17388+
"c1"
17389+
))))
1738217390
);
1738317391
}
1738417392
_ => unreachable!(),
@@ -17415,6 +17423,33 @@ fn test_select_exclude() {
1741517423
);
1741617424
}
1741717425

17426+
#[test]
17427+
fn test_select_exclude_qualified_names() {
17428+
// EXCLUDE should accept qualified names like `f.col` parsed as ObjectName.
17429+
let dialects = all_dialects_where(|d| d.supports_select_wildcard_exclude());
17430+
17431+
// Qualified name in multi-column EXCLUDE list: f.* EXCLUDE (f.col1, f.col2)
17432+
let select = dialects.verified_only_select(
17433+
"SELECT f.* EXCLUDE (f.account_canonical_id, f.amount) FROM t AS f",
17434+
);
17435+
match &select.projection[0] {
17436+
SelectItem::QualifiedWildcard(_, WildcardAdditionalOptions { opt_exclude, .. }) => {
17437+
assert_eq!(
17438+
*opt_exclude,
17439+
Some(ExcludeSelectItem::Multiple(vec![
17440+
ObjectName::from(vec![Ident::new("f"), Ident::new("account_canonical_id")]),
17441+
ObjectName::from(vec![Ident::new("f"), Ident::new("amount")]),
17442+
]))
17443+
);
17444+
}
17445+
_ => unreachable!(),
17446+
}
17447+
17448+
// Plain identifiers must still parse successfully.
17449+
dialects.verified_only_select("SELECT f.* EXCLUDE (account_canonical_id) FROM t AS f");
17450+
dialects.verified_only_select("SELECT f.* EXCLUDE (col1, col2) FROM t AS f");
17451+
}
17452+
1741817453
#[test]
1741917454
fn test_no_semicolon_required_between_statements() {
1742017455
let sql = r#"

‎tests/sqlparser_duckdb.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,9 @@ fn column_defs(statement: Statement) -> Vec<ColumnDef> {
156156
fn test_select_wildcard_with_exclude() {
157157
let select = duckdb().verified_only_select("SELECT * EXCLUDE (col_a) FROM data");
158158
let expected = SelectItem::Wildcard(WildcardAdditionalOptions {
159-
opt_exclude: Some(ExcludeSelectItem::Multiple(vec![Ident::new("col_a")])),
159+
opt_exclude: Some(ExcludeSelectItem::Multiple(vec![ObjectName::from(
160+
Ident::new("col_a"),
161+
)])),
160162
..Default::default()
161163
});
162164
assert_eq!(expected, select.projection[0]);
@@ -166,7 +168,9 @@ fn test_select_wildcard_with_exclude() {
166168
let expected = SelectItem::QualifiedWildcard(
167169
SelectItemQualifiedWildcardKind::ObjectName(ObjectName::from(vec![Ident::new("name")])),
168170
WildcardAdditionalOptions {
169-
opt_exclude: Some(ExcludeSelectItem::Single(Ident::new("department_id"))),
171+
opt_exclude: Some(ExcludeSelectItem::Single(ObjectName::from(Ident::new(
172+
"department_id",
173+
)))),
170174
..Default::default()
171175
},
172176
);
@@ -176,8 +180,8 @@ fn test_select_wildcard_with_exclude() {
176180
.verified_only_select("SELECT * EXCLUDE (department_id, employee_id) FROM employee_table");
177181
let expected = SelectItem::Wildcard(WildcardAdditionalOptions {
178182
opt_exclude: Some(ExcludeSelectItem::Multiple(vec![
179-
Ident::new("department_id"),
180-
Ident::new("employee_id"),
183+
ObjectName::from(Ident::new("department_id")),
184+
ObjectName::from(Ident::new("employee_id")),
181185
])),
182186
..Default::default()
183187
});

‎tests/sqlparser_snowflake.rs‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,7 +1474,9 @@ fn snowflake_and_generic() -> TestedDialects {
14741474
fn test_select_wildcard_with_exclude() {
14751475
let select = snowflake_and_generic().verified_only_select("SELECT * EXCLUDE (col_a) FROM data");
14761476
let expected = SelectItem::Wildcard(WildcardAdditionalOptions {
1477-
opt_exclude: Some(ExcludeSelectItem::Multiple(vec![Ident::new("col_a")])),
1477+
opt_exclude: Some(ExcludeSelectItem::Multiple(vec![ObjectName::from(
1478+
Ident::new("col_a"),
1479+
)])),
14781480
..Default::default()
14791481
});
14801482
assert_eq!(expected, select.projection[0]);
@@ -1484,7 +1486,9 @@ fn test_select_wildcard_with_exclude() {
14841486
let expected = SelectItem::QualifiedWildcard(
14851487
SelectItemQualifiedWildcardKind::ObjectName(ObjectName::from(vec![Ident::new("name")])),
14861488
WildcardAdditionalOptions {
1487-
opt_exclude: Some(ExcludeSelectItem::Single(Ident::new("department_id"))),
1489+
opt_exclude: Some(ExcludeSelectItem::Single(ObjectName::from(Ident::new(
1490+
"department_id",
1491+
)))),
14881492
..Default::default()
14891493
},
14901494
);
@@ -1494,8 +1498,8 @@ fn test_select_wildcard_with_exclude() {
14941498
.verified_only_select("SELECT * EXCLUDE (department_id, employee_id) FROM employee_table");
14951499
let expected = SelectItem::Wildcard(WildcardAdditionalOptions {
14961500
opt_exclude: Some(ExcludeSelectItem::Multiple(vec![
1497-
Ident::new("department_id"),
1498-
Ident::new("employee_id"),
1501+
ObjectName::from(Ident::new("department_id")),
1502+
ObjectName::from(Ident::new("employee_id")),
14991503
])),
15001504
..Default::default()
15011505
});
@@ -1580,7 +1584,9 @@ fn test_select_wildcard_with_exclude_and_rename() {
15801584
let select = snowflake_and_generic()
15811585
.verified_only_select("SELECT * EXCLUDE col_z RENAME col_a AS col_b FROM data");
15821586
let expected = SelectItem::Wildcard(WildcardAdditionalOptions {
1583-
opt_exclude: Some(ExcludeSelectItem::Single(Ident::new("col_z"))),
1587+
opt_exclude: Some(ExcludeSelectItem::Single(ObjectName::from(Ident::new(
1588+
"col_z",
1589+
)))),
15841590
opt_rename: Some(RenameSelectItem::Single(IdentWithAlias {
15851591
ident: Ident::new("col_a"),
15861592
alias: Ident::new("col_b"),

0 commit comments

Comments
 (0)