You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by hvanhovell <gi...@git.apache.org> on 2016/08/17 11:22:36 UTC
[GitHub] spark pull request #14685: [SPARK-17106][SQL] Simplify the SubqueryExpressio...
GitHub user hvanhovell opened a pull request:
https://github.com/apache/spark/pull/14685
[SPARK-17106][SQL] Simplify the SubqueryExpression interface
## What changes were proposed in this pull request?
The current subquery expression interface contains a little bit of technical debt in the form of a few different access paths to get and set the query contained by the expression. This is confusing to anyone who goes over this code.
This PR unifies these access paths.
## How was this patch tested?
(Existing tests)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hvanhovell/spark SPARK-17106
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/14685.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 #14685
----
commit 8f11d35b55011f4b68365abe5da6b3936184df71
Author: Herman van Hovell <hv...@databricks.com>
Date: 2016-08-17T11:09:39Z
Simplify subquery expression interface.
commit d2bd26cf868fc6a44b01353af4bf40f8f01576cb
Author: Herman van Hovell <hv...@databricks.com>
Date: 2016-08-17T11:13:24Z
Merge remote-tracking branch 'apache-github/master' into SPARK-17106
# Conflicts:
# sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/subquery.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 issue #14685: [SPARK-17106][SQL] Simplify the SubqueryExpression inter...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/14685
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/63916/
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 #14685: [SPARK-17106][SQL] Simplify the SubqueryExpression inter...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14685
**[Test build #63916 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63916/consoleFull)** for PR 14685 at commit [`d2bd26c`](https://github.com/apache/spark/commit/d2bd26cf868fc6a44b01353af4bf40f8f01576cb).
---
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 #14685: [SPARK-17106][SQL] Simplify the SubqueryExpression inter...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/14685
**[Test build #63916 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63916/consoleFull)** for PR 14685 at commit [`d2bd26c`](https://github.com/apache/spark/commit/d2bd26cf868fc6a44b01353af4bf40f8f01576cb).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class SubqueryAlias(`
---
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 #14685: [SPARK-17106][SQL] Simplify the SubqueryExpression inter...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/14685
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 #14685: [SPARK-17106][SQL] Simplify the SubqueryExpressio...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/14685#discussion_r75128301
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/subquery.scala ---
@@ -56,30 +44,29 @@ trait ExecSubqueryExpression extends SubqueryExpression {
* This is the physical copy of ScalarSubquery to be used inside SparkPlan.
*/
case class ScalarSubquery(
- executedPlan: SubqueryExec,
+ plan: SubqueryExec,
exprId: ExprId)
extends ExecSubqueryExpression {
- override def dataType: DataType = executedPlan.schema.fields.head.dataType
+ override def dataType: DataType = plan.schema.fields.head.dataType
override def children: Seq[Expression] = Nil
override def nullable: Boolean = true
- override def toString: String = executedPlan.simpleString
-
- def withExecutedPlan(plan: SubqueryExec): ExecSubqueryExpression = copy(executedPlan = plan)
+ override def toString: String = plan.simpleString
+ override def withNewPlan(query: SubqueryExec): ScalarSubquery = copy(plan = query)
override def semanticEquals(other: Expression): Boolean = other match {
- case s: ScalarSubquery => executedPlan.sameResult(executedPlan)
+ case s: ScalarSubquery => plan.sameResult(s.plan)
--- End diff --
Good catch
---
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 #14685: [SPARK-17106][SQL] Simplify the SubqueryExpressio...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/14685#discussion_r75106052
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/subquery.scala ---
@@ -56,30 +44,29 @@ trait ExecSubqueryExpression extends SubqueryExpression {
* This is the physical copy of ScalarSubquery to be used inside SparkPlan.
*/
case class ScalarSubquery(
- executedPlan: SubqueryExec,
+ plan: SubqueryExec,
exprId: ExprId)
extends ExecSubqueryExpression {
- override def dataType: DataType = executedPlan.schema.fields.head.dataType
+ override def dataType: DataType = plan.schema.fields.head.dataType
override def children: Seq[Expression] = Nil
override def nullable: Boolean = true
- override def toString: String = executedPlan.simpleString
-
- def withExecutedPlan(plan: SubqueryExec): ExecSubqueryExpression = copy(executedPlan = plan)
+ override def toString: String = plan.simpleString
+ override def withNewPlan(query: SubqueryExec): ScalarSubquery = copy(plan = query)
override def semanticEquals(other: Expression): Boolean = other match {
- case s: ScalarSubquery => executedPlan.sameResult(executedPlan)
+ case s: ScalarSubquery => plan.sameResult(s.plan)
--- End diff --
This fixes a small bug in `sematicEquals`.
---
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 #14685: [SPARK-17106][SQL] Simplify the SubqueryExpression inter...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/14685
cc @davies (we discussed this in PR https://github.com/apache/spark/pull/14548)
---
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 #14685: [SPARK-17106][SQL] Simplify the SubqueryExpressio...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/14685
---
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 #14685: [SPARK-17106][SQL] Simplify the SubqueryExpression inter...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on the issue:
https://github.com/apache/spark/pull/14685
LGTM
---
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