[SPARK-23238][SQL] Externalize SQLConf configurations exposed in documentation #20403

HyukjinKwon · 2018-01-26T08:50:47Z

What changes were proposed in this pull request?

This PR proposes to expose few internal configurations found in the documentation.

Also it fixes the description for spark.sql.execution.arrow.enabled.
It's quite self-explanatory.

How was this patch tested?

N/A

HyukjinKwon · 2018-01-26T09:00:14Z

cc @ueshin and @BryanCutler, mind taking a look please?

ueshin · 2018-01-26T09:06:28Z

LGTM.

SparkQA · 2018-01-26T10:30:10Z

Test build #86693 has finished for PR 20403 at commit 4899e33.

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

HyukjinKwon · 2018-01-26T10:57:37Z

retest this please

SparkQA · 2018-01-26T13:58:31Z

Test build #86701 has finished for PR 20403 at commit 4899e33.

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

ueshin · 2018-01-26T14:10:15Z

Jenkins, retest this please.

SparkQA · 2018-01-26T16:06:28Z

Test build #86705 has finished for PR 20403 at commit 4899e33.

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

HyukjinKwon · 2018-01-26T16:16:17Z

retest this please

gatorsmile · 2018-01-26T16:43:14Z

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

@@ -1045,9 +1045,10 @@ object SQLConf {
    buildConf("spark.sql.execution.arrow.enabled")
      .internal()
      .doc("Make use of Apache Arrow for columnar data transfers. Currently available " +


When true, make use of

gatorsmile · 2018-01-26T16:48:29Z

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

@@ -1045,9 +1045,10 @@ object SQLConf {
    buildConf("spark.sql.execution.arrow.enabled")
      .internal()


This conf will be mentioned in the doc #19575. I think this is not qualified as an internal conf.

Could you change it in this PR and use this JIRA number https://issues.apache.org/jira/browse/SPARK-23238

BryanCutler

LGTM

gatorsmile · 2018-01-26T17:49:47Z

LGTM

SparkQA · 2018-01-26T19:19:18Z

Test build #86712 has finished for PR 20403 at commit 4899e33.

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

SparkQA · 2018-01-26T20:26:20Z

Test build #86716 has finished for PR 20403 at commit 1f4d288.

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

BryanCutler · 2018-01-26T22:23:42Z

this has been failing due to #19892 which was recently reverted

BryanCutler · 2018-01-26T22:23:51Z

jenkins retest this please

SparkQA · 2018-01-26T22:28:47Z

Test build #86726 has started for PR 20403 at commit 1f4d288.

HyukjinKwon · 2018-01-27T02:39:06Z

retest this please

viirya · 2018-01-27T07:32:59Z

LGTM

gatorsmile · 2018-01-27T17:06:58Z

retest this please

gatorsmile · 2018-01-27T17:07:10Z

ok to test

SparkQA · 2018-01-27T21:08:03Z

Test build #86730 has finished for PR 20403 at commit 1f4d288.

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

viirya · 2018-01-27T23:04:21Z

retest this please.

SparkQA · 2018-01-28T02:32:07Z

Test build #86737 has finished for PR 20403 at commit 1f4d288.

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

viirya · 2018-01-28T02:56:08Z

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

+        "for use with pyspark.sql.DataFrame.toPandas, and " +
+        "pyspark.sql.SparkSession.createDataFrame when its input is a Pandas DataFrame. " +
+        "The following data types are unsupported: " +
+        "MapType, ArrayType of TimestampType, and nested StructType.")
      .booleanConf
      .createWithDefault(false)


spark.sql.execution.arrow.maxRecordsPerBatch is also mentioned in the doc change at #19575. Shall we also externalize it?

Yup. Let me update spark.sql.inMemoryColumnarStorage.compressed and spark.sql.inMemoryColumnarStorage.batchSize too. These are also exposed but internals.

SparkQA · 2018-01-28T07:49:31Z

Test build #86742 has finished for PR 20403 at commit 0c05526.

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

viirya · 2018-01-28T13:18:04Z

Let's also update the PR title.

gatorsmile · 2018-01-29T03:28:01Z

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

      .booleanConf
      .createWithDefault(false)

  val ARROW_EXECUTION_MAX_RECORDS_PER_BATCH =
    buildConf("spark.sql.execution.arrow.maxRecordsPerBatch")
-      .internal()


I am not sure about conf. https://github.com/apache/spark/pull/19575/files#r164252424

If we want to merge this PR now, maybe revert this change?

We can externalize this conf in that PR #19575, if we believe this conf is the one we will use in the long term.

Makes sense. Let me take this out. Is this only one you are concerned of for now?

gatorsmile · 2018-01-29T04:25:41Z

LGTM

viirya

LGTM

SparkQA · 2018-01-29T07:13:58Z

Test build #86749 has finished for PR 20403 at commit fd7f5c0.

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

…mentation ## What changes were proposed in this pull request? This PR proposes to expose few internal configurations found in the documentation. Also it fixes the description for `spark.sql.execution.arrow.enabled`. It's quite self-explanatory. ## How was this patch tested? N/A Author: hyukjinkwon <gurwls223@gmail.com> Closes #20403 from HyukjinKwon/minor-doc-arrow. (cherry picked from commit 39d2c6b) Signed-off-by: hyukjinkwon <gurwls223@gmail.com>

HyukjinKwon · 2018-01-29T12:11:47Z

Merged to master and branch-2.3.

Thanks @ueshin, @gatorsmile, @BryanCutler, @felixcheung and @viirya.

Minor doc correction for 'spark.sql.execution.arrow.enabled'

4899e33

gatorsmile reviewed Jan 26, 2018

View reviewed changes

Address comments

1f4d288

HyukjinKwon changed the title ~~[MINOR][PYTHON] Minor doc correction for 'spark.sql.execution.arrow.enabled'~~ [SPARK-23238][PYTHON] Minor doc correction for 'spark.sql.execution.arrow.enabled' Jan 26, 2018

HyukjinKwon changed the title ~~[SPARK-23238][PYTHON] Minor doc correction for 'spark.sql.execution.arrow.enabled'~~ [SPARK-23238][PYTHON] Externalize SQLConf spark.sql.execution.arrow.enabled Jan 26, 2018

BryanCutler approved these changes Jan 26, 2018

View reviewed changes

felixcheung approved these changes Jan 27, 2018

View reviewed changes

viirya reviewed Jan 28, 2018

View reviewed changes

Externalize few more configurations mentioned in the documentation

0c05526

HyukjinKwon changed the title ~~[SPARK-23238][PYTHON] Externalize SQLConf spark.sql.execution.arrow.enabled~~ [SPARK-23238][SQL] Externalize SQLConf spark.sql.execution.arrow.enabled Jan 28, 2018

HyukjinKwon changed the title ~~[SPARK-23238][SQL] Externalize SQLConf spark.sql.execution.arrow.enabled~~ [SPARK-23238][SQL] Externalize SQLConf configurations exposed in documentation Jan 28, 2018

gatorsmile reviewed Jan 29, 2018

View reviewed changes

Take out 'spark.sql.execution.arrow.maxRecordsPerBatch' for now

fd7f5c0

viirya approved these changes Jan 29, 2018

View reviewed changes

asfgit closed this in 39d2c6b Jan 29, 2018

HyukjinKwon deleted the minor-doc-arrow branch October 16, 2018 12:44

		@@ -1045,9 +1045,10 @@ object SQLConf {
		buildConf("spark.sql.execution.arrow.enabled")
		.internal()

[SPARK-23238][SQL] Externalize SQLConf configurations exposed in documentation #20403

[SPARK-23238][SQL] Externalize SQLConf configurations exposed in documentation #20403

Uh oh!

Conversation

HyukjinKwon commented Jan 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

HyukjinKwon commented Jan 26, 2018

Uh oh!

ueshin commented Jan 26, 2018

Uh oh!

SparkQA commented Jan 26, 2018

Uh oh!

HyukjinKwon commented Jan 26, 2018

Uh oh!

SparkQA commented Jan 26, 2018

Uh oh!

ueshin commented Jan 26, 2018

Uh oh!

SparkQA commented Jan 26, 2018

Uh oh!

HyukjinKwon commented Jan 26, 2018

Uh oh!

gatorsmile Jan 26, 2018

Choose a reason for hiding this comment

Uh oh!

gatorsmile Jan 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gatorsmile Jan 26, 2018

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon Jan 26, 2018

Choose a reason for hiding this comment

Uh oh!

BryanCutler left a comment

Choose a reason for hiding this comment

Uh oh!

gatorsmile commented Jan 26, 2018

Uh oh!

SparkQA commented Jan 26, 2018

Uh oh!

SparkQA commented Jan 26, 2018

Uh oh!

BryanCutler commented Jan 26, 2018

Uh oh!

BryanCutler commented Jan 26, 2018

Uh oh!

SparkQA commented Jan 26, 2018

Uh oh!

HyukjinKwon commented Jan 27, 2018

Uh oh!

viirya commented Jan 27, 2018

Uh oh!

gatorsmile commented Jan 27, 2018

Uh oh!

gatorsmile commented Jan 27, 2018

Uh oh!

SparkQA commented Jan 27, 2018

Uh oh!

viirya commented Jan 27, 2018

Uh oh!

SparkQA commented Jan 28, 2018

Uh oh!

viirya Jan 28, 2018

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon Jan 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jan 28, 2018

Uh oh!

viirya commented Jan 28, 2018

Uh oh!

gatorsmile Jan 29, 2018

Choose a reason for hiding this comment

HyukjinKwon commented Jan 26, 2018 •

edited

Loading

gatorsmile Jan 26, 2018 •

edited

Loading

HyukjinKwon Jan 28, 2018 •

edited

Loading

HyukjinKwon commented Jan 29, 2018 •

edited

Loading