[SPARK-7961][SQL]Refactor SQLConf to display better error message #6747

zsxwing · 2015-06-10T14:59:19Z

Add SQLConfEntry to store the information about a configuration. For those configurations that cannot be found in sql-programming-guide.md, I left the doc as <TODO>.
Verify the value when setting a configuration if this is in SQLConf.
Use SET -v to display all public configurations.

zsxwing · 2015-06-10T15:08:17Z

A simple example of the conf command:

scala> sqlContext.sql("conf").collect().foreach(v => println(v.toSeq.mkString("\t")))
spark.sql.inMemoryColumnarStorage.batchSize 10000   Controls the size of batches for columnar caching.  Larger batch sizes can improve memory utilization and compression, but risk OOMs when caching data.
spark.sql.hive.verifyPartitionPath  true    <TODO>
spark.sql.retainGroupColumns    true    <TODO>
spark.sql.codegen   false   When true, code will be dynamically generated at runtime for expression evaluation in a specific query. For some queries with complicated expression this option can lead to significant speed-ups. However, for simple queries this can actually slow down query execution.
spark.sql.planner.sortMergeJoin false   <TODO>
spark.sql.shuffle.partitions    200 Configures the number of partitions to use when shuffling data for joins or aggregations.
spark.sql.planner.externalSort  false   When true, performs sorts spilling to disk as needed otherwise sort each partition in memory.
spark.sql.broadcastTimeout  300 <TODO>
spark.sql.parquet.filterPushdown    false   Turn on Parquet filter pushdown optimization. This feature is turned off by default because of a known bug in Paruet 1.6.0rc3 (<a href="https://issues.apache.org/jira/browse/PARQUET-136">PARQUET-136</a>). However, if your table doesn't contain any nullable string or binary columns, it's still safe to turn this feature on.
spark.sql.parquet.cacheMetadata true    Turns on caching of Parquet schema metadata. Can speed up querying of static data.
spark.sql.unsafe.enabled    false   <TDDO>
spark.sql.useSerializer2    true    <TODO>
...

1. Add `SQLConfEntry` to store the information about a configuration. For those configurations that cannot be found in `sql-programming-guide.md`, I left the doc as `<TODO>`. 2. Verify the value when setting a configuration if this is in SQLConf. 3. Add a command `conf` to display all public configurations.

SparkQA · 2015-06-10T16:11:01Z

Test build #34606 has finished for PR 6747 at commit 326881d.

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

SparkQA · 2015-06-10T16:31:22Z

Test build #34607 has finished for PR 6747 at commit f3c1b33.

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

rxin · 2015-06-10T17:52:58Z

sql/core/src/main/scala/org/apache/spark/sql/SQLConf.scala

maybe remove the float version, and just use double version?

SparkQA · 2015-06-11T09:27:41Z

Test build #34678 has finished for PR 6747 at commit 99c9c16.

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

SparkQA · 2015-06-11T09:50:07Z

Test build #34680 has finished for PR 6747 at commit 8973ced.

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

SparkQA · 2015-06-11T11:42:10Z

Test build #34685 has finished for PR 6747 at commit 6e47e56.

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

SparkQA · 2015-06-11T15:44:05Z

Test build #34690 has finished for PR 6747 at commit 4abd807.

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

zsxwing · 2015-06-12T00:17:42Z

Addressed all comments

marmbrus · 2015-06-12T02:26:02Z

sql/core/src/main/scala/org/apache/spark/sql/SQLConf.scala

Can we add scaladoc for the semantics of these options. In particular, isPublic isn't obvious to me.

Added scaladoc for SQLConfEntry

marmbrus · 2015-06-12T02:54:10Z

Thanks for working on this much needed cleanup! A few comments (some of which were preexisting):

The schema for set commands is wrong (we need some column name)

scala> sql("SET -v")
res4: org.apache.spark.sql.DataFrame = [: string]

Related to the above, I think we should consider breaking out the parts of commands like SET and SET -v into columns, instead of just returning a concatenated string.
- When the option has a default, I think we should show that instead of <undefined>

scala> sql("SET spark.sql.autoBroadcastJoinThreshold").collect()
res1: Array[org.apache.spark.sql.Row] = Array([spark.sql.autoBroadcastJoinThreshold=<undefined>])

It seems a bunch of boilerplate could be removed and we could get some more typesafety by making the conf entries inter classes of SQLConf with methods like .value and .set(...). This would avoid having to have an entry and a def for each config option. Perhaps I'm missing something though.

