[SPARK-24783][SQL]spark.sql.shuffle.partitions=0 should throw exception #23835

WindCanDie · 2019-02-19T03:52:08Z

this PR add checkValue this spark.sql.shuffle.partitions>0 and sparkSqlConf test

How was this patch tested

runring test("[SPARK-24783][SQL]spark.sql.shuffle.partitions=0 should throw exception ")

When spark.sql.shuffle.partitions<=0 run test throw IllegalArgumentException and message"e value of sql.shuffle.partitions must be greater than 0"
this catch and assert exception message
spark.sql.shuffle.partitions=0 should throw exception

JIRA link

https://issues.apache.org/jira/browse/SPARK-24783?page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel&amp%3BfocusedCommentId=16771085#comment-16771085

spark.sql.shuffle.partitions=0 should throw exception

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

gatorsmile

@WindCanDie Could you add a test case for reproducing the bug that is reported in the JIRA?

srowen

Agree a little test would be good

when spark.sql.shuffle.partitions=0 error test

WindCanDie · 2019-02-19T14:30:02Z

ok add test on the bug

sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsSuite.scala

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

gatorsmile · 2019-02-19T17:52:48Z

ok to test

sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsSuite.scala

SparkQA · 2019-02-19T19:41:39Z

Test build #102513 has finished for PR 23835 at commit 0584115.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

move test on joinsuite ,

when spark.sql.shuffle.partitions=0 error test Recovery format

HeartSaVioR · 2019-02-20T03:50:35Z

Please fill the template of PR instead of rewriting it: the title and content of PR will become commit message, so Spark community requires PRs to have formulated format.

https://github.com/apache/spark/blob/master/.github/PULL_REQUEST_TEMPLATE

WindCanDie · 2019-02-20T05:09:21Z

Please fill the template of PR instead of rewriting it: the title and content of PR will become commit message, so Spark community requires PRs to have formulated format.

https://github.com/apache/spark/blob/master/.github/PULL_REQUEST_TEMPLATE

ok
##Frist this PR add spark.sql.shuffle.partitions>0 checkValue and test

How was this patch tested

runring JoinSuite. test("[SPARK-24783][SQL]spark.sql.shuffle.partitions=0 should throw exception ")
on park.sql.shuffle.partitions<=0 run test throw IllegalArgumentException and message"e value of sql.shuffle.partitions must be greater than 0"
this catch and assert exception message

SparkQA · 2019-02-20T05:27:06Z

Test build #102527 has finished for PR 23835 at commit d2c535b.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala

unindent the body by one unit

sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala

HyukjinKwon · 2019-02-21T02:17:34Z

re: #23835 (comment), please edit PR description instead of leaving it as a comment here. Take a look at https://github.com/databricks/scala-style-guide for style and https://spark.apache.org/contributing.html as well

SparkQA · 2019-02-21T02:19:16Z

Test build #102568 has finished for PR 23835 at commit 8597480.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala

mov test to sqlconfsuite

SparkQA · 2019-02-21T02:38:39Z

Test build #102569 has finished for PR 23835 at commit 18437e3.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

WindCanDie · 2019-02-21T02:41:55Z

re: #23835 (comment), please edit PR description instead of leaving it as a comment here. Take a look at https://github.com/databricks/scala-style-guide for style and https://spark.apache.org/contributing.html as well

I will read it carefully

code Scalastyle

SparkQA · 2019-02-21T07:51:55Z

Test build #102574 has finished for PR 23835 at commit 6e2a773.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

HyukjinKwon

Looks good otherwise.

srowen · 2019-02-25T10:53:37Z

Ping @WindCanDie

maropu · 2019-03-01T02:34:38Z

ping, again @WindCanDie

WindCanDie · 2019-03-01T03:05:48Z

ping, again @WindCanDie
I'm here
I don't quite understand what problems can you tell me in detail?

maropu · 2019-03-01T03:22:11Z

Have you checked @HyukjinKwon 's comments?

WindCanDie

Have you checked @HyukjinKwon 's comments?
this ?
nit: spark.sql.shuffle.partitions?
nit this is what abbreviation I did not understand

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

test title to be consisten

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

SparkQA · 2019-03-01T08:08:57Z

Test build #102896 has finished for PR 23835 at commit 2b6ca75.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

spark.sql.shuffle.partitions message

SparkQA · 2019-03-01T12:25:45Z

Test build #102903 has finished for PR 23835 at commit 1a7b15e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2019-03-01T13:29:35Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

@@ -267,6 +267,7 @@ object SQLConf {
      "Note: For structured streaming, this configuration cannot be changed between query " +
      "restarts from the same checkpoint location.")
    .intConf
+    .checkValue(_ > 0, "the value of spark.sql.shuffle.partitions must be greater than 0")


the -> The

Please read the comments closely.

srowen · 2019-03-04T14:36:21Z

Ping @WindCanDie

srowen · 2019-03-07T14:50:13Z

No follow up; I'm going to finish this at #24008

## What changes were proposed in this pull request? Throw an exception if spark.sql.shuffle.partitions=0 This takes over #23835 ## How was this patch tested? Existing tests. Closes #24008 from srowen/SPARK-24783.2. Lead-authored-by: Sean Owen <sean.owen@databricks.com> Co-authored-by: WindCanDie <491237260@qq.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

SPARK-24783

b5dc305

spark.sql.shuffle.partitions=0 should throw exception

liupc reviewed Feb 19, 2019

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Outdated Show resolved Hide resolved

gatorsmile requested changes Feb 19, 2019

