[GitHub] spark pull request #21705: [SPARK-24727][SQL] Add a static config to control...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/21705 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21705: [SPARK-24727][SQL] Add a static config to control...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/21705#discussion_r199986545
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/internal/ExecutorSideSQLConfSuite.scala
---
@@ -63,4 +71,15 @@ class ExecutorSideSQLConfSuite extends SparkFunSuite
with SQLTestUtils {
}
}
}
+
+ test("SPARK-24727 CODEGEN_CACHE_SIZE is correctly referenced at the
executor side") {
--- End diff --
very nit: `CODEGEN_CACHE_SIZE` -> `CODEGEN_CACHE_MAX_ENTRIES`
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21705: [SPARK-24727][SQL] Add a static config to control...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/21705#discussion_r199940775
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/StaticSQLConf.scala
---
@@ -66,6 +66,12 @@ object StaticSQLConf {
.checkValue(cacheSize => cacheSize >= 0, "The maximum size of the
cache must not be negative")
.createWithDefault(1000)
+ val CODEGEN_CACHE_SIZE = buildStaticConf("spark.sql.codegen.cacheSize")
+ .doc("The maximum cache size for generated classes.")
--- End diff --
spark.sql.codegen.cache.maxEntries?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21705: [SPARK-24727][SQL] Add a static config to control...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/21705#discussion_r199935428
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/StaticSQLConf.scala
---
@@ -66,6 +66,12 @@ object StaticSQLConf {
.checkValue(cacheSize => cacheSize >= 0, "The maximum size of the
cache must not be negative")
.createWithDefault(1000)
+ val CODEGEN_CACHE_SIZE = buildStaticConf("spark.sql.codegen.cacheSize")
+ .doc("The maximum cache size for generated classes.")
+ .intConf
--- End diff --
this is internal, right?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21705: [SPARK-24727][SQL] Add a static config to control...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/21705#discussion_r199936887
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/StaticSQLConf.scala
---
@@ -66,6 +66,12 @@ object StaticSQLConf {
.checkValue(cacheSize => cacheSize >= 0, "The maximum size of the
cache must not be negative")
.createWithDefault(1000)
+ val CODEGEN_CACHE_SIZE = buildStaticConf("spark.sql.codegen.cacheSize")
+ .doc("The maximum cache size for generated classes.")
--- End diff --
We need to explain what is the cache is. It is hard to tell based on this
description.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21705: [SPARK-24727][SQL] Add a static config to control...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21705#discussion_r199858281
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/internal/ExecutorSideSQLConfSuite.scala
---
@@ -40,16 +40,24 @@ class ExecutorSideSQLConfSuite extends SparkFunSuite
with SQLTestUtils {
spark = null
}
+ private def withStaticSQLConf(pairs: (String, String)*)(f: => Unit):
Unit = {
--- End diff --
ah sorry for the confusion about "call", I mean rename this method...
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21705: [SPARK-24727][SQL] Add a static config to control...
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/21705#discussion_r199820857
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/internal/ExecutorSideSQLConfSuite.scala
---
@@ -40,16 +40,24 @@ class ExecutorSideSQLConfSuite extends SparkFunSuite
with SQLTestUtils {
spark = null
}
+ private def withStaticSQLConf(pairs: (String, String)*)(f: => Unit):
Unit = {
--- End diff --
In case of `withSQLConf`, I got an exception;
```
- SPARK-24727 CODEGEN_CACHE_SIZE is correctly referenced at the executor
side *** FAILED ***
org.apache.spark.sql.AnalysisException: Cannot modify the value of a
static config: spark.sql.codegen.cacheSize;
at
org.apache.spark.sql.catalyst.plans.PlanTestBase$$anonfun$withSQLConf$1.apply(PlanTest.scala:172)
```
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21705: [SPARK-24727][SQL] Add a static config to control...
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/21705#discussion_r199817849
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/internal/ExecutorSideSQLConfSuite.scala
---
@@ -40,16 +40,24 @@ class ExecutorSideSQLConfSuite extends SparkFunSuite
with SQLTestUtils {
spark = null
}
+ private def withStaticSQLConf(pairs: (String, String)*)(f: => Unit):
Unit = {
--- End diff --
will check
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21705: [SPARK-24727][SQL] Add a static config to control...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21705#discussion_r199817343
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/internal/ExecutorSideSQLConfSuite.scala
---
@@ -40,16 +40,24 @@ class ExecutorSideSQLConfSuite extends SparkFunSuite
with SQLTestUtils {
spark = null
}
+ private def withStaticSQLConf(pairs: (String, String)*)(f: => Unit):
Unit = {
--- End diff --
ah sorry I was wrong. Static conf is no different from normal conf, it's
just immutable during runtime. Maybe just call this method `withSQLConf`?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21705: [SPARK-24727][SQL] Add a static config to control...
GitHub user maropu opened a pull request: https://github.com/apache/spark/pull/21705 [SPARK-24727][SQL] Add a static config to control cache size for generated classes ## What changes were proposed in this pull request? Since SPARK-24250 has been resolved, executors correctly references user-defined configurations. So, this pr added a static config to control cache size for generated classes in `CodeGenerator`. ## How was this patch tested? Manually checked that executors referenced `spark.sql.cacheSize` correctly. You can merge this pull request into a Git repository by running: $ git pull https://github.com/maropu/spark SPARK-24727 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/21705.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #21705 commit 8ee8e00f156e577b32b01d015b8bd24f72ae7340 Author: Takeshi Yamamuro Date: 2018-07-03T13:14:35Z Fix --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
