[SPARK-49558][SQL] Add SQL pipe syntax for LIMIT/OFFSET and ORDER/SORT/CLUSTER/DISTRIBUTE BY #48413

dtenedor · 2024-10-10T16:47:07Z

What changes were proposed in this pull request?

This PR adds SQL pipe syntax support for LIMIT/OFFSET and ORDER/SORT/CLUSTER/DISTRIBUTE BY.

For example:

CREATE TABLE t(x INT, y STRING) USING CSV;
INSERT INTO t VALUES (0, 'abc'), (1, 'def');

TABLE t
|> ORDER BY x
|> LIMIT 1 OFFSET 1

1	def

Why are the changes needed?

The SQL pipe operator syntax will let users compose queries in a more flexible fashion.

Does this PR introduce any user-facing change?

Yes, see above.

How was this patch tested?

This PR adds a few unit test cases, but mostly relies on golden file test coverage. I did this to make sure the answers are correct as this feature is implemented and also so we can look at the analyzer output plans to ensure they look right as well.

Was this patch authored or co-authored using generative AI tooling?

No

dtenedor · 2024-10-10T16:48:30Z

cc @cloud-fan @gengliangwang this is the PR to support LIMIT/OFFSET + sorting. There are a few more changes in the AstBuilder for this one but still contained only in the parser.

sql/core/src/test/resources/sql-tests/inputs/pipe-operators.sql

sql/core/src/test/resources/sql-tests/results/pipe-operators.sql.out

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

sql/core/src/test/resources/sql-tests/results/pipe-operators.sql.out

dtenedor

Thanks @cloud-fan for your review!

sql/core/src/test/resources/sql-tests/inputs/pipe-operators.sql

sql/core/src/test/resources/sql-tests/results/pipe-operators.sql.out

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

sql/core/src/test/resources/sql-tests/results/pipe-operators.sql.out

cloud-fan · 2024-10-14T09:58:45Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

-      throw QueryParsingErrors.combinationQueryResultClausesUnsupportedError(ctx)
+      val allClauses = "ORDER BY/SORT BY/DISTRIBUTE BY/CLUSTER BY"
+      if (forPipeOperators) {
+        throw QueryParsingErrors.clausesWithPipeOperatorsUnsupportedError(ctx, allClauses)


We should always call combinationQueryResultClausesUnsupportedError here, as this is not related to pipe. We don't support this combination in both pipe and classic SQL.

Sounds good, this is done.

cloud-fan · 2024-10-14T10:00:43Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

+      val limitClause = "LIMIT"
+      if (forPipeOperators && clause.nonEmpty && clause != offsetClause) {
+        throw QueryParsingErrors.clausesWithPipeOperatorsUnsupportedError(
+          ctx, s"the $clause and $limitClause clauses")


Suggested change

ctx, s"the $clause and $limitClause clauses")

ctx, s"the co-existence of $clause and $limitClause clauses")

Sounds good, this is done.

cloud-fan · 2024-10-14T10:00:56Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

    val withOffset = withWindow.optional(offset) {
+      if (forPipeOperators && clause.nonEmpty) {
+        throw QueryParsingErrors.clausesWithPipeOperatorsUnsupportedError(
+          ctx, s"the $clause and $offsetClause clauses")


Suggested change

ctx, s"the $clause and $offsetClause clauses")

ctx, s"the co-existence of $clause and $offsetClause clauses")

Sounds good, this is done.

common/utils/src/main/resources/error/error-conditions.json

cloud-fan · 2024-10-14T10:05:50Z

common/utils/src/main/resources/error/error-conditions.json

@@ -4926,9 +4926,14 @@
          "Catalog <catalogName> does not support <operation>."
        ]
      },
+      "CLAUSE_WITH_PIPE_OPERATORS" : {


Maybe @srielau can also comment here, but it looks to me that we have two new errors:

WINDOW in the SQL pipe operator. We don't support window definition in SQL pipe yet.

More than one QUERY_RESULT_CLAUSES in SQL pipe. SQL pipe is designed to specify one operator at a time, so we don't allow any combination, such as ORDER BY col LIMIT 1, or LIMIT 1 OFFSET 1

appears to be a 0A000 (unsupported feature)

Isn't that a syntax error?

yes, 2 is a syntax error for the pipe statement.

Thanks, split this into two new errors per suggestion.

respond to code review comments respond to code review comments respond to code review comments

dtenedor

Thanks @cloud-fan for your reviews!

dtenedor · 2024-10-16T20:25:35Z

common/utils/src/main/resources/error/error-conditions.json

@@ -4926,9 +4926,14 @@
          "Catalog <catalogName> does not support <operation>."
        ]
      },
