Skip to content

[SPARK-17982][SQL] SQLBuilder should wrap the generated SQL with parenthesis for LIMIT #15546

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.

Sign up for GitHub

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

Closed
dongjoon-hyun wants to merge 2 commits into from
Closed

[SPARK-17982][SQL] SQLBuilder should wrap the generated SQL with parenthesis for LIMIT #15546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e76a675
[SPARK-17982][SQL] SQLBuilder should wrap the generated SQL with pare…
dongjoon-hyun Oct 19, 2016
604414e
Add comments.
dongjoon-hyun Nov 10, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion sql/core/src/main/scala/org/apache/spark/sql/catalyst/SQLBuilder.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,14 @@ class SQLBuilder private (
case g: Generate =>
generateToSQL(g)

case Limit(limitExpr, child) =>
// This prevents a pattern of `((...) AS gen_subquery_0 LIMIT 1)` which does not work.
// For example, `SELECT * FROM (SELECT id FROM tbl TABLESAMPLE (2 ROWS))` makes this plan.
case Limit(limitExpr, child: SubqueryAlias) =>
s"${toSQL(child)} LIMIT ${limitExpr.sql}"

case Limit(limitExpr, child) =>
s"(${toSQL(child)} LIMIT ${limitExpr.sql})"
Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Nov 2, 2016

Choose a reason for hiding this comment

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

For all child logical plan except SubqueryAlias, () is used.


case Filter(condition, child) =>
val whereOrHaving = child match {
case _: Aggregate => "HAVING"
Expand Down
2 changes: 1 addition & 1 deletion sql/hive/src/test/resources/sqlgen/generate_with_other_1.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ WHERE id > 2
ORDER BY val, id
LIMIT 5
--------------------------------------------------------------------------------
SELECT `gen_attr_0` AS `val`, `gen_attr_1` AS `id` FROM (SELECT `gen_attr_0`, `gen_attr_1` FROM (SELECT gen_subquery_0.`gen_attr_2`, gen_subquery_0.`gen_attr_3`, gen_subquery_0.`gen_attr_4`, gen_subquery_0.`gen_attr_1` FROM (SELECT `arr` AS `gen_attr_2`, `arr2` AS `gen_attr_3`, `json` AS `gen_attr_4`, `id` AS `gen_attr_1` FROM `default`.`parquet_t3`) AS gen_subquery_0 WHERE (`gen_attr_1` > CAST(2 AS BIGINT))) AS gen_subquery_1 LATERAL VIEW explode(`gen_attr_2`) gen_subquery_2 AS `gen_attr_0` ORDER BY `gen_attr_0` ASC NULLS FIRST, `gen_attr_1` ASC NULLS FIRST LIMIT 5) AS parquet_t3
SELECT `gen_attr_0` AS `val`, `gen_attr_1` AS `id` FROM ((SELECT `gen_attr_0`, `gen_attr_1` FROM (SELECT gen_subquery_0.`gen_attr_2`, gen_subquery_0.`gen_attr_3`, gen_subquery_0.`gen_attr_4`, gen_subquery_0.`gen_attr_1` FROM (SELECT `arr` AS `gen_attr_2`, `arr2` AS `gen_attr_3`, `json` AS `gen_attr_4`, `id` AS `gen_attr_1` FROM `default`.`parquet_t3`) AS gen_subquery_0 WHERE (`gen_attr_1` > CAST(2 AS BIGINT))) AS gen_subquery_1 LATERAL VIEW explode(`gen_attr_2`) gen_subquery_2 AS `gen_attr_0` ORDER BY `gen_attr_0` ASC NULLS FIRST, `gen_attr_1` ASC NULLS FIRST LIMIT 5)) AS parquet_t3
2 changes: 1 addition & 1 deletion sql/hive/src/test/resources/sqlgen/generate_with_other_2.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ WHERE val > 2
ORDER BY val, id
LIMIT 5
--------------------------------------------------------------------------------
SELECT `gen_attr_0` AS `val`, `gen_attr_1` AS `id` FROM (SELECT `gen_attr_0`, `gen_attr_1` FROM (SELECT `arr` AS `gen_attr_4`, `arr2` AS `gen_attr_3`, `json` AS `gen_attr_5`, `id` AS `gen_attr_1` FROM `default`.`parquet_t3`) AS gen_subquery_0 LATERAL VIEW explode(`gen_attr_3`) gen_subquery_2 AS `gen_attr_2` LATERAL VIEW explode(`gen_attr_2`) gen_subquery_3 AS `gen_attr_0` WHERE (`gen_attr_0` > CAST(2 AS BIGINT)) ORDER BY `gen_attr_0` ASC NULLS FIRST, `gen_attr_1` ASC NULLS FIRST LIMIT 5) AS gen_subquery_1
SELECT `gen_attr_0` AS `val`, `gen_attr_1` AS `id` FROM ((SELECT `gen_attr_0`, `gen_attr_1` FROM (SELECT `arr` AS `gen_attr_4`, `arr2` AS `gen_attr_3`, `json` AS `gen_attr_5`, `id` AS `gen_attr_1` FROM `default`.`parquet_t3`) AS gen_subquery_0 LATERAL VIEW explode(`gen_attr_3`) gen_subquery_2 AS `gen_attr_2` LATERAL VIEW explode(`gen_attr_2`) gen_subquery_3 AS `gen_attr_0` WHERE (`gen_attr_0` > CAST(2 AS BIGINT)) ORDER BY `gen_attr_0` ASC NULLS FIRST, `gen_attr_1` ASC NULLS FIRST LIMIT 5)) AS gen_subquery_1
4 changes: 4 additions & 0 deletions sql/hive/src/test/resources/sqlgen/limit.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-- This file is automatically generated by LogicalPlanToSQLSuite.
SELECT * FROM (SELECT id FROM tbl LIMIT 2)
--------------------------------------------------------------------------------
Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Oct 19, 2016

Choose a reason for hiding this comment

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

Without this PR, this test case fails because it generate the following query. Note that the third line.

SELECT `gen_attr_0` AS `id`
FROM (SELECT `gen_attr_0`
             FROM SELECT `gen_attr_0`
                         FROM (SELECT `id` AS `gen_attr_0`, `name` AS `gen_attr_1`
                                     FROM `default`.`tbl`) AS gen_subquery_0 LIMIT 2) AS tbl

SELECT `gen_attr_0` AS `id` FROM (SELECT `gen_attr_0` FROM (SELECT `gen_attr_0` FROM (SELECT `id` AS `gen_attr_0`, `name` AS `gen_attr_1` FROM `default`.`tbl`) AS gen_subquery_0 LIMIT 2)) AS tbl
10 changes: 10 additions & 0 deletions sql/hive/src/test/scala/org/apache/spark/sql/catalyst/LogicalPlanToSQLSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1173,4 +1173,14 @@ class LogicalPlanToSQLSuite extends SQLBuilderTest with SQLTestUtils {
)
}
}

test("SPARK-17982 - limit") {
withTable("tbl") {
sql("CREATE TABLE tbl(id INT, name STRING)")
checkSQL(
"SELECT * FROM (SELECT id FROM tbl LIMIT 2)",
"limit"
)
}
}
}