Skip to content

Support HAVING/LIMIT/OFFSET/FETCH without FROM and other follow-ups #116

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 7 commits into from
Jun 17, 2019
Merged
6 changes: 3 additions & 3 deletions src/dialect/keywords.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,16 +420,16 @@ define_keywords!(
/// can be parsed unambiguously without looking ahead.
pub const RESERVED_FOR_TABLE_ALIAS: &[&str] = &[
// Reserved as both a table and a column alias:
WITH, SELECT, WHERE, GROUP, ORDER, UNION, EXCEPT, INTERSECT,
WITH, SELECT, WHERE, GROUP, ORDER, LIMIT, OFFSET, FETCH, UNION, EXCEPT, INTERSECT,
// Reserved only as a table alias in the `FROM`/`JOIN` clauses:
ON, JOIN, INNER, CROSS, FULL, LEFT, RIGHT, NATURAL, USING, LIMIT, OFFSET, FETCH,
ON, JOIN, INNER, CROSS, FULL, LEFT, RIGHT, NATURAL, USING,
];

/// Can't be used as a column alias, so that `SELECT <expr> alias`
/// can be parsed unambiguously without looking ahead.
pub const RESERVED_FOR_COLUMN_ALIAS: &[&str] = &[
// Reserved as both a table and a column alias:
WITH, SELECT, WHERE, GROUP, ORDER, UNION, EXCEPT, INTERSECT,
WITH, SELECT, WHERE, GROUP, ORDER, LIMIT, OFFSET, FETCH, UNION, EXCEPT, INTERSECT,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this isn't the fault of this commit, but it's a shame that these keywords have to be duplicated between the two constants. I don't have any obvious solutions, and definitely doesn't need to be considered in this PR; just thought I'd flag it in case you happened to have any ideas.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like it either, but the solutions seemed too complex for a trivial problem...

These lists will become dialect-specific at some point, and we would be able to build the reserved lists in the dialect constructor then, I guess.

// Reserved only as a column alias in the `SELECT` clause:
FROM,
];
9 changes: 2 additions & 7 deletions src/sqlast/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,18 +163,13 @@ impl ToString for SQLSelect {
/// number of columns in the query matches the number of columns in the query.
#[derive(Debug, Clone, PartialEq, Hash)]
pub struct Cte {
pub alias: SQLIdent,
pub alias: TableAlias,
pub query: SQLQuery,
pub renamed_columns: Vec<SQLIdent>,
}

impl ToString for Cte {
fn to_string(&self) -> String {
let mut s = self.alias.clone();
if !self.renamed_columns.is_empty() {
s += &format!(" ({})", comma_separated_string(&self.renamed_columns));
}
s + &format!(" AS ({})", self.query.to_string())
format!("{} AS ({})", self.alias.to_string(), self.query.to_string())
}
}

Expand Down
13 changes: 7 additions & 6 deletions src/sqlparser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,13 +574,13 @@ impl Parser {
self.expected("IN or BETWEEN after NOT", self.peek_token())
}
}
// Can only happen if `get_precedence` got out of sync with this function
// Can only happen if `get_next_precedence` got out of sync with this function
_ => panic!("No infix parser for token {:?}", tok),
}
} else if Token::DoubleColon == tok {
self.parse_pg_cast(expr)
} else {
// Can only happen if `get_precedence` got out of sync with this function
// Can only happen if `get_next_precedence` got out of sync with this function
panic!("No infix parser for token {:?}", tok)
}
}
Expand Down Expand Up @@ -636,7 +636,7 @@ impl Parser {
/// Get the precedence of the next token
pub fn get_next_precedence(&self) -> Result<u8, ParserError> {
if let Some(token) = self.peek_token() {
debug!("get_precedence() {:?}", token);
debug!("get_next_precedence() {:?}", token);

match &token {
Token::SQLWord(k) if k.keyword == "OR" => Ok(5),
Expand Down Expand Up @@ -1475,14 +1475,15 @@ impl Parser {
fn parse_cte_list(&mut self) -> Result<Vec<Cte>, ParserError> {
let mut cte = vec![];
loop {
let alias = self.parse_identifier()?;
let renamed_columns = self.parse_parenthesized_column_list(Optional)?;
let alias = TableAlias {
name: self.parse_identifier()?,
columns: self.parse_parenthesized_column_list(Optional)?,
};
self.expect_keyword("AS")?;
self.expect_token(&Token::LParen)?;
cte.push(Cte {
alias,
query: self.parse_query()?,
renamed_columns,
});
self.expect_token(&Token::RParen)?;
if !self.consume_token(&Token::Comma) {
Expand Down
107 changes: 28 additions & 79 deletions tests/sqlparser_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,13 +212,13 @@ fn parse_simple_select() {
}

#[test]
fn parse_select_with_limit_but_no_where() {
let sql = "SELECT id, fname, lname FROM customer LIMIT 5";
let select = verified_only_select(sql);
assert_eq!(false, select.distinct);
assert_eq!(3, select.projection.len());
let select = verified_query(sql);
assert_eq!(Some(ASTNode::SQLValue(Value::Long(5))), select.limit);
fn parse_limit_is_not_an_alias() {
// In dialects supporting LIMIT it shouldn't be parsed as a table alias
let ast = verified_query("SELECT id FROM customer LIMIT 1");
assert_eq!(Some(ASTNode::SQLValue(Value::Long(1))), ast.limit);

let ast = verified_query("SELECT 1 LIMIT 5");
assert_eq!(Some(ASTNode::SQLValue(Value::Long(5))), ast.limit);
}

#[test]
Expand Down Expand Up @@ -1791,14 +1791,10 @@ fn parse_ctes() {
fn assert_ctes_in_select(expected: &[&str], sel: &SQLQuery) {
let mut i = 0;
for exp in expected {
let Cte {
query,
alias,
renamed_columns,
} = &sel.ctes[i];
let Cte { alias, query } = &sel.ctes[i];
assert_eq!(*exp, query.to_string());
assert_eq!(if i == 0 { "a" } else { "b" }, alias);
assert!(renamed_columns.is_empty());
assert_eq!(if i == 0 { "a" } else { "b" }, alias.name);
assert!(alias.columns.is_empty());
i += 1;
}
}
Expand Down Expand Up @@ -1841,7 +1837,7 @@ fn parse_cte_renamed_columns() {
let query = all_dialects().verified_query(sql);
assert_eq!(
vec!["col1", "col2"],
query.ctes.first().unwrap().renamed_columns
query.ctes.first().unwrap().alias.columns
);
}

Expand Down Expand Up @@ -2201,6 +2197,8 @@ fn parse_offset() {
},
_ => panic!("Test broke"),
}
let ast = verified_query("SELECT 'foo' OFFSET 0 ROWS");
assert_eq!(ast.offset, Some(ASTNode::SQLValue(Value::Long(0))));
}

#[test]
Expand All @@ -2213,15 +2211,15 @@ fn parse_singular_row_offset() {

#[test]
fn parse_fetch() {
const FETCH_FIRST_TWO_ROWS_ONLY: Fetch = Fetch {
with_ties: false,
percent: false,
quantity: Some(ASTNode::SQLValue(Value::Long(2))),
};
let ast = verified_query("SELECT foo FROM bar FETCH FIRST 2 ROWS ONLY");
assert_eq!(
ast.fetch,
Some(Fetch {
with_ties: false,
percent: false,
quantity: Some(ASTNode::SQLValue(Value::Long(2))),
})
);
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
let ast = verified_query("SELECT 'foo' FETCH FIRST 2 ROWS ONLY");
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
let ast = verified_query("SELECT foo FROM bar FETCH FIRST ROWS ONLY");
assert_eq!(
ast.fetch,
Expand All @@ -2232,23 +2230,9 @@ fn parse_fetch() {
})
);
let ast = verified_query("SELECT foo FROM bar WHERE foo = 4 FETCH FIRST 2 ROWS ONLY");
assert_eq!(
ast.fetch,
Some(Fetch {
with_ties: false,
percent: false,
quantity: Some(ASTNode::SQLValue(Value::Long(2))),
})
);
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
let ast = verified_query("SELECT foo FROM bar ORDER BY baz FETCH FIRST 2 ROWS ONLY");
assert_eq!(
ast.fetch,
Some(Fetch {
with_ties: false,
percent: false,
quantity: Some(ASTNode::SQLValue(Value::Long(2))),
})
);
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
let ast = verified_query(
"SELECT foo FROM bar WHERE foo = 4 ORDER BY baz FETCH FIRST 2 ROWS WITH TIES",
);
Expand All @@ -2273,63 +2257,28 @@ fn parse_fetch() {
"SELECT foo FROM bar WHERE foo = 4 ORDER BY baz OFFSET 2 ROWS FETCH FIRST 2 ROWS ONLY",
);
assert_eq!(ast.offset, Some(ASTNode::SQLValue(Value::Long(2))));
assert_eq!(
ast.fetch,
Some(Fetch {
with_ties: false,
percent: false,
quantity: Some(ASTNode::SQLValue(Value::Long(2))),
})
);
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
let ast = verified_query(
"SELECT foo FROM (SELECT * FROM bar FETCH FIRST 2 ROWS ONLY) FETCH FIRST 2 ROWS ONLY",
);
assert_eq!(
ast.fetch,
Some(Fetch {
with_ties: false,
percent: false,
quantity: Some(ASTNode::SQLValue(Value::Long(2))),
})
);
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
match ast.body {
SQLSetExpr::Select(s) => match only(s.from).relation {
TableFactor::Derived { subquery, .. } => {
assert_eq!(
subquery.fetch,
Some(Fetch {
with_ties: false,
percent: false,
quantity: Some(ASTNode::SQLValue(Value::Long(2))),
})
);
assert_eq!(subquery.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
}
_ => panic!("Test broke"),
},
_ => panic!("Test broke"),
}
let ast = verified_query("SELECT foo FROM (SELECT * FROM bar OFFSET 2 ROWS FETCH FIRST 2 ROWS ONLY) OFFSET 2 ROWS FETCH FIRST 2 ROWS ONLY");
assert_eq!(ast.offset, Some(ASTNode::SQLValue(Value::Long(2))));
assert_eq!(
ast.fetch,
Some(Fetch {
with_ties: false,
percent: false,
quantity: Some(ASTNode::SQLValue(Value::Long(2))),
})
);
assert_eq!(ast.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
match ast.body {
SQLSetExpr::Select(s) => match only(s.from).relation {
TableFactor::Derived { subquery, .. } => {
assert_eq!(subquery.offset, Some(ASTNode::SQLValue(Value::Long(2))));
assert_eq!(
subquery.fetch,
Some(Fetch {
with_ties: false,
percent: false,
quantity: Some(ASTNode::SQLValue(Value::Long(2))),
})
);
assert_eq!(subquery.fetch, Some(FETCH_FIRST_TWO_ROWS_ONLY));
}
_ => panic!("Test broke"),
},
Expand Down