This is an automated email from the ASF dual-hosted git repository.
ulyssesyou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kyuubi.git
The following commit(s) were added to refs/heads/master by this push:
new ec232c18b [KYUUBI #6551] Allow insert zorder when global sort is false
and the plan is Repartition or RepartitionByExpression.
ec232c18b is described below
commit ec232c18b5700bca361d45770a8fd3c6117c6f48
Author: huangxiaoping <[email protected]>
AuthorDate: Tue Jul 23 09:36:21 2024 +0800
[KYUUBI #6551] Allow insert zorder when global sort is false and the plan
is Repartition or RepartitionByExpression.
# :mag: Description
## Issue References ๐
This pull request fixes #6551
## Describe Your Solution ๐ง
Update `canInsertZorder` to allow insert zorder when global sort is `false`
and the plan is `Repartition` or `RepartitionByExpression`.
## Types of changes :bookmark:
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
## Test Plan ๐งช
#### Behavior Without This Pull Request :coffin:
#### Behavior With This Pull Request :tada:
#### Related Unit Tests
/kyuubi-extension-spark-common/src/test/scala/org/apache/spark/sql/ZorderSuiteBase.scala
---
# Checklist ๐
- [x] This patch was not authored or co-authored using [Generative
Tooling](https://www.apache.org/legal/generative-tooling.html)
**Be nice. Be informative.**
Closes #6552 from huangxiaopingRD/6551.
Closes #6551
b597443c3 [huangxiaoping] Fix code style
618594667 [huangxiaoping] [KYUUBI #6551] Allow insert zorder when when the
plan is Repartition or RepartitionByExpression
Authored-by: huangxiaoping <[email protected]>
Signed-off-by: ulyssesyou <[email protected]>
---
.../sql/zorder/InsertZorderBeforeWriting33.scala | 2 ++
.../org/apache/spark/sql/ZorderSuiteBase.scala | 25 ++++++++++++++++++++++
.../sql/zorder/InsertZorderBeforeWriting.scala | 2 ++
.../org/apache/spark/sql/ZorderSuiteBase.scala | 25 ++++++++++++++++++++++
.../sql/zorder/InsertZorderBeforeWriting.scala | 2 ++
.../org/apache/spark/sql/ZorderSuiteBase.scala | 25 ++++++++++++++++++++++
6 files changed, 81 insertions(+)
diff --git
a/extensions/spark/kyuubi-extension-spark-3-3/src/main/scala/org/apache/kyuubi/sql/zorder/InsertZorderBeforeWriting33.scala
b/extensions/spark/kyuubi-extension-spark-3-3/src/main/scala/org/apache/kyuubi/sql/zorder/InsertZorderBeforeWriting33.scala
index 083c80afd..4ae2057dc 100644
---
a/extensions/spark/kyuubi-extension-spark-3-3/src/main/scala/org/apache/kyuubi/sql/zorder/InsertZorderBeforeWriting33.scala
+++
b/extensions/spark/kyuubi-extension-spark-3-3/src/main/scala/org/apache/kyuubi/sql/zorder/InsertZorderBeforeWriting33.scala
@@ -46,6 +46,8 @@ trait InsertZorderHelper33 extends Rule[LogicalPlan] with
ZorderBuilder {
def canInsertZorder(query: LogicalPlan): Boolean = query match {
case Project(_, child) => canInsertZorder(child)
+ case _: RepartitionByExpression | _: Repartition
+ if !conf.getConf(KyuubiSQLConf.ZORDER_GLOBAL_SORT_ENABLED) => true
// TODO: actually, we can force zorder even if existed some shuffle
case _: Sort => false
case _: RepartitionByExpression => false
diff --git
a/extensions/spark/kyuubi-extension-spark-3-3/src/test/scala/org/apache/spark/sql/ZorderSuiteBase.scala
b/extensions/spark/kyuubi-extension-spark-3-3/src/test/scala/org/apache/spark/sql/ZorderSuiteBase.scala
index 476402fba..82ca8d4d2 100644
---
a/extensions/spark/kyuubi-extension-spark-3-3/src/test/scala/org/apache/spark/sql/ZorderSuiteBase.scala
+++
b/extensions/spark/kyuubi-extension-spark-3-3/src/test/scala/org/apache/spark/sql/ZorderSuiteBase.scala
@@ -639,6 +639,31 @@ trait ZorderSuiteBase extends KyuubiSparkSQLExtensionTest
with ExpressionEvalHel
}
}
+ test("Allow insert zorder after repartition if zorder using local sort") {
+ withTable("t") {
+ sql(
+ """
+ |CREATE TABLE t (c1 int, c2 string) TBLPROPERTIES (
+ |'kyuubi.zorder.enabled'= 'true',
+ |'kyuubi.zorder.cols'= 'c1,c2')
+ |""".stripMargin)
+ withSQLConf(KyuubiSQLConf.ZORDER_GLOBAL_SORT_ENABLED.key -> "false") {
+ val p1 = sql("INSERT INTO TABLE t SELECT /*+ REPARTITION(1) */* FROM
VALUES(1,'a')")
+ .queryExecution.analyzed
+ assert(p1.collect {
+ case sort: Sort if !sort.global => sort
+ }.size == 1)
+ }
+ withSQLConf(KyuubiSQLConf.ZORDER_GLOBAL_SORT_ENABLED.key -> "true") {
+ val p2 = sql("INSERT INTO TABLE t SELECT /*+ REPARTITION(1) */* FROM
VALUES(1,'a')")
+ .queryExecution.analyzed
+ assert(p2.collect {
+ case sort: Sort if !sort.global => sort
+ }.size == 0)
+ }
+ }
+ }
+
test("fast approach test") {
Seq[Seq[Any]](
Seq(1L, 2L),
diff --git
a/extensions/spark/kyuubi-extension-spark-3-4/src/main/scala/org/apache/kyuubi/sql/zorder/InsertZorderBeforeWriting.scala
b/extensions/spark/kyuubi-extension-spark-3-4/src/main/scala/org/apache/kyuubi/sql/zorder/InsertZorderBeforeWriting.scala
index b3f98ec6d..73ed5e253 100644
---
a/extensions/spark/kyuubi-extension-spark-3-4/src/main/scala/org/apache/kyuubi/sql/zorder/InsertZorderBeforeWriting.scala
+++
b/extensions/spark/kyuubi-extension-spark-3-4/src/main/scala/org/apache/kyuubi/sql/zorder/InsertZorderBeforeWriting.scala
@@ -45,6 +45,8 @@ trait InsertZorderHelper33 extends Rule[LogicalPlan] with
ZorderBuilder {
def canInsertZorder(query: LogicalPlan): Boolean = query match {
case Project(_, child) => canInsertZorder(child)
+ case _: RepartitionByExpression | _: Repartition
+ if !conf.getConf(KyuubiSQLConf.ZORDER_GLOBAL_SORT_ENABLED) => true
// TODO: actually, we can force zorder even if existed some shuffle
case _: Sort => false
case _: RepartitionByExpression => false
diff --git
a/extensions/spark/kyuubi-extension-spark-3-4/src/test/scala/org/apache/spark/sql/ZorderSuiteBase.scala
b/extensions/spark/kyuubi-extension-spark-3-4/src/test/scala/org/apache/spark/sql/ZorderSuiteBase.scala
index 2d3eec957..26c8ebd1d 100644
---
a/extensions/spark/kyuubi-extension-spark-3-4/src/test/scala/org/apache/spark/sql/ZorderSuiteBase.scala
+++
b/extensions/spark/kyuubi-extension-spark-3-4/src/test/scala/org/apache/spark/sql/ZorderSuiteBase.scala
@@ -640,6 +640,31 @@ trait ZorderSuiteBase extends KyuubiSparkSQLExtensionTest
with ExpressionEvalHel
}
}
+ test("Allow insert zorder after repartition if zorder using local sort") {
+ withTable("t") {
+ sql(
+ """
+ |CREATE TABLE t (c1 int, c2 string) TBLPROPERTIES (
+ |'kyuubi.zorder.enabled'= 'true',
+ |'kyuubi.zorder.cols'= 'c1,c2')
+ |""".stripMargin)
+ withSQLConf(KyuubiSQLConf.ZORDER_GLOBAL_SORT_ENABLED.key -> "false") {
+ val p1 = sql("INSERT INTO TABLE t SELECT /*+ REPARTITION(1) */* FROM
VALUES(1,'a')")
+ .queryExecution.analyzed
+ assert(p1.collect {
+ case sort: Sort if !sort.global => sort
+ }.size == 1)
+ }
+ withSQLConf(KyuubiSQLConf.ZORDER_GLOBAL_SORT_ENABLED.key -> "true") {
+ val p2 = sql("INSERT INTO TABLE t SELECT /*+ REPARTITION(1) */* FROM
VALUES(1,'a')")
+ .queryExecution.analyzed
+ assert(p2.collect {
+ case sort: Sort if !sort.global => sort
+ }.size == 0)
+ }
+ }
+ }
+
test("fast approach test") {
Seq[Seq[Any]](
Seq(1L, 2L),
diff --git
a/extensions/spark/kyuubi-extension-spark-3-5/src/main/scala/org/apache/kyuubi/sql/zorder/InsertZorderBeforeWriting.scala
b/extensions/spark/kyuubi-extension-spark-3-5/src/main/scala/org/apache/kyuubi/sql/zorder/InsertZorderBeforeWriting.scala
index b3f98ec6d..73ed5e253 100644
---
a/extensions/spark/kyuubi-extension-spark-3-5/src/main/scala/org/apache/kyuubi/sql/zorder/InsertZorderBeforeWriting.scala
+++
b/extensions/spark/kyuubi-extension-spark-3-5/src/main/scala/org/apache/kyuubi/sql/zorder/InsertZorderBeforeWriting.scala
@@ -45,6 +45,8 @@ trait InsertZorderHelper33 extends Rule[LogicalPlan] with
ZorderBuilder {
def canInsertZorder(query: LogicalPlan): Boolean = query match {
case Project(_, child) => canInsertZorder(child)
+ case _: RepartitionByExpression | _: Repartition
+ if !conf.getConf(KyuubiSQLConf.ZORDER_GLOBAL_SORT_ENABLED) => true
// TODO: actually, we can force zorder even if existed some shuffle
case _: Sort => false
case _: RepartitionByExpression => false
diff --git
a/extensions/spark/kyuubi-extension-spark-3-5/src/test/scala/org/apache/spark/sql/ZorderSuiteBase.scala
b/extensions/spark/kyuubi-extension-spark-3-5/src/test/scala/org/apache/spark/sql/ZorderSuiteBase.scala
index 2d3eec957..26c8ebd1d 100644
---
a/extensions/spark/kyuubi-extension-spark-3-5/src/test/scala/org/apache/spark/sql/ZorderSuiteBase.scala
+++
b/extensions/spark/kyuubi-extension-spark-3-5/src/test/scala/org/apache/spark/sql/ZorderSuiteBase.scala
@@ -640,6 +640,31 @@ trait ZorderSuiteBase extends KyuubiSparkSQLExtensionTest
with ExpressionEvalHel
}
}
+ test("Allow insert zorder after repartition if zorder using local sort") {
+ withTable("t") {
+ sql(
+ """
+ |CREATE TABLE t (c1 int, c2 string) TBLPROPERTIES (
+ |'kyuubi.zorder.enabled'= 'true',
+ |'kyuubi.zorder.cols'= 'c1,c2')
+ |""".stripMargin)
+ withSQLConf(KyuubiSQLConf.ZORDER_GLOBAL_SORT_ENABLED.key -> "false") {
+ val p1 = sql("INSERT INTO TABLE t SELECT /*+ REPARTITION(1) */* FROM
VALUES(1,'a')")
+ .queryExecution.analyzed
+ assert(p1.collect {
+ case sort: Sort if !sort.global => sort
+ }.size == 1)
+ }
+ withSQLConf(KyuubiSQLConf.ZORDER_GLOBAL_SORT_ENABLED.key -> "true") {
+ val p2 = sql("INSERT INTO TABLE t SELECT /*+ REPARTITION(1) */* FROM
VALUES(1,'a')")
+ .queryExecution.analyzed
+ assert(p2.collect {
+ case sort: Sort if !sort.global => sort
+ }.size == 0)
+ }
+ }
+ }
+
test("fast approach test") {
Seq[Seq[Any]](
Seq(1L, 2L),