View reviewed changes

srowen approved these changes Feb 19, 2019

View reviewed changes

srowen reviewed Feb 19, 2019

View reviewed changes

SPARK-24783

0584115

when spark.sql.shuffle.partitions=0 error test

srowen requested changes Feb 19, 2019

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsSuite.scala Outdated Show resolved Hide resolved

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Outdated Show resolved Hide resolved

gatorsmile reviewed Feb 19, 2019

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsSuite.scala Outdated Show resolved Hide resolved

gatorsmile reviewed Feb 19, 2019

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsSuite.scala Outdated Show resolved Hide resolved

gatorsmile reviewed Feb 19, 2019

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsSuite.scala Outdated Show resolved Hide resolved

langjx added 2 commits February 20, 2019 10:09

SPARK-24783

7efe0a7

move test on joinsuite ,

SPARK-24783

d2c535b

when spark.sql.shuffle.partitions=0 error test Recovery format

srowen reviewed Feb 20, 2019

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala Outdated Show resolved Hide resolved

##SPARK-24783 when spark.sql.shuffle.partitions=0

8597480

unindent the body by one unit

HyukjinKwon reviewed Feb 21, 2019

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala Outdated Show resolved Hide resolved

HyukjinKwon reviewed Feb 21, 2019

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/JoinSuite.scala Outdated Show resolved Hide resolved

[SPARK-24783]when spark.sql.shuffle.partitions=0

18437e3

mov test to sqlconfsuite

[SPARK-24783]when spark.sql.shuffle.partitions=0

6e2a773

code Scalastyle

HyukjinKwon reviewed Feb 21, 2019

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala Outdated Show resolved Hide resolved

HyukjinKwon reviewed Feb 21, 2019

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Outdated Show resolved Hide resolved

HyukjinKwon approved these changes Feb 21, 2019

View reviewed changes

WindCanDie commented Mar 1, 2019

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Outdated Show resolved Hide resolved

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Outdated Show resolved Hide resolved

[SPARK-24783]when spark.sql.shuffle.partitions=0

2b6ca75

test title to be consisten

HyukjinKwon reviewed Mar 1, 2019

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Outdated Show resolved Hide resolved

[SPARK-24783]when spark.sql.shuffle.partitions=0

1a7b15e

spark.sql.shuffle.partitions message

HyukjinKwon reviewed Mar 1, 2019

View reviewed changes

srowen mentioned this pull request Mar 7, 2019

[SPARK-24783][SQL] spark.sql.shuffle.partitions=0 should throw exception #24008

Closed

srowen closed this Mar 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-24783][SQL]spark.sql.shuffle.partitions=0 should throw exception #23835

[SPARK-24783][SQL]spark.sql.shuffle.partitions=0 should throw exception #23835

WindCanDie commented Feb 19, 2019 •

edited

Loading

gatorsmile left a comment

srowen left a comment

WindCanDie commented Feb 19, 2019

gatorsmile commented Feb 19, 2019

SparkQA commented Feb 19, 2019

HeartSaVioR commented Feb 20, 2019

WindCanDie commented Feb 20, 2019

SparkQA commented Feb 20, 2019

HyukjinKwon commented Feb 21, 2019

SparkQA commented Feb 21, 2019

SparkQA commented Feb 21, 2019

WindCanDie commented Feb 21, 2019

SparkQA commented Feb 21, 2019

HyukjinKwon left a comment

srowen commented Feb 25, 2019

maropu commented Mar 1, 2019

WindCanDie commented Mar 1, 2019 •

edited

Loading

maropu commented Mar 1, 2019

WindCanDie left a comment

SparkQA commented Mar 1, 2019

SparkQA commented Mar 1, 2019

HyukjinKwon Mar 1, 2019

srowen commented Mar 4, 2019

srowen commented Mar 7, 2019

[SPARK-24783][SQL]spark.sql.shuffle.partitions=0 should throw exception #23835

[SPARK-24783][SQL]spark.sql.shuffle.partitions=0 should throw exception #23835

Conversation

WindCanDie commented Feb 19, 2019 • edited Loading

this PR add checkValue this spark.sql.shuffle.partitions>0 and sparkSqlConf test

How was this patch tested

JIRA link

gatorsmile left a comment

Choose a reason for hiding this comment

srowen left a comment

Choose a reason for hiding this comment

WindCanDie commented Feb 19, 2019

gatorsmile commented Feb 19, 2019

SparkQA commented Feb 19, 2019

HeartSaVioR commented Feb 20, 2019

WindCanDie commented Feb 20, 2019

How was this patch tested

SparkQA commented Feb 20, 2019

HyukjinKwon commented Feb 21, 2019

SparkQA commented Feb 21, 2019

SparkQA commented Feb 21, 2019

WindCanDie commented Feb 21, 2019

SparkQA commented Feb 21, 2019

HyukjinKwon left a comment

Choose a reason for hiding this comment

srowen commented Feb 25, 2019

maropu commented Mar 1, 2019

WindCanDie commented Mar 1, 2019 • edited Loading

maropu commented Mar 1, 2019

WindCanDie left a comment

Choose a reason for hiding this comment

SparkQA commented Mar 1, 2019

SparkQA commented Mar 1, 2019

HyukjinKwon Mar 1, 2019

Choose a reason for hiding this comment

srowen commented Mar 4, 2019

srowen commented Mar 7, 2019

WindCanDie commented Feb 19, 2019 •

edited

Loading

WindCanDie commented Mar 1, 2019 •

edited

Loading