marmbrus · 2015-06-12T02:56:56Z

sql/core/src/main/scala/org/apache/spark/sql/SQLConf.scala

Could we also have an entry type for configs like these that validates the option is in some list?

Added enumConf and used it for PARQUET_COMPRESSION

SparkQA · 2015-06-12T07:24:38Z

Test build #34763 has finished for PR 6747 at commit 037b1db.

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

zsxwing · 2015-06-12T08:08:54Z

The schema for set commands is wrong (we need some column name)

Added schemas for all set commands and split results into columns. For the configuration that has a default value, if it's not set , set <configuration> will show the default value.

It seems a bunch of boilerplate could be removed and we could get some more typesafety by making the conf entries inter classes of SQLConf with methods like .value and .set(...). This would avoid having to have an entry and a def for each config option. Perhaps I'm missing something though.

Sounds good. I will try it.

zsxwing · 2015-06-12T08:19:29Z

It seems a bunch of boilerplate could be removed and we could get some more typesafety by making the conf entries inter classes of SQLConf with methods like .value and .set(...). This would avoid having to have an entry and a def for each config option. Perhaps I'm missing something though.

One disadvantage is the size of a SQLConf instance will increase a lot since all SQLConfEntries are put into it.

liancheng · 2015-06-12T08:22:33Z

sql/core/src/main/scala/org/apache/spark/sql/SQLConf.scala

Adding a def unsetConf[T](entry: SQLConfEntry[T]) here can be convenient.

liancheng · 2015-06-12T08:44:17Z

sql/core/src/main/scala/org/apache/spark/sql/execution/commands.scala

Nit: Seems that we can remove the newline here.

liancheng · 2015-06-12T09:04:37Z

One of the things that I envy Hive is ConfVars, and now we finally have our own version :)

Some follow-ups I can think of (but not necessarily to be done in this PR):

Add generic version of setConf, getConf, unsetConf to SQLContext (note that HiveContext.setConf overrides SQLContext.setConf and also manipulates HiveConf there)
Replace all occurrences of sqlContext.setConf/getConf/unsetConf to the corresponding generic version

SparkQA · 2015-06-12T09:41:27Z

Test build #34767 has finished for PR 6747 at commit a2f4add.

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

SparkQA · 2015-06-12T10:24:45Z

Test build #34768 has finished for PR 6747 at commit 3c5f03e.

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

liancheng · 2015-06-12T10:48:20Z

Another thought is that, we probably also want to migrate data sources options to this new API.

@marmbrus How do you think?

SparkQA · 2015-06-12T16:07:55Z

