LucaCappelletti94 commented on code in PR #2499:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2499#discussion_r4093450689
##########
src/parser/mod.rs:
##########
@@ -13636,6 +13666,57 @@ impl<'a> Parser<'a> {
}
}
+ fn parse_limit_quantity(&mut self) -> Result<ParsedLimit, ParserError> {
+ if !self.dialect.supports_limit_percent() {
+ return self.parse_limit().map(ParsedLimit::Rows);
+ }
+
+ if self.parse_keyword(Keyword::ALL) {
+ return Ok(ParsedLimit::Rows(None));
+ }
+
+ let limit = self.parse_expr_until(Parser::at_limit_percent_suffix)?;
+ if !self.consume_token(&Token::Mod) {
+ return Ok(ParsedLimit::Rows(Some(limit)));
+ }
+
+ if matches!(
+ &limit,
+ Expr::Value(value) if matches!(&value.value, Value::Number(_,
true))
+ ) {
+ return self.expected_ref("an expression", self.peek_token_ref());
+ }
+
+ Ok(ParsedLimit::Percent(limit))
Review Comment:
You should drop the `Value::Number(_, true)` check. It rejects `LIMIT 25L%`
while `LIMIT 25L` and `LIMIT (25L)%` both parse, and without it `LIMIT 25L%`
round-trips.
```suggestion
Ok(ParsedLimit::Percent(limit))
```
##########
tests/sqlparser_duckdb.rs:
##########
@@ -925,3 +925,333 @@ fn test_duckdb_lambda_function() {
let sql_transform = "SELECT list_transform([1, 2, 3], lambda x : x * 2)";
duckdb().verified_stmt(sql_transform);
}
+
+#[test]
+fn test_limit_percent_round_trip_and_ast() {
+ duckdb().one_statement_parses_to(
+ "SELECT n FROM (VALUES (1), (2), (3), (4)) AS t(n) ORDER BY n LIMIT
50%",
+ "SELECT n FROM (VALUES (1), (2), (3), (4)) AS t (n) ORDER BY n LIMIT
50%",
+ );
+
+ let query = duckdb().verified_query("SELECT 1 LIMIT 25% OFFSET 1");
+ assert_eq!(
+ query.limit_clause,
+ Some(LimitClause::Percent {
+ limit: Expr::value(number("25")),
+ offset: Some(Offset {
+ value: Expr::value(number("1")),
+ rows: OffsetRows::None,
+ }),
+ })
+ );
+
+ duckdb().one_statement_parses_to("SELECT 1 OFFSET 1 LIMIT 25%", "SELECT 1
LIMIT 25% OFFSET 1");
+ duckdb().verified_stmt("SELECT * FROM (SELECT 1 LIMIT 25%) AS t");
+ duckdb().statements_parse_to(
+ "SELECT 1 LIMIT 25%; SELECT 2",
+ "SELECT 1 LIMIT 25%; SELECT 2",
+ );
+}
+
+#[test]
+fn test_limit_percent_walkthrough_examples() {
+ for (sql, canonical, limit, offset) in [
+ (
+ "SELECT 1 LIMIT 10 * 5% OFFSET 2",
+ "SELECT 1 LIMIT 10 * 5% OFFSET 2",
+ Expr::BinaryOp {
+ left: Box::new(Expr::value(number("10"))),
+ op: BinaryOperator::Multiply,
+ right: Box::new(Expr::value(number("5"))),
+ },
+ Some(Offset {
+ value: Expr::value(number("2")),
+ rows: OffsetRows::None,
+ }),
+ ),
+ (
+ "SELECT 1 LIMIT 25% OFFSET 2",
+ "SELECT 1 LIMIT 25% OFFSET 2",
+ Expr::value(number("25")),
+ Some(Offset {
+ value: Expr::value(number("2")),
+ rows: OffsetRows::None,
+ }),
+ ),
+ (
+ "SELECT 1 OFFSET 2 LIMIT 25%",
+ "SELECT 1 LIMIT 25% OFFSET 2",
+ Expr::value(number("25")),
+ Some(Offset {
+ value: Expr::value(number("2")),
+ rows: OffsetRows::None,
+ }),
+ ),
+ (
+ "SELECT 1 OFFSET 5 LIMIT 10%",
+ "SELECT 1 LIMIT 10% OFFSET 5",
+ Expr::value(number("10")),
+ Some(Offset {
+ value: Expr::value(number("5")),
+ rows: OffsetRows::None,
+ }),
+ ),
+ ] {
+ let query = duckdb().verified_query_with_canonical(sql, canonical);
+ assert_eq!(
+ query.limit_clause,
+ Some(LimitClause::Percent { limit, offset }),
+ "{sql}",
+ );
+ }
+}
+
+#[test]
+fn test_limit_percent_expression_quantities() {
+ for sql in [
+ "SELECT 1 LIMIT ?%",
+ "SELECT 1 LIMIT $1%",
+ "SELECT 1 LIMIT -25%",
+ "SELECT 1 LIMIT +25%",
+ "SELECT 1 LIMIT (25)%",
+ "SELECT 1 LIMIT (10 + 15)%",
+ "SELECT 1 LIMIT (10 % 3)%",
+ "SELECT 1 LIMIT 10 * 5%",
+ "SELECT 1 LIMIT 10 % 3%",
+ "SELECT 1 LIMIT abs(10 % 3)%",
+ "SELECT 1 LIMIT CAST(25 AS INTEGER)%",
+ ] {
+ duckdb().verified_stmt(sql);
+ }
+}
+
+#[test]
+fn test_limit_percent_preserves_modulo() {
+ duckdb_and_generic().verified_stmt("SELECT 5 % 2");
+
+ let query = duckdb_and_generic().verified_query("SELECT 1 LIMIT 5 % 2");
+ assert_eq!(
+ query.limit_clause,
+ Some(LimitClause::LimitOffset {
+ limit: Some(Expr::BinaryOp {
+ left: Box::new(Expr::value(number("5"))),
+ op: BinaryOperator::Modulo,
+ right: Box::new(Expr::value(number("2"))),
+ }),
+ offset: None,
+ limit_by: vec![],
+ })
+ );
+
+ duckdb_and_generic().verified_stmt("SELECT 1 LIMIT (5 % 2)");
+
+ for sql in [
+ "SELECT 1 LIMIT ? % 2",
+ "SELECT 1 LIMIT $1 % 2",
+ "SELECT 1 LIMIT -25 % 2",
+ "SELECT 1 LIMIT (25) % 2",
+ "SELECT 1 LIMIT 10 + 15 % 2",
+ "SELECT 1 LIMIT CAST(25 AS INTEGER) % 2",
+ ] {
+ duckdb_and_generic().verified_stmt(sql);
+ }
+}
+
+#[test]
+fn test_limit_percent_rejects_malformed_syntax() {
+ for (sql, expected) in [
+ (
+ "SELECT 1 LIMIT %",
+ ParserError::ParserError("Expected: an expression, found:
%".to_string()),
+ ),
+ (
+ "SELECT 1 LIMIT ALL%",
+ ParserError::ParserError("Expected: end of statement, found:
%".to_string()),
+ ),
+ (
+ "SELECT 1 LIMIT 25%%",
+ ParserError::ParserError("Expected: an expression, found:
%".to_string()),
+ ),
+ (
+ "SELECT 1 LIMIT 25L%",
+ ParserError::ParserError("Expected: an expression, found:
EOF".to_string()),
+ ),
Review Comment:
You should remove this case along with the check above, since it fails once
the check is gone.
```suggestion
```
##########
tests/sqlparser_duckdb.rs:
##########
@@ -925,3 +925,333 @@ fn test_duckdb_lambda_function() {
let sql_transform = "SELECT list_transform([1, 2, 3], lambda x : x * 2)";
duckdb().verified_stmt(sql_transform);
}
+
+#[test]
+fn test_limit_percent_round_trip_and_ast() {
+ duckdb().one_statement_parses_to(
+ "SELECT n FROM (VALUES (1), (2), (3), (4)) AS t(n) ORDER BY n LIMIT
50%",
+ "SELECT n FROM (VALUES (1), (2), (3), (4)) AS t (n) ORDER BY n LIMIT
50%",
+ );
+
+ let query = duckdb().verified_query("SELECT 1 LIMIT 25% OFFSET 1");
+ assert_eq!(
+ query.limit_clause,
+ Some(LimitClause::Percent {
+ limit: Expr::value(number("25")),
+ offset: Some(Offset {
+ value: Expr::value(number("1")),
+ rows: OffsetRows::None,
+ }),
+ })
+ );
+
+ duckdb().one_statement_parses_to("SELECT 1 OFFSET 1 LIMIT 25%", "SELECT 1
LIMIT 25% OFFSET 1");
+ duckdb().verified_stmt("SELECT * FROM (SELECT 1 LIMIT 25%) AS t");
+ duckdb().statements_parse_to(
+ "SELECT 1 LIMIT 25%; SELECT 2",
+ "SELECT 1 LIMIT 25%; SELECT 2",
+ );
+}
+
+#[test]
+fn test_limit_percent_walkthrough_examples() {
+ for (sql, canonical, limit, offset) in [
+ (
+ "SELECT 1 LIMIT 10 * 5% OFFSET 2",
+ "SELECT 1 LIMIT 10 * 5% OFFSET 2",
+ Expr::BinaryOp {
+ left: Box::new(Expr::value(number("10"))),
+ op: BinaryOperator::Multiply,
+ right: Box::new(Expr::value(number("5"))),
+ },
+ Some(Offset {
+ value: Expr::value(number("2")),
+ rows: OffsetRows::None,
+ }),
+ ),
+ (
+ "SELECT 1 LIMIT 25% OFFSET 2",
+ "SELECT 1 LIMIT 25% OFFSET 2",
+ Expr::value(number("25")),
+ Some(Offset {
+ value: Expr::value(number("2")),
+ rows: OffsetRows::None,
+ }),
+ ),
+ (
+ "SELECT 1 OFFSET 2 LIMIT 25%",
+ "SELECT 1 LIMIT 25% OFFSET 2",
+ Expr::value(number("25")),
+ Some(Offset {
+ value: Expr::value(number("2")),
+ rows: OffsetRows::None,
+ }),
+ ),
+ (
+ "SELECT 1 OFFSET 5 LIMIT 10%",
+ "SELECT 1 LIMIT 10% OFFSET 5",
+ Expr::value(number("10")),
+ Some(Offset {
+ value: Expr::value(number("5")),
+ rows: OffsetRows::None,
+ }),
+ ),
+ ] {
+ let query = duckdb().verified_query_with_canonical(sql, canonical);
+ assert_eq!(
+ query.limit_clause,
+ Some(LimitClause::Percent { limit, offset }),
+ "{sql}",
+ );
+ }
+}
+
+#[test]
+fn test_limit_percent_expression_quantities() {
+ for sql in [
+ "SELECT 1 LIMIT ?%",
+ "SELECT 1 LIMIT $1%",
+ "SELECT 1 LIMIT -25%",
+ "SELECT 1 LIMIT +25%",
+ "SELECT 1 LIMIT (25)%",
+ "SELECT 1 LIMIT (10 + 15)%",
+ "SELECT 1 LIMIT (10 % 3)%",
+ "SELECT 1 LIMIT 10 * 5%",
+ "SELECT 1 LIMIT 10 % 3%",
+ "SELECT 1 LIMIT abs(10 % 3)%",
+ "SELECT 1 LIMIT CAST(25 AS INTEGER)%",
+ ] {
+ duckdb().verified_stmt(sql);
+ }
+}
+
+#[test]
+fn test_limit_percent_preserves_modulo() {
+ duckdb_and_generic().verified_stmt("SELECT 5 % 2");
+
+ let query = duckdb_and_generic().verified_query("SELECT 1 LIMIT 5 % 2");
+ assert_eq!(
+ query.limit_clause,
+ Some(LimitClause::LimitOffset {
+ limit: Some(Expr::BinaryOp {
+ left: Box::new(Expr::value(number("5"))),
+ op: BinaryOperator::Modulo,
+ right: Box::new(Expr::value(number("2"))),
+ }),
+ offset: None,
+ limit_by: vec![],
+ })
+ );
+
+ duckdb_and_generic().verified_stmt("SELECT 1 LIMIT (5 % 2)");
+
+ for sql in [
+ "SELECT 1 LIMIT ? % 2",
+ "SELECT 1 LIMIT $1 % 2",
+ "SELECT 1 LIMIT -25 % 2",
+ "SELECT 1 LIMIT (25) % 2",
+ "SELECT 1 LIMIT 10 + 15 % 2",
+ "SELECT 1 LIMIT CAST(25 AS INTEGER) % 2",
+ ] {
+ duckdb_and_generic().verified_stmt(sql);
+ }
+}
+
+#[test]
+fn test_limit_percent_rejects_malformed_syntax() {
+ for (sql, expected) in [
+ (
+ "SELECT 1 LIMIT %",
+ ParserError::ParserError("Expected: an expression, found:
%".to_string()),
+ ),
+ (
+ "SELECT 1 LIMIT ALL%",
+ ParserError::ParserError("Expected: end of statement, found:
%".to_string()),
+ ),
+ (
+ "SELECT 1 LIMIT 25%%",
+ ParserError::ParserError("Expected: an expression, found:
%".to_string()),
+ ),
+ (
+ "SELECT 1 LIMIT 25L%",
+ ParserError::ParserError("Expected: an expression, found:
EOF".to_string()),
+ ),
+ (
+ "SELECT 1 LIMIT 25 PERCENT",
+ ParserError::ParserError("Expected: end of statement, found:
PERCENT".to_string()),
+ ),
Review Comment:
You may want to remove also this case. DuckDB 1.4.4 accepts `SELECT 1 LIMIT
25 PERCENT` (and `LIMIT 2.5 PERCENT`, `LIMIT 5 PERCENT OFFSET 1`), so the test
asserts that valid DuckDB SQL is rejected.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
