You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2017/12/31 13:30:30 UTC
[GitHub] spark pull request #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
GitHub user gatorsmile opened a pull request:
https://github.com/apache/spark/pull/20127
[SPARK-22932] [SQL] Refactor AnalysisContext
## What changes were proposed in this pull request?
Add a `reset` function to ensure the state in `AnalysisContext ` is per-query.
## How was this patch tested?
The existing test cases
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gatorsmile/spark refactorAnalysisContext
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20127.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 #20127
----
commit f158a951b779e56e06d2c73234bac5c79055b2f5
Author: gatorsmile <ga...@...>
Date: 2017-12-31T13:21:13Z
refactor
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20127
**[Test build #85570 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85570/testReport)** for PR 20127 at commit [`f158a95`](https://github.com/apache/spark/commit/f158a951b779e56e06d2c73234bac5c79055b2f5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20127
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20127
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20127
cc @cloud-fan @jiangxb1987
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20127
**[Test build #85557 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85557/testReport)** for PR 20127 at commit [`f158a95`](https://github.com/apache/spark/commit/f158a951b779e56e06d2c73234bac5c79055b2f5).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20127
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/20127#discussion_r159150900
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -95,6 +98,17 @@ class Analyzer(
this(catalog, conf, conf.optimizerMaxIterations)
}
+ override def execute(plan: LogicalPlan): LogicalPlan = {
+ AnalysisContext.reset()
+ try {
+ executeSameContext(plan)
+ } finally {
+ AnalysisContext.reset()
+ }
+ }
+
+ private def executeSameContext(plan: LogicalPlan): LogicalPlan = super.execute(plan)
--- End diff --
`executeWithSameContext`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20127
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85562/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20127
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85570/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20127
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85557/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/20127
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20127
**[Test build #85557 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85557/testReport)** for PR 20127 at commit [`f158a95`](https://github.com/apache/spark/commit/f158a951b779e56e06d2c73234bac5c79055b2f5).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20127
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20127
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20127#discussion_r159174832
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -95,6 +98,17 @@ class Analyzer(
this(catalog, conf, conf.optimizerMaxIterations)
}
+ override def execute(plan: LogicalPlan): LogicalPlan = {
+ AnalysisContext.reset()
+ try {
+ executeSameContext(plan)
+ } finally {
+ AnalysisContext.reset()
+ }
+ }
+
+ private def executeSameContext(plan: LogicalPlan): LogicalPlan = super.execute(plan)
--- End diff --
+1
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/20127
Hi, @gatorsmile .
This PR seems to be landed into only `master`, but is marked as 2.3.0 in JIRA.
Could you merge this into `branch-2.3` or fix the version into 2.4 or 3.0?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20127
**[Test build #85570 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85570/testReport)** for PR 20127 at commit [`f158a95`](https://github.com/apache/spark/commit/f158a951b779e56e06d2c73234bac5c79055b2f5).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on the issue:
https://github.com/apache/spark/pull/20127
lgtm
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/20127#discussion_r159150948
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -70,6 +71,8 @@ object AnalysisContext {
}
def get: AnalysisContext = value.get()
+ def reset(): Unit = value.remove()
--- End diff --
`private`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20127
Thanks! Merged to master
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20127
Yeah, I manually merged it to 2.3 branch.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/20127#discussion_r159168110
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala ---
@@ -70,6 +71,8 @@ object AnalysisContext {
}
def get: AnalysisContext = value.get()
+ def reset(): Unit = value.remove()
--- End diff --
Will be resolved by the future PR.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20127
**[Test build #85562 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85562/testReport)** for PR 20127 at commit [`f158a95`](https://github.com/apache/spark/commit/f158a951b779e56e06d2c73234bac5c79055b2f5).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20127: [SPARK-22932] [SQL] Refactor AnalysisContext
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20127
**[Test build #85562 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85562/testReport)** for PR 20127 at commit [`f158a95`](https://github.com/apache/spark/commit/f158a951b779e56e06d2c73234bac5c79055b2f5).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org