+      "CLAUSE_WITH_PIPE_OPERATORS" : {


Thanks, split this into two new errors per suggestion.

gengliangwang · 2024-10-16T20:43:28Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala


    // OFFSET
    // - OFFSET 0 is the same as omitting the OFFSET clause
+    val offsetClause = "OFFSET"


nit: shall we put all the constant clauses into an object?

Sure, it is good to keep magic strings out of the code.

cloud-fan · 2024-10-17T01:24:23Z

common/utils/src/main/resources/error/error-conditions.json

@@ -5006,6 +5011,11 @@
          "Multiple bucket TRANSFORMs."
        ]
      },
+      "MULTIPLE_QUERY_RESULT_CLAUSES_WITH_PIPE_OPERATORS" : {


shall we turn it into a top-level error class? This is syntax error, not unsupported feature, and should have a different SQL state.

cloud-fan · 2024-10-17T01:26:54Z

common/utils/src/main/resources/error/error-conditions.json

@@ -5006,6 +5011,11 @@
          "Multiple bucket TRANSFORMs."
        ]
      },
+      "MULTIPLE_QUERY_RESULT_CLAUSES_WITH_PIPE_OPERATORS" : {
+        "message" : [
+          "Syntax error: the SQL pipe operator syntax using |> does not support <clauses>. Please separate the multiple result clauses into separate pipe operators and then retry the query again."


how about

<clause1> and <clause2> can not coexist in SQL pipe operator using '|>'. Please separate ...

The idea is to make the error parameters simple, so that people can consume it programatically.

commit

6808620

dtenedor marked this pull request as ready for review October 10, 2024 16:47

github-actions bot added the SQL label Oct 10, 2024

fix test

cd9d6f6

cloud-fan reviewed Oct 11, 2024

View reviewed changes

sql/core/src/test/resources/sql-tests/inputs/pipe-operators.sql Show resolved Hide resolved

cloud-fan reviewed Oct 11, 2024

View reviewed changes

sql/core/src/test/resources/sql-tests/results/pipe-operators.sql.out Outdated Show resolved Hide resolved

cloud-fan reviewed Oct 11, 2024

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Oct 11, 2024

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Oct 11, 2024

View reviewed changes

sql/core/src/test/resources/sql-tests/results/pipe-operators.sql.out Outdated Show resolved Hide resolved

dtenedor commented Oct 11, 2024

View reviewed changes

dtenedor requested a review from cloud-fan October 11, 2024 18:04

cloud-fan reviewed Oct 14, 2024

View reviewed changes

common/utils/src/main/resources/error/error-conditions.json Show resolved Hide resolved

cloud-fan reviewed Oct 14, 2024

View reviewed changes

respond to code review comments

d55dce0

respond to code review comments respond to code review comments respond to code review comments

dtenedor force-pushed the pipe-order-by branch from 5205e12 to d55dce0 Compare October 16, 2024 20:25

dtenedor commented Oct 16, 2024

View reviewed changes

dtenedor requested a review from cloud-fan October 16, 2024 20:25

respond to code review comments

a4584f5

gengliangwang reviewed Oct 16, 2024

View reviewed changes

gengliangwang approved these changes Oct 16, 2024

View reviewed changes

respond to code review comments

8e5dc29

cloud-fan reviewed Oct 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-49558][SQL] Add SQL pipe syntax for LIMIT/OFFSET and ORDER/SORT/CLUSTER/DISTRIBUTE BY #48413

[SPARK-49558][SQL] Add SQL pipe syntax for LIMIT/OFFSET and ORDER/SORT/CLUSTER/DISTRIBUTE BY #48413

dtenedor commented Oct 10, 2024

dtenedor commented Oct 10, 2024

dtenedor left a comment

cloud-fan Oct 14, 2024

dtenedor Oct 16, 2024

cloud-fan Oct 14, 2024

dtenedor Oct 16, 2024

cloud-fan Oct 14, 2024

dtenedor Oct 16, 2024

cloud-fan Oct 14, 2024

srielau Oct 14, 2024

cloud-fan Oct 14, 2024

dtenedor Oct 16, 2024

dtenedor left a comment

dtenedor Oct 16, 2024

gengliangwang Oct 16, 2024

dtenedor Oct 16, 2024

cloud-fan Oct 17, 2024

cloud-fan Oct 17, 2024 •

edited

Loading

cloud-fan Oct 17, 2024

	ctx, s"the $clause and $limitClause clauses")
	ctx, s"the co-existence of $clause and $limitClause clauses")

	ctx, s"the $clause and $offsetClause clauses")
	ctx, s"the co-existence of $clause and $offsetClause clauses")

[SPARK-49558][SQL] Add SQL pipe syntax for LIMIT/OFFSET and ORDER/SORT/CLUSTER/DISTRIBUTE BY #48413

Are you sure you want to change the base?

[SPARK-49558][SQL] Add SQL pipe syntax for LIMIT/OFFSET and ORDER/SORT/CLUSTER/DISTRIBUTE BY #48413

Conversation

dtenedor commented Oct 10, 2024

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

dtenedor commented Oct 10, 2024

dtenedor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtenedor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloud-fan Oct 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloud-fan Oct 17, 2024 •

edited

Loading