Skip to content

Commit

Permalink
refactor: remove ExcludeCol
Browse files Browse the repository at this point in the history
  • Loading branch information
TCeason committed Nov 29, 2022
1 parent a7f1923 commit e187b04
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 124 deletions.
58 changes: 24 additions & 34 deletions src/query/ast/src/ast/format/syntax/query.rs
Expand Up @@ -20,7 +20,6 @@ use crate::ast::format::syntax::inline_dot;
use crate::ast::format::syntax::interweave_comma;
use crate::ast::format::syntax::parenthenized;
use crate::ast::format::syntax::NEST_FACTOR;
use crate::ast::ExcludeCol;
use crate::ast::Expr;
use crate::ast::JoinCondition;
use crate::ast::JoinOperator;
Expand Down Expand Up @@ -133,39 +132,30 @@ fn pretty_select_list(select_list: Vec<SelectTarget>) -> RcDoc {
.map(|indirection| RcDoc::text(indirection.to_string())),
)
.group();
docs.append(if let Some(exclude) = exclude {
match exclude {
ExcludeCol::Col(col) => RcDoc::line().append(
RcDoc::text("EXCEPT")
.append(RcDoc::space().nest(NEST_FACTOR))
.append(RcDoc::text(col.to_string())),
),
ExcludeCol::Cols(cols) => {
if !cols.is_empty() {
RcDoc::line()
.append(
RcDoc::text("EXCEPT").append(
if cols.len() > 1 {
RcDoc::line()
} else {
RcDoc::space()
}
.nest(NEST_FACTOR),
),
)
.append(
interweave_comma(cols.into_iter().map(|ident| {
RcDoc::space()
.append(RcDoc::space())
.append(RcDoc::text(ident.to_string()))
}))
.nest(NEST_FACTOR)
.group(),
)
} else {
RcDoc::nil()
}
}
docs.append(if let Some(cols) = exclude {
if !cols.is_empty() {
RcDoc::line()
.append(
RcDoc::text("EXCLUDE").append(
if cols.len() > 1 {
RcDoc::line()
} else {
RcDoc::space()
}
.nest(NEST_FACTOR),
),
)
.append(
interweave_comma(cols.into_iter().map(|ident| {
RcDoc::space()
.append(RcDoc::space())
.append(RcDoc::text(ident.to_string()))
}))
.nest(NEST_FACTOR)
.group(),
)
} else {
RcDoc::nil()
}
} else {
RcDoc::nil()
Expand Down
41 changes: 7 additions & 34 deletions src/query/ast/src/ast/query.rs
Expand Up @@ -123,29 +123,10 @@ pub enum SelectTarget<'a> {
// For simplicity, wildcard is involved.
QualifiedName {
qualified: QualifiedName<'a>,
exclude: Option<ExcludeCol<'a>>,
exclude: Option<Vec<Identifier<'a>>>,
},
}

#[derive(Debug, Clone, PartialEq)]
pub enum ExcludeCol<'a> {
Col(Identifier<'a>),
Cols(Vec<Identifier<'a>>),
}

impl Display for ExcludeCol<'_> {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
ExcludeCol::Col(col) => write!(f, "{col}"),
ExcludeCol::Cols(cols) => {
write!(f, "(")?;
write_comma_separated_list(f, cols)?;
write!(f, ")")
}
}
}
}

pub type QualifiedName<'a> = Vec<Indirection<'a>>;

/// Indirection of a select result, like a part of `db.table.column`.
Expand Down Expand Up @@ -394,20 +375,12 @@ impl<'a> Display for SelectTarget<'a> {
}
SelectTarget::QualifiedName { qualified, exclude } => {
write_period_separated_list(f, qualified)?;
if let Some(exclude) = exclude {
// ORDER BY clause
match exclude {
ExcludeCol::Col(col) => {
write!(f, " EXCLUDE")?;
write!(f, " {col}")?;
}
ExcludeCol::Cols(cols) => {
if !cols.is_empty() {
write!(f, " EXCLUDE (")?;
write_comma_separated_list(f, cols)?;
write!(f, ")")?;
}
}
if let Some(cols) = exclude {
// EXCLUDE
if !cols.is_empty() {
write!(f, " EXCLUDE (")?;
write_comma_separated_list(f, cols)?;
write!(f, ")")?;
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/query/ast/src/parser/query.rs
Expand Up @@ -83,18 +83,18 @@ pub fn with(i: Input) -> IResult<With> {
)(i)
}

pub fn exclude_col(i: Input) -> IResult<ExcludeCol> {
pub fn exclude_col(i: Input) -> IResult<Vec<Identifier>> {
let var = map(
rule! {
#ident
},
ExcludeCol::Col,
|col| vec![col],
);
let vars = map(
rule! {
"(" ~ ^#comma_separated_list1(ident) ~ ")"
},
|(_, cols, _)| ExcludeCol::Cols(cols),
|(_, cols, _)| cols,
);

rule!(
Expand Down
11 changes: 3 additions & 8 deletions src/query/ast/src/visitors/walk.rs
Expand Up @@ -197,14 +197,9 @@ pub fn walk_select_target<'a, V: Visitor<'a>>(visitor: &mut V, target: &'a Selec
Indirection::Star => {}
}
}
if let Some(exclude) = exclude {
match exclude {
ExcludeCol::Col(col) => visitor.visit_identifier(col),
ExcludeCol::Cols(cols) => {
for ident in cols.iter() {
visitor.visit_identifier(ident);
}
}
if let Some(cols) = exclude {
for ident in cols.iter() {
visitor.visit_identifier(ident);
}
}
}
Expand Down
11 changes: 3 additions & 8 deletions src/query/ast/src/visitors/walk_mut.rs
Expand Up @@ -197,14 +197,9 @@ pub fn walk_select_target_mut<'a, V: VisitorMut>(visitor: &mut V, target: &mut S
Indirection::Star => {}
}
}
if let Some(exclude) = exclude {
match exclude {
ExcludeCol::Col(col) => visitor.visit_identifier(col),
ExcludeCol::Cols(cols) => {
for ident in cols {
visitor.visit_identifier(ident);
}
}
if let Some(cols) = exclude {
for ident in cols {
visitor.visit_identifier(ident);
}
}
}
Expand Down
42 changes: 20 additions & 22 deletions src/query/ast/tests/it/testdata/query.txt
@@ -1,7 +1,7 @@
---------- Input ----------
select * exclude c1, b.* exclude (c2, c3, c4) from customer inner join orders on a = b limit 1
---------- Output ---------
SELECT * EXCLUDE c1, b.* EXCLUDE (c2, c3, c4) FROM customer INNER JOIN orders ON a = b LIMIT 1
SELECT * EXCLUDE (c1), b.* EXCLUDE (c2, c3, c4) FROM customer INNER JOIN orders ON a = b LIMIT 1
---------- AST ------------
Query {
span: [
Expand Down Expand Up @@ -70,13 +70,13 @@ Query {
Star,
],
exclude: Some(
Col(
[
Identifier {
name: "c1",
quote: None,
span: Ident(17..19),
},
),
],
),
},
QualifiedName {
Expand All @@ -91,25 +91,23 @@ Query {
Star,
],
exclude: Some(
Cols(
[
Identifier {
name: "c2",
quote: None,
span: Ident(34..36),
},
Identifier {
name: "c3",
quote: None,
span: Ident(38..40),
},
Identifier {
name: "c4",
quote: None,
span: Ident(42..44),
},
],
),
[
Identifier {
name: "c2",
quote: None,
span: Ident(34..36),
},
Identifier {
name: "c3",
quote: None,
span: Ident(38..40),
},
Identifier {
name: "c4",
quote: None,
span: Ident(42..44),
},
],
),
},
],
Expand Down
22 changes: 7 additions & 15 deletions src/query/sql/src/planner/binder/project.rs
Expand Up @@ -15,7 +15,6 @@
use std::collections::HashMap;
use std::collections::HashSet;

use common_ast::ast::ExcludeCol;
use common_ast::ast::Identifier;
use common_ast::ast::Indirection;
use common_ast::ast::SelectTarget;
Expand Down Expand Up @@ -159,20 +158,13 @@ impl<'a> Binder {
} => {
// Handle qualified name as select target
let mut exclude_cols: HashSet<String> = HashSet::new();
if let Some(exclude) = exclude {
match exclude {
ExcludeCol::Col(col) => {
exclude_cols.insert(col.name.clone());
}
ExcludeCol::Cols(cols) => {
for col in cols {
exclude_cols.insert(col.name.clone());
}
if exclude_cols.len() < cols.len() {
// * except (id, id)
return Err(ErrorCode::SemanticError("duplicate column name"));
}
}
if let Some(cols) = exclude {
for col in cols {
exclude_cols.insert(col.name.clone());
}
if exclude_cols.len() < cols.len() {
// * except (id, id)
return Err(ErrorCode::SemanticError("duplicate column name"));
}
}

Expand Down

0 comments on commit e187b04

Please sign in to comment.