You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by kunalkhamar <gi...@git.apache.org> on 2017/03/21 21:19:27 UTC
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
GitHub user kunalkhamar opened a pull request:
https://github.com/apache/spark/pull/17379
[SPARK-20048][SQL] Cloning SessionState does not clone query execution listeners
## What changes were proposed in this pull request?
Bugfix from SPARK-19540.
Cloning SessionState does not clone query execution listeners, so cloned session is unable to listen to events on queries.
## How was this patch tested?
- Unit test
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kunalkhamar/spark clone-bugfix
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/17379.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 #17379
----
commit ad77fe9ad258eac224f069bbc89294818ee6b549
Author: Kunal Khamar <kk...@outlook.com>
Date: 2017-03-21T21:16:04Z
Fix cloning of listener manager. Remove redundant comments.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r108663100
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/sessionStateBuilders.scala ---
@@ -134,6 +135,14 @@ abstract class BaseSessionStateBuilder(
}
/**
+ * Interface exposed to the user for registering user-defined functions.
+ *
+ * Note 1: The user-defined functions must be deterministic.
+ * Note 2: This depends on the `functionRegistry` field.
+ */
+ protected def udf: UDFRegistration = new UDFRegistration(functionRegistry)
--- End diff --
It also contains a mix-in `WithTestConf`. But renaming is fine.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r107821546
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SessionState.scala ---
@@ -134,17 +131,20 @@ private[sql] class SessionState(
SessionState.mergeSparkConf(confCopy, sparkContext.getConf)
- new SessionState(
- sparkContext,
- newSparkSession.sharedState,
- confCopy,
- experimentalMethods.clone(),
- functionRegistryCopy,
- catalogCopy,
- sqlParser,
- SessionState.createAnalyzer(newSparkSession, catalogCopy, confCopy),
- new StreamingQueryManager(newSparkSession),
- queryExecutionCreator)
+ val newSessionState = new SessionState(
+ sparkContext,
+ newSparkSession.sharedState,
+ confCopy,
+ experimentalMethods.clone(),
+ functionRegistryCopy,
+ catalogCopy,
+ sqlParser,
+ SessionState.createAnalyzer(newSparkSession, catalogCopy, confCopy),
+ new StreamingQueryManager(newSparkSession),
+ queryExecutionCreator) {
+ override val listenerManager: ExecutionListenerManager = self.listenerManager.clone()
--- End diff --
I think the general rule we followed in the cloneSession PR is that `val`s that directly depend on `SparkSession` to be initialized, are to be constructor params. We leave the other `val`s in the body of class.
The advantage here is less duplicated code between `SessionState`, `HiveSessionState` and `TestHiveSparkSession`.
This is consistent with that, what would be the advantage of changing it to be a param?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75002 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75002/testReport)** for PR 17379 at commit [`ad77fe9`](https://github.com/apache/spark/commit/ad77fe9ad258eac224f069bbc89294818ee6b549).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/75002/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/17379
Merging to master. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r108586629
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SessionState.scala ---
@@ -37,38 +37,42 @@ import org.apache.spark.sql.util.ExecutionListenerManager
/**
* A class that holds all session-specific state in a given [[SparkSession]].
*
- * @param sparkContext The [[SparkContext]].
- * @param sharedState The shared state.
+ * @param sharedState The state shared across sessions, e.g. global view manager, external catalog.
* @param conf SQL-specific key-value configurations.
- * @param experimentalMethods The experimental methods.
+ * @param experimentalMethods Interface to add custom planning strategies and optimizers.
* @param functionRegistry Internal catalog for managing functions registered by the user.
+ * @param udf Interface exposed to the user for registering user-defined functions.
* @param catalog Internal catalog for managing table and database states.
* @param sqlParser Parser that extracts expressions, plans, table identifiers etc. from SQL texts.
* @param analyzer Logical query plan analyzer for resolving unresolved attributes and relations.
* @param optimizer Logical query plan optimizer.
* @param planner Planner that converts optimized logical plans to physical plans
* @param streamingQueryManager Interface to start and stop streaming queries.
+ * @param listenerManager Interface to register custom
+ * [[org.apache.spark.sql.util.QueryExecutionListener]]s
+ * @param resourceLoader Session shared resource loader to load JARs, files, etc
* @param createQueryExecution Function used to create QueryExecution objects.
* @param createClone Function used to create clones of the session state.
*/
private[sql] class SessionState(
- sparkContext: SparkContext,
sharedState: SharedState,
val conf: SQLConf,
val experimentalMethods: ExperimentalMethods,
val functionRegistry: FunctionRegistry,
+ val udf: UDFRegistration,
--- End diff --
changed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75338 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75338/testReport)** for PR 17379 at commit [`119dae9`](https://github.com/apache/spark/commit/119dae974554bc7a1755b8532c373464618ad56d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75015 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75015/testReport)** for PR 17379 at commit [`ad77fe9`](https://github.com/apache/spark/commit/ad77fe9ad258eac224f069bbc89294818ee6b549).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r108748031
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/BaseSessionStateBuilder.scala ---
@@ -134,6 +135,14 @@ abstract class BaseSessionStateBuilder(
}
/**
+ * Interface exposed to the user for registering user-defined functions.
+ *
+ * Note 1: The user-defined functions must be deterministic.
+ * Note 2: This depends on the `functionRegistry` field.
+ */
+ protected def udfRegistration: UDFRegistration = new UDFRegistration(functionRegistry)
--- End diff --
It was the only thing not initialized in the builder, thought it would be more consistent to have all initialization in the builder. Is this okay?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75331 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75331/testReport)** for PR 17379 at commit [`cad1b63`](https://github.com/apache/spark/commit/cad1b6314c64fc5308d3b5ad0a86285356abbac0).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r108662296
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/BaseSessionStateBuilder.scala ---
@@ -134,6 +135,14 @@ abstract class BaseSessionStateBuilder(
}
/**
+ * Interface exposed to the user for registering user-defined functions.
+ *
+ * Note 1: The user-defined functions must be deterministic.
+ * Note 2: This depends on the `functionRegistry` field.
+ */
+ protected def udfRegistration: UDFRegistration = new UDFRegistration(functionRegistry)
--- End diff --
Minor: I was wondering why you moved this into the builder?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/17379
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r108586704
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionState.scala ---
@@ -32,15 +32,15 @@ import org.apache.spark.sql.internal.{BaseSessionStateBuilder, SessionResourceLo
*/
private[hive] object HiveSessionState {
/**
- * Create a new Hive aware [[SessionState]]. for the given session.
+ * Create a new Hive aware [[SessionState]] for the given session.
*/
def apply(session: SparkSession): SessionState = {
new HiveSessionStateBuilder(session).build()
}
}
/**
- * Builder that produces a [[HiveSessionState]].
+ * Builder that produces a Hive aware [[SessionState]].
*/
@Experimental
@InterfaceStability.Unstable
--- End diff --
Renamed, removed `object HiveSessionState`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r107790543
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SessionStateSuite.scala ---
@@ -122,6 +125,59 @@ class SessionStateSuite extends SparkFunSuite
}
}
+ test("fork new session and inherit listener manager") {
+ def createTestListener: (ArrayBuffer[String], QueryExecutionListener) = {
--- End diff --
super nit: cleaner to do
```
class CommandCollector extends QueryExecutionListener {
val commands = ...
...
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on the issue:
https://github.com/apache/spark/pull/17379
@hvanhovell it should be HiveSessionStateBuilder now
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r108532136
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SessionState.scala ---
@@ -134,17 +131,20 @@ private[sql] class SessionState(
SessionState.mergeSparkConf(confCopy, sparkContext.getConf)
- new SessionState(
- sparkContext,
- newSparkSession.sharedState,
- confCopy,
- experimentalMethods.clone(),
- functionRegistryCopy,
- catalogCopy,
- sqlParser,
- SessionState.createAnalyzer(newSparkSession, catalogCopy, confCopy),
- new StreamingQueryManager(newSparkSession),
- queryExecutionCreator)
+ val newSessionState = new SessionState(
+ sparkContext,
+ newSparkSession.sharedState,
+ confCopy,
+ experimentalMethods.clone(),
+ functionRegistryCopy,
+ catalogCopy,
+ sqlParser,
+ SessionState.createAnalyzer(newSparkSession, catalogCopy, confCopy),
+ new StreamingQueryManager(newSparkSession),
+ queryExecutionCreator) {
+ override val listenerManager: ExecutionListenerManager = self.listenerManager.clone()
--- End diff --
Builders were added in [https://github.com/apache/spark/pull/17433](url). Changed initialization to use that pattern, so it is a constructor param now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75328 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75328/testReport)** for PR 17379 at commit [`16f5bea`](https://github.com/apache/spark/commit/16f5beae3d08627606c13ccb301d624836cb1233).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/75328/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r108586634
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/sessionStateBuilders.scala ---
@@ -134,6 +135,14 @@ abstract class BaseSessionStateBuilder(
}
/**
+ * Interface exposed to the user for registering user-defined functions.
+ *
+ * Note 1: The user-defined functions must be deterministic.
+ * Note 2: This depends on the `functionRegistry` field.
+ */
+ protected def udf: UDFRegistration = new UDFRegistration(functionRegistry)
--- End diff --
changed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r107786778
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionState.scala ---
@@ -32,19 +32,9 @@ import org.apache.spark.sql.streaming.StreamingQueryManager
/**
* A class that holds all session-specific state in a given [[SparkSession]] backed by Hive.
- * @param sparkContext The [[SparkContext]].
- * @param sharedState The shared state.
- * @param conf SQL-specific key-value configurations.
- * @param experimentalMethods The experimental methods.
- * @param functionRegistry Internal catalog for managing functions registered by the user.
* @param catalog Internal catalog for managing table and database states that uses Hive client for
* interacting with the metastore.
- * @param sqlParser Parser that extracts expressions, plans, table identifiers etc. from SQL texts.
* @param metadataHive The Hive metadata client.
- * @param analyzer Logical query plan analyzer for resolving unresolved attributes and relations.
- * @param streamingQueryManager Interface to start and stop
- * [[org.apache.spark.sql.streaming.StreamingQuery]]s.
- * @param queryExecutionCreator Lambda to create a [[QueryExecution]] from a [[LogicalPlan]]
--- End diff --
why have these been removed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r107819804
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SessionState.scala ---
@@ -38,10 +38,7 @@ import org.apache.spark.sql.util.ExecutionListenerManager
/**
* A class that holds all session-specific state in a given [[SparkSession]].
- * @param sparkContext The [[SparkContext]].
- * @param sharedState The shared state.
* @param conf SQL-specific key-value configurations.
- * @param experimentalMethods The experimental methods.
--- End diff --
These comments are adding little or no value. We should remove or make them more detailed, which would you prefer? If the latter, what's a good doc for shared state and experimental methods?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75066 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75066/testReport)** for PR 17379 at commit [`f63e81d`](https://github.com/apache/spark/commit/f63e81de5c0119e736ad0ddea7977da1060893a9).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75331 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75331/testReport)** for PR 17379 at commit [`cad1b63`](https://github.com/apache/spark/commit/cad1b6314c64fc5308d3b5ad0a86285356abbac0).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r107786408
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SessionState.scala ---
@@ -38,10 +38,7 @@ import org.apache.spark.sql.util.ExecutionListenerManager
/**
* A class that holds all session-specific state in a given [[SparkSession]].
- * @param sparkContext The [[SparkContext]].
- * @param sharedState The shared state.
* @param conf SQL-specific key-value configurations.
- * @param experimentalMethods The experimental methods.
* @param functionRegistry Internal catalog for managing functions registered by the user.
--- End diff --
why have you removed these?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r108562157
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SessionState.scala ---
@@ -37,38 +37,42 @@ import org.apache.spark.sql.util.ExecutionListenerManager
/**
* A class that holds all session-specific state in a given [[SparkSession]].
*
- * @param sparkContext The [[SparkContext]].
- * @param sharedState The shared state.
+ * @param sharedState The state shared across sessions, e.g. global view manager, external catalog.
* @param conf SQL-specific key-value configurations.
- * @param experimentalMethods The experimental methods.
+ * @param experimentalMethods Interface to add custom planning strategies and optimizers.
* @param functionRegistry Internal catalog for managing functions registered by the user.
+ * @param udf Interface exposed to the user for registering user-defined functions.
* @param catalog Internal catalog for managing table and database states.
* @param sqlParser Parser that extracts expressions, plans, table identifiers etc. from SQL texts.
* @param analyzer Logical query plan analyzer for resolving unresolved attributes and relations.
* @param optimizer Logical query plan optimizer.
* @param planner Planner that converts optimized logical plans to physical plans
* @param streamingQueryManager Interface to start and stop streaming queries.
+ * @param listenerManager Interface to register custom
+ * [[org.apache.spark.sql.util.QueryExecutionListener]]s
+ * @param resourceLoader Session shared resource loader to load JARs, files, etc
* @param createQueryExecution Function used to create QueryExecution objects.
* @param createClone Function used to create clones of the session state.
*/
private[sql] class SessionState(
- sparkContext: SparkContext,
sharedState: SharedState,
val conf: SQLConf,
val experimentalMethods: ExperimentalMethods,
val functionRegistry: FunctionRegistry,
+ val udf: UDFRegistration,
--- End diff --
udf -> udfRegistration
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r108531631
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionState.scala ---
@@ -32,19 +32,9 @@ import org.apache.spark.sql.streaming.StreamingQueryManager
/**
* A class that holds all session-specific state in a given [[SparkSession]] backed by Hive.
- * @param sparkContext The [[SparkContext]].
- * @param sharedState The shared state.
- * @param conf SQL-specific key-value configurations.
- * @param experimentalMethods The experimental methods.
- * @param functionRegistry Internal catalog for managing functions registered by the user.
* @param catalog Internal catalog for managing table and database states that uses Hive client for
* interacting with the metastore.
- * @param sqlParser Parser that extracts expressions, plans, table identifiers etc. from SQL texts.
* @param metadataHive The Hive metadata client.
- * @param analyzer Logical query plan analyzer for resolving unresolved attributes and relations.
- * @param streamingQueryManager Interface to start and stop
- * [[org.apache.spark.sql.streaming.StreamingQuery]]s.
- * @param queryExecutionCreator Lambda to create a [[QueryExecution]] from a [[LogicalPlan]]
--- End diff --
`HiveSessionState` is gone, so this is not relevant anymore.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75337 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75337/testReport)** for PR 17379 at commit [`be191c6`](https://github.com/apache/spark/commit/be191c62f7d549931debbc08f21e025edf418faa).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/75338/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/75066/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75337 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75337/testReport)** for PR 17379 at commit [`be191c6`](https://github.com/apache/spark/commit/be191c62f7d549931debbc08f21e025edf418faa).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on the issue:
https://github.com/apache/spark/pull/17379
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75135 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75135/testReport)** for PR 17379 at commit [`eb5581a`](https://github.com/apache/spark/commit/eb5581a8b685407c33a3ff76885dd6b9cb6f5fd8).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75135 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75135/testReport)** for PR 17379 at commit [`eb5581a`](https://github.com/apache/spark/commit/eb5581a8b685407c33a3ff76885dd6b9cb6f5fd8).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r107788598
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SessionState.scala ---
@@ -134,17 +131,20 @@ private[sql] class SessionState(
SessionState.mergeSparkConf(confCopy, sparkContext.getConf)
- new SessionState(
- sparkContext,
- newSparkSession.sharedState,
- confCopy,
- experimentalMethods.clone(),
- functionRegistryCopy,
- catalogCopy,
- sqlParser,
- SessionState.createAnalyzer(newSparkSession, catalogCopy, confCopy),
- new StreamingQueryManager(newSparkSession),
- queryExecutionCreator)
+ val newSessionState = new SessionState(
+ sparkContext,
+ newSparkSession.sharedState,
+ confCopy,
+ experimentalMethods.clone(),
+ functionRegistryCopy,
+ catalogCopy,
+ sqlParser,
+ SessionState.createAnalyzer(newSparkSession, catalogCopy, confCopy),
+ new StreamingQueryManager(newSparkSession),
+ queryExecutionCreator) {
+ override val listenerManager: ExecutionListenerManager = self.listenerManager.clone()
--- End diff --
Why do it this way? Why not pass it as a constructor param?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/75015/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75002 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75002/testReport)** for PR 17379 at commit [`ad77fe9`](https://github.com/apache/spark/commit/ad77fe9ad258eac224f069bbc89294818ee6b549).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r108532483
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SessionState.scala ---
@@ -38,10 +38,7 @@ import org.apache.spark.sql.util.ExecutionListenerManager
/**
* A class that holds all session-specific state in a given [[SparkSession]].
- * @param sparkContext The [[SparkContext]].
- * @param sharedState The shared state.
* @param conf SQL-specific key-value configurations.
- * @param experimentalMethods The experimental methods.
--- End diff --
Added it back, hopefully more descriptive.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r108662869
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionState.scala ---
@@ -32,15 +32,15 @@ import org.apache.spark.sql.internal.{BaseSessionStateBuilder, SessionResourceLo
*/
private[hive] object HiveSessionState {
/**
- * Create a new Hive aware [[SessionState]]. for the given session.
+ * Create a new Hive aware [[SessionState]] for the given session.
*/
def apply(session: SparkSession): SessionState = {
new HiveSessionStateBuilder(session).build()
}
}
/**
- * Builder that produces a [[HiveSessionState]].
+ * Builder that produces a Hive aware [[SessionState]].
*/
@Experimental
@InterfaceStability.Unstable
--- End diff --
Yeah, lets rename the file to `HiveSessionBuilder.scala`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r107819998
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionState.scala ---
@@ -32,19 +32,9 @@ import org.apache.spark.sql.streaming.StreamingQueryManager
/**
* A class that holds all session-specific state in a given [[SparkSession]] backed by Hive.
- * @param sparkContext The [[SparkContext]].
- * @param sharedState The shared state.
- * @param conf SQL-specific key-value configurations.
- * @param experimentalMethods The experimental methods.
- * @param functionRegistry Internal catalog for managing functions registered by the user.
* @param catalog Internal catalog for managing table and database states that uses Hive client for
* interacting with the metastore.
- * @param sqlParser Parser that extracts expressions, plans, table identifiers etc. from SQL texts.
* @param metadataHive The Hive metadata client.
- * @param analyzer Logical query plan analyzer for resolving unresolved attributes and relations.
- * @param streamingQueryManager Interface to start and stop
- * [[org.apache.spark.sql.streaming.StreamingQuery]]s.
- * @param queryExecutionCreator Lambda to create a [[QueryExecution]] from a [[LogicalPlan]]
--- End diff --
Each of these is identical to their `SessionState` counterpart and should be inherited by Scaladoc comment inheritance, do we still need them?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/75135/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75066 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75066/testReport)** for PR 17379 at commit [`f63e81d`](https://github.com/apache/spark/commit/f63e81de5c0119e736ad0ddea7977da1060893a9).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75328 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75328/testReport)** for PR 17379 at commit [`16f5bea`](https://github.com/apache/spark/commit/16f5beae3d08627606c13ccb301d624836cb1233).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r108577718
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionState.scala ---
@@ -32,15 +32,15 @@ import org.apache.spark.sql.internal.{BaseSessionStateBuilder, SessionResourceLo
*/
private[hive] object HiveSessionState {
/**
- * Create a new Hive aware [[SessionState]]. for the given session.
+ * Create a new Hive aware [[SessionState]] for the given session.
*/
def apply(session: SparkSession): SessionState = {
new HiveSessionStateBuilder(session).build()
}
}
/**
- * Builder that produces a [[HiveSessionState]].
+ * Builder that produces a Hive aware [[SessionState]].
*/
@Experimental
@InterfaceStability.Unstable
--- End diff --
This file should not be named HiveSessionState anymore. It doesnt even have the class HiveSessionState. It does have an object HiveSession, but do we need that object any more?
@hvanhovell
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75338 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75338/testReport)** for PR 17379 at commit [`119dae9`](https://github.com/apache/spark/commit/119dae974554bc7a1755b8532c373464618ad56d).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r107786481
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SessionState.scala ---
@@ -38,10 +38,7 @@ import org.apache.spark.sql.util.ExecutionListenerManager
/**
* A class that holds all session-specific state in a given [[SparkSession]].
- * @param sparkContext The [[SparkContext]].
- * @param sharedState The shared state.
* @param conf SQL-specific key-value configurations.
- * @param experimentalMethods The experimental methods.
--- End diff --
why have you removed these?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r107788736
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SessionStateSuite.scala ---
@@ -122,6 +125,59 @@ class SessionStateSuite extends SparkFunSuite
}
}
+ test("fork new session and inherit listener manager") {
+ def createTestListener: (ArrayBuffer[String], QueryExecutionListener) = {
--- End diff --
Add a comment on what it creates? what the two things returned.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17379
**[Test build #75015 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/75015/testReport)** for PR 17379 at commit [`ad77fe9`](https://github.com/apache/spark/commit/ad77fe9ad258eac224f069bbc89294818ee6b549).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r108576926
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/sessionStateBuilders.scala ---
@@ -134,6 +135,14 @@ abstract class BaseSessionStateBuilder(
}
/**
+ * Interface exposed to the user for registering user-defined functions.
+ *
+ * Note 1: The user-defined functions must be deterministic.
+ * Note 2: This depends on the `functionRegistry` field.
+ */
+ protected def udf: UDFRegistration = new UDFRegistration(functionRegistry)
--- End diff --
This file only contains effectively one builder. So it should be named after the class.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/75331/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on the issue:
https://github.com/apache/spark/pull/17379
cc @hvanhovell
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #17379: [SPARK-20048][SQL] Cloning SessionState does not clone q...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17379
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/75337/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #17379: [SPARK-20048][SQL] Cloning SessionState does not ...
Posted by kunalkhamar <gi...@git.apache.org>.
Github user kunalkhamar commented on a diff in the pull request:
https://github.com/apache/spark/pull/17379#discussion_r107820985
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/SessionStateSuite.scala ---
@@ -122,6 +125,59 @@ class SessionStateSuite extends SparkFunSuite
}
}
+ test("fork new session and inherit listener manager") {
+ def createTestListener: (ArrayBuffer[String], QueryExecutionListener) = {
--- End diff --
neat, changed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org