Test build #34775 has finished for PR 6747 at commit cf950c1.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableCommand with Logging
- case class SetInFilter[T <: Comparable[T]](

SparkQA · 2015-06-16T16:01:52Z

Test build #34982 has finished for PR 6747 at commit e014f53.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class IsNull(child: Expression) extends UnaryExpression with Predicate
- case class IsNotNull(child: Expression) extends UnaryExpression with Predicate
- case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableCommand with Logging

SparkQA · 2015-06-17T18:47:16Z

Test build #35047 has finished for PR 6747 at commit 7d09bad.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableCommand with Logging

rxin · 2015-06-18T06:23:13Z

Alright merging this. Thanks.

1. Add `SQLConfEntry` to store the information about a configuration. For those configurations that cannot be found in `sql-programming-guide.md`, I left the doc as `<TODO>`. 2. Verify the value when setting a configuration if this is in SQLConf. 3. Use `SET -v` to display all public configurations. Author: zsxwing <zsxwing@gmail.com> Closes apache#6747 from zsxwing/sqlconf and squashes the following commits: 7d09bad [zsxwing] Use SQLConfEntry in HiveContext 49f6213 [zsxwing] Add getConf, setConf to SQLContext and HiveContext e014f53 [zsxwing] Merge branch 'master' into sqlconf 93dad8e [zsxwing] Fix the unit tests cf950c1 [zsxwing] Fix the code style and tests 3c5f03e [zsxwing] Add unsetConf(SQLConfEntry) and fix the code style a2f4add [zsxwing] getConf will return the default value if a config is not set 037b1db [zsxwing] Add schema to SetCommand 0520c3c [zsxwing] Merge branch 'master' into sqlconf 7afb0ec [zsxwing] Fix the configurations about HiveThriftServer 7e728e3 [zsxwing] Add doc for SQLConfEntry and fix 'toString' 5e95b10 [zsxwing] Add enumConf c6ba76d [zsxwing] setRawString => setConfString, getRawString => getConfString 4abd807 [zsxwing] Fix the test for 'set -v' 6e47e56 [zsxwing] Fix the compilation error 8973ced [zsxwing] Remove floatConf 1fc3a8b [zsxwing] Remove the 'conf' command and use 'set -v' instead 99c9c16 [zsxwing] Fix tests that use SQLConfEntry as a string 88a03cc [zsxwing] Add new lines between confs and return types ce7c6c8 [zsxwing] Remove seqConf f3c1b33 [zsxwing] Refactor SQLConf to display better error message

rxin reviewed Jun 10, 2015
View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/SQLConf.scala Outdated

Copy link

Contributor

rxin Jun 10, 2015

Choose a reason for hiding this comment

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

maybe remove the float version, and just use double version?

zsxwing added 5 commits June 11, 2015 15:53

Remove seqConf

ce7c6c8

Add new lines between confs and return types

88a03cc

Fix tests that use SQLConfEntry as a string

99c9c16

Remove the 'conf' command and use 'set -v' instead

1fc3a8b

Remove floatConf

8973ced

Fix the compilation error

6e47e56

Fix the test for 'set -v'

4abd807

marmbrus reviewed Jun 12, 2015
View reviewed changes

setRawString => setConfString, getRawString => getConfString

c6ba76d

zsxwing added 5 commits June 12, 2015 14:02

Add enumConf

5e95b10

Add doc for SQLConfEntry and fix 'toString'

7e728e3

Fix the configurations about HiveThriftServer

7afb0ec

Merge branch 'master' into sqlconf

0520c3c

Add schema to SetCommand

037b1db

getConf will return the default value if a config is not set

a2f4add

liancheng reviewed Jun 12, 2015
View reviewed changes

Add unsetConf(SQLConfEntry) and fix the code style

3c5f03e

liancheng reviewed Jun 12, 2015
View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/commands.scala Outdated

Copy link

Contributor

liancheng Jun 12, 2015

Choose a reason for hiding this comment

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

Nit: Seems that we can remove the newline here.

Fix the code style and tests

cf950c1

zsxwing added 2 commits June 13, 2015 09:22

Fix the unit tests

93dad8e

Merge branch 'master' into sqlconf

e014f53

zsxwing added 2 commits June 17, 2015 22:33

Add getConf, setConf to SQLContext and HiveContext

49f6213

Use SQLConfEntry in HiveContext

7d09bad

asfgit closed this in 78a430e Jun 18, 2015

zsxwing deleted the sqlconf branch June 18, 2015 13:26

[SPARK-7961][SQL]Refactor SQLConf to display better error message #6747

[SPARK-7961][SQL]Refactor SQLConf to display better error message #6747

Uh oh!

Conversation

zsxwing commented Jun 10, 2015

Uh oh!

zsxwing commented Jun 10, 2015

Uh oh!

SparkQA commented Jun 10, 2015

Uh oh!

SparkQA commented Jun 10, 2015

Uh oh!

rxin Jun 10, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 11, 2015

Uh oh!

SparkQA commented Jun 11, 2015

Uh oh!

SparkQA commented Jun 11, 2015

Uh oh!

SparkQA commented Jun 11, 2015

Uh oh!

zsxwing commented Jun 12, 2015

Uh oh!

marmbrus Jun 12, 2015

Choose a reason for hiding this comment

Uh oh!

zsxwing Jun 12, 2015

Choose a reason for hiding this comment

Uh oh!

marmbrus commented Jun 12, 2015

Uh oh!

marmbrus Jun 12, 2015

Choose a reason for hiding this comment

Uh oh!

zsxwing Jun 12, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 12, 2015

Uh oh!

zsxwing commented Jun 12, 2015

Uh oh!

zsxwing commented Jun 12, 2015

Uh oh!

liancheng Jun 12, 2015

Choose a reason for hiding this comment

Uh oh!

liancheng Jun 12, 2015

Choose a reason for hiding this comment

Uh oh!

liancheng commented Jun 12, 2015

Uh oh!

SparkQA commented Jun 12, 2015

Uh oh!

SparkQA commented Jun 12, 2015

Uh oh!

liancheng commented Jun 12, 2015

Uh oh!

SparkQA commented Jun 12, 2015

Uh oh!

SparkQA commented Jun 16, 2015

Uh oh!

SparkQA commented Jun 17, 2015

Uh oh!

rxin commented Jun 18, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants