You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/11/09 15:19:43 UTC
[GitHub] [spark] prakharjain09 opened a new pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
prakharjain09 opened a new pull request #30300:
URL: https://github.com/apache/spark/pull/30300
### What changes were proposed in this pull request?
This pull request tries to remove unneeded exchanges in cases when we have an INNER join followed by a Project with alias.
Ex - SortMergeJoin (SMJ2)
|
Project
|
SortMergeJoin (SMJ1)
In such cases, The projects output partitioning is same as SMJ1's partitioning but if there are aliases in Project, we need to normalize the attributes.
### Why are the changes needed?
To remove unneeded exchanges.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
New UT added.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726041363
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727842806
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35745/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r521211539
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -25,20 +25,14 @@ import org.apache.spark.sql.catalyst.plans.physical.{HashPartitioning, Partition
trait AliasAwareOutputExpression extends UnaryExecNode {
protected def outputExpressions: Seq[NamedExpression]
- protected def hasAlias: Boolean = outputExpressions.collectFirst { case _: Alias => }.isDefined
+ lazy val aliasMap = AttributeMap(outputExpressions.collect {
+ case a @ Alias(child: AttributeReference, _) => (child, a.toAttribute)
+ })
- protected def replaceAliases(exprs: Seq[Expression]): Seq[Expression] = {
- exprs.map {
- case a: AttributeReference => replaceAlias(a).getOrElse(a)
- case other => other
- }
- }
+ protected def hasAlias: Boolean = aliasMap.nonEmpty
protected def replaceAlias(attr: AttributeReference): Option[Attribute] = {
Review comment:
I looked into AliasHelper - it has utility methods to pushdown expressions. i.e. suppose we have f1 filter on top of Project, how to transform f1 so that we can use it below Project. For this it creates mapping between newName -> oldExpression.
In our case, we want different kind of map. given an expression coming from below the project, how will it look above Project. For this we need oldAttributeReference -> newAttributeReference map.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726739504
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning with respect to aliases to avoid unneeded exchanges
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-725381027
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724383172
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726739504
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r522234818
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala
##########
@@ -895,6 +895,121 @@ class PlannerSuite extends SharedSparkSession with AdaptiveSparkPlanHelper {
}
}
+ test("SPARK-33399: aliases should be handled properly in PartitioningCollection output" +
+ " partitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id, t2.id as t2id
+ | FROM t1, t2
+ | WHERE t1.id = t2.id
+ |) t12, t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+ }
+ }
+ }
+
+ test("SPARK-33399: aliases should be handled properly in HashPartitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t1id, t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id
+ | FROM t1 LEFT SEMI JOIN t2
+ | ON t1.id = t2.id
+ |) t12 INNER JOIN t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+ }
+ }
+ }
+
+ test("SPARK-33399: alias handling should happen properly for RangePartitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ val df = spark.range(1, 100)
+ .select(col("id").as("id1")).groupBy("id1").count()
+ // Plan for this will be Range -> ProjectWithAlias -> HashAggregate -> HashAggregate
+ // if Project normalizes alias in its Range outputPartitioning, then no Exchange should come
+ // in between HashAggregates
+ val planned = df.queryExecution.executedPlan
Review comment:
Added checks to assert partitioning is what we expect.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu closed pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
maropu closed pull request #30300:
URL: https://github.com/apache/spark/pull/30300
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724079456
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-728024896
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r522091225
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala
##########
@@ -895,6 +895,121 @@ class PlannerSuite extends SharedSparkSession with AdaptiveSparkPlanHelper {
}
}
+ test("SPARK-33399: aliases should be handled properly in PartitioningCollection output" +
+ " partitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id, t2.id as t2id
+ | FROM t1, t2
+ | WHERE t1.id = t2.id
+ |) t12, t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+ }
+ }
+ }
+
+ test("SPARK-33399: aliases should be handled properly in HashPartitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t1id, t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id
+ | FROM t1 LEFT SEMI JOIN t2
+ | ON t1.id = t2.id
+ |) t12 INNER JOIN t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+ }
+ }
+ }
+
+ test("SPARK-33399: alias handling should happen properly for RangePartitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ val df = spark.range(1, 100)
+ .select(col("id").as("id1")).groupBy("id1").count()
+ // Plan for this will be Range -> ProjectWithAlias -> HashAggregate -> HashAggregate
+ // if Project normalizes alias in its Range outputPartitioning, then no Exchange should come
+ // in between HashAggregates
+ val planned = df.queryExecution.executedPlan
Review comment:
Ah, last my nit comment: could you check the query use `RangePartitioning` correctly in this test (I have the same comment for the other tests, too)?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726221712
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35615/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726602421
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/35646/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r521206322
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -48,13 +42,13 @@ trait AliasAwareOutputExpression extends UnaryExecNode {
*/
trait AliasAwareOutputPartitioning extends AliasAwareOutputExpression {
final override def outputPartitioning: Partitioning = {
- if (hasAlias) {
- child.outputPartitioning match {
- case h: HashPartitioning => h.copy(expressions = replaceAliases(h.expressions))
Review comment:
Added separate UTs for HashPartitioning, RangePartitioning, PartitioningCollection, SinglePartition.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r522016699
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -25,20 +25,14 @@ import org.apache.spark.sql.catalyst.plans.physical.{HashPartitioning, Partition
trait AliasAwareOutputExpression extends UnaryExecNode {
protected def outputExpressions: Seq[NamedExpression]
- protected def hasAlias: Boolean = outputExpressions.collectFirst { case _: Alias => }.isDefined
+ private lazy val aliasMap = AttributeMap(outputExpressions.collect {
+ case a @ Alias(child: AttributeReference, _) => (child, a.toAttribute)
+ })
- protected def replaceAliases(exprs: Seq[Expression]): Seq[Expression] = {
- exprs.map {
- case a: AttributeReference => replaceAlias(a).getOrElse(a)
- case other => other
- }
- }
+ protected def hasAlias: Boolean = aliasMap.nonEmpty
protected def replaceAlias(attr: AttributeReference): Option[Attribute] = {
- outputExpressions.collectFirst {
- case a @ Alias(child: AttributeReference, _) if child.semanticEquals(attr) =>
- a.toAttribute
- }
+ aliasMap.get(attr)
Review comment:
removed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726602414
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning with respect to aliases to avoid unneeded exchanges
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-725294802
**[Test build #130926 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130926/testReport)** for PR 30300 at commit [`adf3a66`](https://github.com/apache/spark/commit/adf3a66b454aa7f95e9e346e1b6440b79c8b3c1b).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726668466
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35654/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-728019749
**[Test build #131143 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131143/testReport)** for PR 30300 at commit [`16e1db2`](https://github.com/apache/spark/commit/16e1db202f3522ebe607894b93811bb54bdd9a0c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` >>> class VectorAccumulatorParam(AccumulatorParam):`
* ` fully qualified classname of key Writable class (e.g. \"org.apache.hadoop.io.Text\")`
* ` fully qualified classname of key Writable class (e.g. \"org.apache.hadoop.io.Text\")`
* ` fully qualified classname of key Writable class (e.g. \"org.apache.hadoop.io.Text\")`
* ` fully qualified classname of key Writable class (e.g. \"org.apache.hadoop.io.Text\")`
* `class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with Logging `
* `abstract class AbstractSqlParser extends ParserInterface with Logging `
* `class CatalystSqlParser extends AbstractSqlParser `
* `class SparkSqlParser extends AbstractSqlParser `
* `class SparkSqlAstBuilder extends AstBuilder `
* `class VariableSubstitution `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r521214124
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala
##########
@@ -895,6 +895,73 @@ class PlannerSuite extends SharedSparkSession with AdaptiveSparkPlanHelper {
}
}
+ test("No extra exchanges in case of [Inner Join -> Project with aliases -> Inner join]") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withSQLConf(SQLConf.CONSTRAINT_PROPAGATION_ENABLED.key -> "false") {
Review comment:
In specific test case, I was getting this error initially and so I turned off that config-
org.apache.spark.sql.catalyst.errors.package$TreeNodeException: Once strategy's idempotence is broken for batch Infer Filters
But now I have simplified my testcase and no longer need this config - creating separate jira for idempotence issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724488706
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724360087
**[Test build #130810 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130810/testReport)** for PR 30300 at commit [`dd6b841`](https://github.com/apache/spark/commit/dd6b8411b9be52b2d6cd104e3c2fa408083b339e).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726668476
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning with respect to aliases to avoid unneeded exchanges
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-725347851
@maropu @viirya Thanks for the review. I have addressed majority of the review comments.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727812985
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/35739/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727778057
**[Test build #131136 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131136/testReport)** for PR 30300 at commit [`16e1db2`](https://github.com/apache/spark/commit/16e1db202f3522ebe607894b93811bb54bdd9a0c).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727821362
**[Test build #131143 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131143/testReport)** for PR 30300 at commit [`16e1db2`](https://github.com/apache/spark/commit/16e1db202f3522ebe607894b93811bb54bdd9a0c).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726185701
**[Test build #131009 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131009/testReport)** for PR 30300 at commit [`f4fd12e`](https://github.com/apache/spark/commit/f4fd12e514b17d9e78f7e662df3c3fb502fd61d0).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726590510
**[Test build #131040 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131040/testReport)** for PR 30300 at commit [`c66874a`](https://github.com/apache/spark/commit/c66874a00545cb0b10a6eb533ce65bb71610fd18).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-725294802
**[Test build #130926 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130926/testReport)** for PR 30300 at commit [`adf3a66`](https://github.com/apache/spark/commit/adf3a66b454aa7f95e9e346e1b6440b79c8b3c1b).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727778057
**[Test build #131136 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131136/testReport)** for PR 30300 at commit [`16e1db2`](https://github.com/apache/spark/commit/16e1db202f3522ebe607894b93811bb54bdd9a0c).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726235351
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724488706
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning with respect to aliases to avoid unneeded exchanges
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-725381012
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35531/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r522155907
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala
##########
@@ -895,6 +895,121 @@ class PlannerSuite extends SharedSparkSession with AdaptiveSparkPlanHelper {
}
}
+ test("SPARK-33399: aliases should be handled properly in PartitioningCollection output" +
+ " partitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id, t2.id as t2id
+ | FROM t1, t2
+ | WHERE t1.id = t2.id
+ |) t12, t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+ }
+ }
+ }
+
+ test("SPARK-33399: aliases should be handled properly in HashPartitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t1id, t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id
+ | FROM t1 LEFT SEMI JOIN t2
+ | ON t1.id = t2.id
+ |) t12 INNER JOIN t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+ }
+ }
+ }
+
+ test("SPARK-33399: alias handling should happen properly for RangePartitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ val df = spark.range(1, 100)
+ .select(col("id").as("id1")).groupBy("id1").count()
+ // Plan for this will be Range -> ProjectWithAlias -> HashAggregate -> HashAggregate
+ // if Project normalizes alias in its Range outputPartitioning, then no Exchange should come
+ // in between HashAggregates
+ val planned = df.queryExecution.executedPlan
Review comment:
@maropu you mean to add assertions that the partitioning here is RangePartitioning and not something else?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-728024896
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning with respect to aliases to avoid unneeded exchanges
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-725435699
**[Test build #130926 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130926/testReport)** for PR 30300 at commit [`adf3a66`](https://github.com/apache/spark/commit/adf3a66b454aa7f95e9e346e1b6440b79c8b3c1b).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726668476
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning with respect to aliases to avoid unneeded exchanges
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-725436994
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726235351
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726185701
**[Test build #131009 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131009/testReport)** for PR 30300 at commit [`f4fd12e`](https://github.com/apache/spark/commit/f4fd12e514b17d9e78f7e662df3c3fb502fd61d0).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726632814
**[Test build #131048 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131048/testReport)** for PR 30300 at commit [`c66874a`](https://github.com/apache/spark/commit/c66874a00545cb0b10a6eb533ce65bb71610fd18).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726632814
**[Test build #131048 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131048/testReport)** for PR 30300 at commit [`c66874a`](https://github.com/apache/spark/commit/c66874a00545cb0b10a6eb533ce65bb71610fd18).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r522077305
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -25,19 +25,15 @@ import org.apache.spark.sql.catalyst.plans.physical.{HashPartitioning, Partition
trait AliasAwareOutputExpression extends UnaryExecNode {
protected def outputExpressions: Seq[NamedExpression]
- protected def hasAlias: Boolean = outputExpressions.collectFirst { case _: Alias => }.isDefined
+ private lazy val aliasMap = AttributeMap(outputExpressions.collect {
+ case a @ Alias(child: AttributeReference, _) => (child, a.toAttribute)
+ })
- protected def replaceAliases(exprs: Seq[Expression]): Seq[Expression] = {
- exprs.map {
- case a: AttributeReference => replaceAlias(a).getOrElse(a)
- case other => other
- }
- }
+ protected def hasAlias: Boolean = aliasMap.nonEmpty
- protected def replaceAlias(attr: AttributeReference): Option[Attribute] = {
- outputExpressions.collectFirst {
- case a @ Alias(child: AttributeReference, _) if child.semanticEquals(attr) =>
- a.toAttribute
+ protected def normalizeExpression(exp: Expression): Expression = {
+ exp.transform {
+ case attr: AttributeReference => aliasMap.get(attr).getOrElse(attr)
Review comment:
nit: `aliasMap.getOrElse(attr)`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726341695
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724079456
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r521208639
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -48,13 +42,13 @@ trait AliasAwareOutputExpression extends UnaryExecNode {
*/
trait AliasAwareOutputPartitioning extends AliasAwareOutputExpression {
final override def outputPartitioning: Partitioning = {
- if (hasAlias) {
- child.outputPartitioning match {
- case h: HashPartitioning => h.copy(expressions = replaceAliases(h.expressions))
- case other => other
- }
- } else {
- child.outputPartitioning
+ child.outputPartitioning match {
+ case e: Expression if hasAlias =>
+ val normalizedExp = e.transformDown {
Review comment:
renaming -> done.
yes transform is needed here as each Partitioning can be a tree in itself. Example for INNER join, output partitioning is: PartitioningCollection with 2 HashPartitioning as childrens. Also in each HashPartitioning you can have expressions like 2*X, which might needs to be transformed to 2*aliasedX.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726590719
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727799605
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35739/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r522556996
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala
##########
@@ -895,6 +895,140 @@ class PlannerSuite extends SharedSparkSession with AdaptiveSparkPlanHelper {
}
}
+ test("SPARK-33399: aliases should be handled properly in PartitioningCollection output" +
+ " partitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id, t2.id as t2id
+ | FROM t1, t2
+ | WHERE t1.id = t2.id
+ |) t12, t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+ }
+ }
+ }
+
+ test("SPARK-33399: aliases should be handled properly in HashPartitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t1id, t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id
+ | FROM t1 LEFT SEMI JOIN t2
+ | ON t1.id = t2.id
+ |) t12 INNER JOIN t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+
+ val projects = planned.collect { case p: ProjectExec => p }
+ assert(projects.exists(_.outputPartitioning match {
+ case HashPartitioning(Seq(a: AttributeReference), _) => a.name == "t1id"
+ case _ => false
+ }))
+ }
+ }
+ }
+
+ test("SPARK-33399: alias handling should happen properly for RangePartitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ val df = spark.range(1, 100)
+ .select(col("id").as("id1")).groupBy("id1").count()
+ // Plan for this will be Range -> ProjectWithAlias -> HashAggregate -> HashAggregate
+ // if Project normalizes alias in its Range outputPartitioning, then no Exchange should come
+ // in between HashAggregates
+ val planned = df.queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.isEmpty)
+
+ val projects = planned.collect { case p: ProjectExec => p }
+ assert(projects.exists(_.outputPartitioning match {
+ case RangePartitioning(Seq(_@SortOrder(ar: AttributeReference, _, _, _)), _) =>
Review comment:
Could you add the same assert in the other tests, too?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724085011
cc - @cloud-fan @dongjoon-hyun @imback82
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724383172
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727812979
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726739511
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/131048/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning with respect to aliases to avoid unneeded exchanges
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-725436994
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727812953
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35739/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r540973357
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala
##########
@@ -895,6 +895,170 @@ class PlannerSuite extends SharedSparkSession with AdaptiveSparkPlanHelper {
}
}
+ test("SPARK-33399: aliases should be handled properly in PartitioningCollection output" +
+ " partitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id, t2.id as t2id
+ | FROM t1, t2
+ | WHERE t1.id = t2.id
+ |) t12, t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+
+ val projects = planned.collect { case p: ProjectExec => p }
+ assert(projects.exists(_.outputPartitioning match {
+ case PartitioningCollection(Seq(HashPartitioning(Seq(k1: AttributeReference), _),
+ HashPartitioning(Seq(k2: AttributeReference), _))) if k1.name == "t1id" =>
Review comment:
@maropu Sure. Basically the idea is to stop propagating partitionings and sortOrders corresponding to attributes which are not part of outputset?
Working on this as part of https://issues.apache.org/jira/browse/SPARK-33758.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning with respect to aliases to avoid unneeded exchanges
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r521354211
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -25,20 +25,14 @@ import org.apache.spark.sql.catalyst.plans.physical.{HashPartitioning, Partition
trait AliasAwareOutputExpression extends UnaryExecNode {
protected def outputExpressions: Seq[NamedExpression]
- protected def hasAlias: Boolean = outputExpressions.collectFirst { case _: Alias => }.isDefined
+ private lazy val aliasMap = AttributeMap(outputExpressions.collect {
+ case a @ Alias(child: AttributeReference, _) => (child, a.toAttribute)
+ })
- protected def replaceAliases(exprs: Seq[Expression]): Seq[Expression] = {
- exprs.map {
- case a: AttributeReference => replaceAlias(a).getOrElse(a)
- case other => other
- }
- }
+ protected def hasAlias: Boolean = aliasMap.nonEmpty
protected def replaceAlias(attr: AttributeReference): Option[Attribute] = {
- outputExpressions.collectFirst {
- case a @ Alias(child: AttributeReference, _) if child.semanticEquals(attr) =>
- a.toAttribute
- }
+ aliasMap.get(attr)
Review comment:
do we still need this `replaceAlias` method? It's very short and we can inline it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727855901
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727855878
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35745/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning with respect to aliases to avoid unneeded exchanges
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r521354211
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -25,20 +25,14 @@ import org.apache.spark.sql.catalyst.plans.physical.{HashPartitioning, Partition
trait AliasAwareOutputExpression extends UnaryExecNode {
protected def outputExpressions: Seq[NamedExpression]
- protected def hasAlias: Boolean = outputExpressions.collectFirst { case _: Alias => }.isDefined
+ private lazy val aliasMap = AttributeMap(outputExpressions.collect {
+ case a @ Alias(child: AttributeReference, _) => (child, a.toAttribute)
+ })
- protected def replaceAliases(exprs: Seq[Expression]): Seq[Expression] = {
- exprs.map {
- case a: AttributeReference => replaceAlias(a).getOrElse(a)
- case other => other
- }
- }
+ protected def hasAlias: Boolean = aliasMap.nonEmpty
protected def replaceAlias(attr: AttributeReference): Option[Attribute] = {
- outputExpressions.collectFirst {
- case a @ Alias(child: AttributeReference, _) if child.semanticEquals(attr) =>
- a.toAttribute
- }
+ aliasMap.get(attr)
Review comment:
do we still need this `replaceAlias` method? It's a very short method.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning with respect to aliases to avoid unneeded exchanges
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-725368648
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35531/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726163234
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724080451
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724357216
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727809847
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727817546
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727821362
**[Test build #131143 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131143/testReport)** for PR 30300 at commit [`16e1db2`](https://github.com/apache/spark/commit/16e1db202f3522ebe607894b93811bb54bdd9a0c).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727809847
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727855901
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727812979
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726625615
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726005146
**[Test build #130990 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130990/testReport)** for PR 30300 at commit [`d5a0fbe`](https://github.com/apache/spark/commit/d5a0fbe779a664f62e68a0d59463578461203afc).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r522717726
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -68,11 +65,8 @@ trait AliasAwareOutputOrdering extends AliasAwareOutputExpression {
final override def outputOrdering: Seq[SortOrder] = {
if (hasAlias) {
- orderingExpressions.map { s =>
- s.child match {
- case a: AttributeReference => s.copy(child = replaceAlias(a).getOrElse(a))
- case _ => s
- }
+ orderingExpressions.map { sortOrder =>
+ normalizeExpression(sortOrder).asInstanceOf[SortOrder]
}
Review comment:
done.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala
##########
@@ -895,6 +895,140 @@ class PlannerSuite extends SharedSparkSession with AdaptiveSparkPlanHelper {
}
}
+ test("SPARK-33399: aliases should be handled properly in PartitioningCollection output" +
+ " partitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id, t2.id as t2id
+ | FROM t1, t2
+ | WHERE t1.id = t2.id
+ |) t12, t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+ }
+ }
+ }
+
+ test("SPARK-33399: aliases should be handled properly in HashPartitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t1id, t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id
+ | FROM t1 LEFT SEMI JOIN t2
+ | ON t1.id = t2.id
+ |) t12 INNER JOIN t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+
+ val projects = planned.collect { case p: ProjectExec => p }
+ assert(projects.exists(_.outputPartitioning match {
+ case HashPartitioning(Seq(a: AttributeReference), _) => a.name == "t1id"
+ case _ => false
+ }))
+ }
+ }
+ }
+
+ test("SPARK-33399: alias handling should happen properly for RangePartitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ val df = spark.range(1, 100)
+ .select(col("id").as("id1")).groupBy("id1").count()
+ // Plan for this will be Range -> ProjectWithAlias -> HashAggregate -> HashAggregate
+ // if Project normalizes alias in its Range outputPartitioning, then no Exchange should come
+ // in between HashAggregates
+ val planned = df.queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.isEmpty)
+
+ val projects = planned.collect { case p: ProjectExec => p }
+ assert(projects.exists(_.outputPartitioning match {
+ case RangePartitioning(Seq(_@SortOrder(ar: AttributeReference, _, _, _)), _) =>
Review comment:
done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r521211796
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -25,20 +25,14 @@ import org.apache.spark.sql.catalyst.plans.physical.{HashPartitioning, Partition
trait AliasAwareOutputExpression extends UnaryExecNode {
protected def outputExpressions: Seq[NamedExpression]
- protected def hasAlias: Boolean = outputExpressions.collectFirst { case _: Alias => }.isDefined
+ lazy val aliasMap = AttributeMap(outputExpressions.collect {
Review comment:
> Also, could you use AliasHelper?
Covered in other comment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r520349461
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -48,13 +42,13 @@ trait AliasAwareOutputExpression extends UnaryExecNode {
*/
trait AliasAwareOutputPartitioning extends AliasAwareOutputExpression {
final override def outputPartitioning: Partitioning = {
- if (hasAlias) {
- child.outputPartitioning match {
- case h: HashPartitioning => h.copy(expressions = replaceAliases(h.expressions))
- case other => other
- }
- } else {
- child.outputPartitioning
+ child.outputPartitioning match {
+ case e: Expression if hasAlias =>
Review comment:
Why move `hasAlias` from outer block to pattern case condition? Doesn't it have more comparison?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r521206895
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -48,13 +42,13 @@ trait AliasAwareOutputExpression extends UnaryExecNode {
*/
trait AliasAwareOutputPartitioning extends AliasAwareOutputExpression {
final override def outputPartitioning: Partitioning = {
- if (hasAlias) {
- child.outputPartitioning match {
- case h: HashPartitioning => h.copy(expressions = replaceAliases(h.expressions))
- case other => other
- }
- } else {
- child.outputPartitioning
+ child.outputPartitioning match {
+ case e: Expression if hasAlias =>
Review comment:
Yes - moved it back to original place.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726590726
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/131040/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726590719
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726340731
**[Test build #131009 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131009/testReport)** for PR 30300 at commit [`f4fd12e`](https://github.com/apache/spark/commit/f4fd12e514b17d9e78f7e662df3c3fb502fd61d0).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `trait HasMaxBlockSizeInMB extends Params `
* `class HasMaxBlockSizeInMB(Params):`
* `case class ElementAt(`
* `case class GetArrayItem(`
* `case class Elt(`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r521208639
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -48,13 +42,13 @@ trait AliasAwareOutputExpression extends UnaryExecNode {
*/
trait AliasAwareOutputPartitioning extends AliasAwareOutputExpression {
final override def outputPartitioning: Partitioning = {
- if (hasAlias) {
- child.outputPartitioning match {
- case h: HashPartitioning => h.copy(expressions = replaceAliases(h.expressions))
- case other => other
- }
- } else {
- child.outputPartitioning
+ child.outputPartitioning match {
+ case e: Expression if hasAlias =>
+ val normalizedExp = e.transformDown {
Review comment:
renaming -> done.
yes transform is needed here as each Partitioning can be a tree in itself. Example for INNER join, output partitioning is: PartitioningCollection with 2 HashPartitioning as childrens. Also in each HashPartitioning you can have expressions like 2\*X, which might needs to be transformed to 2\*aliasedX.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
imback82 commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724235910
cc @maropu as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning with respect to aliases to avoid unneeded exchanges
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-725381027
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r540941139
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala
##########
@@ -895,6 +895,170 @@ class PlannerSuite extends SharedSparkSession with AdaptiveSparkPlanHelper {
}
}
+ test("SPARK-33399: aliases should be handled properly in PartitioningCollection output" +
+ " partitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id, t2.id as t2id
+ | FROM t1, t2
+ | WHERE t1.id = t2.id
+ |) t12, t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+
+ val projects = planned.collect { case p: ProjectExec => p }
+ assert(projects.exists(_.outputPartitioning match {
+ case PartitioningCollection(Seq(HashPartitioning(Seq(k1: AttributeReference), _),
+ HashPartitioning(Seq(k2: AttributeReference), _))) if k1.name == "t1id" =>
Review comment:
Oh, it looks interesting. Hi, @prakharjain09, are you interested in the improvement above?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726235329
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35615/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r522445512
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -68,11 +65,8 @@ trait AliasAwareOutputOrdering extends AliasAwareOutputExpression {
final override def outputOrdering: Seq[SortOrder] = {
if (hasAlias) {
- orderingExpressions.map { s =>
- s.child match {
- case a: AttributeReference => s.copy(child = replaceAlias(a).getOrElse(a))
- case _ => s
- }
+ orderingExpressions.map { sortOrder =>
+ normalizeExpression(sortOrder).asInstanceOf[SortOrder]
}
Review comment:
nit: `orderingExpressions.map(normalizeExpression(_).asInstanceOf[SortOrder])`
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala
##########
@@ -895,6 +895,140 @@ class PlannerSuite extends SharedSparkSession with AdaptiveSparkPlanHelper {
}
}
+ test("SPARK-33399: aliases should be handled properly in PartitioningCollection output" +
+ " partitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id, t2.id as t2id
+ | FROM t1, t2
+ | WHERE t1.id = t2.id
+ |) t12, t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+ }
+ }
+ }
+
+ test("SPARK-33399: aliases should be handled properly in HashPartitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t1id, t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id
+ | FROM t1 LEFT SEMI JOIN t2
+ | ON t1.id = t2.id
+ |) t12 INNER JOIN t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+
+ val projects = planned.collect { case p: ProjectExec => p }
+ assert(projects.exists(_.outputPartitioning match {
+ case HashPartitioning(Seq(a: AttributeReference), _) => a.name == "t1id"
+ case _ => false
+ }))
+ }
+ }
+ }
+
+ test("SPARK-33399: alias handling should happen properly for RangePartitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ val df = spark.range(1, 100)
+ .select(col("id").as("id1")).groupBy("id1").count()
+ // Plan for this will be Range -> ProjectWithAlias -> HashAggregate -> HashAggregate
+ // if Project normalizes alias in its Range outputPartitioning, then no Exchange should come
+ // in between HashAggregates
+ val planned = df.queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.isEmpty)
+
+ val projects = planned.collect { case p: ProjectExec => p }
+ assert(projects.exists(_.outputPartitioning match {
+ case RangePartitioning(Seq(_@SortOrder(ar: AttributeReference, _, _, _)), _) =>
Review comment:
nit: `case RangePartitioning(Seq(SortOrder(ar: AttributeReference, _, _, _)), _) =>`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726658021
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35654/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-728695366
Thanks @cloud-fan @maropu @imback82 @viirya for the code reviews and providing suggestions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727809856
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/131136/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726161572
**[Test build #130990 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130990/testReport)** for PR 30300 at commit [`d5a0fbe`](https://github.com/apache/spark/commit/d5a0fbe779a664f62e68a0d59463578461203afc).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724383159
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35420/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726602414
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724488715
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130810/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r522234988
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -25,19 +25,15 @@ import org.apache.spark.sql.catalyst.plans.physical.{HashPartitioning, Partition
trait AliasAwareOutputExpression extends UnaryExecNode {
protected def outputExpressions: Seq[NamedExpression]
- protected def hasAlias: Boolean = outputExpressions.collectFirst { case _: Alias => }.isDefined
+ private lazy val aliasMap = AttributeMap(outputExpressions.collect {
+ case a @ Alias(child: AttributeReference, _) => (child, a.toAttribute)
+ })
- protected def replaceAliases(exprs: Seq[Expression]): Seq[Expression] = {
- exprs.map {
- case a: AttributeReference => replaceAlias(a).getOrElse(a)
- case other => other
- }
- }
+ protected def hasAlias: Boolean = aliasMap.nonEmpty
- protected def replaceAlias(attr: AttributeReference): Option[Attribute] = {
- outputExpressions.collectFirst {
- case a @ Alias(child: AttributeReference, _) if child.semanticEquals(attr) =>
- a.toAttribute
+ protected def normalizeExpression(exp: Expression): Expression = {
+ exp.transform {
+ case attr: AttributeReference => aliasMap.get(attr).getOrElse(attr)
Review comment:
done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726041339
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35596/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r539460891
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala
##########
@@ -895,6 +895,170 @@ class PlannerSuite extends SharedSparkSession with AdaptiveSparkPlanHelper {
}
}
+ test("SPARK-33399: aliases should be handled properly in PartitioningCollection output" +
+ " partitioning") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withTempView("t1", "t2", "t3") {
+ spark.range(10).repartition($"id").createTempView("t1")
+ spark.range(20).repartition($"id").createTempView("t2")
+ spark.range(30).repartition($"id").createTempView("t3")
+ val planned = sql(
+ """
+ |SELECT t3.id as t3id
+ |FROM (
+ | SELECT t1.id as t1id, t2.id as t2id
+ | FROM t1, t2
+ | WHERE t1.id = t2.id
+ |) t12, t3
+ |WHERE t1id = t3.id
+ """.stripMargin).queryExecution.executedPlan
+ val exchanges = planned.collect { case s: ShuffleExchangeExec => s }
+ assert(exchanges.size == 3)
+
+ val projects = planned.collect { case p: ProjectExec => p }
+ assert(projects.exists(_.outputPartitioning match {
+ case PartitioningCollection(Seq(HashPartitioning(Seq(k1: AttributeReference), _),
+ HashPartitioning(Seq(k2: AttributeReference), _))) if k1.name == "t1id" =>
Review comment:
not related to this PR: The `ProjectExec` only outputs `t1id` (after column pruning), and it's a bit redundant to return `PartitioningCollection` here, as `t1id` is the only output and other partitionings are just invalid.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724360087
**[Test build #130810 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130810/testReport)** for PR 30300 at commit [`dd6b841`](https://github.com/apache/spark/commit/dd6b8411b9be52b2d6cd104e3c2fa408083b339e).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726030767
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35596/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r521211796
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -25,20 +25,14 @@ import org.apache.spark.sql.catalyst.plans.physical.{HashPartitioning, Partition
trait AliasAwareOutputExpression extends UnaryExecNode {
protected def outputExpressions: Seq[NamedExpression]
- protected def hasAlias: Boolean = outputExpressions.collectFirst { case _: Alias => }.isDefined
+ lazy val aliasMap = AttributeMap(outputExpressions.collect {
Review comment:
> Also, could you use AliasHelper?
Covered in other comment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726570795
**[Test build #131040 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131040/testReport)** for PR 30300 at commit [`c66874a`](https://github.com/apache/spark/commit/c66874a00545cb0b10a6eb533ce65bb71610fd18).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724486936
**[Test build #130810 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130810/testReport)** for PR 30300 at commit [`dd6b841`](https://github.com/apache/spark/commit/dd6b8411b9be52b2d6cd104e3c2fa408083b339e).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726738825
**[Test build #131048 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131048/testReport)** for PR 30300 at commit [`c66874a`](https://github.com/apache/spark/commit/c66874a00545cb0b10a6eb533ce65bb71610fd18).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726589366
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35646/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724375052
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35420/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726041363
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726602393
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35646/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] prakharjain09 commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
prakharjain09 commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r521209067
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -25,20 +25,14 @@ import org.apache.spark.sql.catalyst.plans.physical.{HashPartitioning, Partition
trait AliasAwareOutputExpression extends UnaryExecNode {
protected def outputExpressions: Seq[NamedExpression]
- protected def hasAlias: Boolean = outputExpressions.collectFirst { case _: Alias => }.isDefined
+ lazy val aliasMap = AttributeMap(outputExpressions.collect {
Review comment:
made this private.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726005146
**[Test build #130990 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130990/testReport)** for PR 30300 at commit [`d5a0fbe`](https://github.com/apache/spark/commit/d5a0fbe779a664f62e68a0d59463578461203afc).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726570795
**[Test build #131040 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131040/testReport)** for PR 30300 at commit [`c66874a`](https://github.com/apache/spark/commit/c66874a00545cb0b10a6eb533ce65bb71610fd18).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726341695
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-727809570
**[Test build #131136 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/131136/testReport)** for PR 30300 at commit [`16e1db2`](https://github.com/apache/spark/commit/16e1db202f3522ebe607894b93811bb54bdd9a0c).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` >>> class VectorAccumulatorParam(AccumulatorParam):`
* ` fully qualified classname of key Writable class (e.g. \"org.apache.hadoop.io.Text\")`
* ` fully qualified classname of key Writable class (e.g. \"org.apache.hadoop.io.Text\")`
* ` fully qualified classname of key Writable class (e.g. \"org.apache.hadoop.io.Text\")`
* ` fully qualified classname of key Writable class (e.g. \"org.apache.hadoop.io.Text\")`
* `class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with Logging `
* `abstract class AbstractSqlParser extends ParserInterface with Logging `
* `class CatalystSqlParser extends AbstractSqlParser `
* `class SparkSqlParser extends AbstractSqlParser `
* `class SparkSqlAstBuilder extends AstBuilder `
* `class VariableSubstitution `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724080451
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-724383176
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/35420/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-726163234
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #30300: [SPARK-33399][SQL] Normalize output partitioning of Project with respect to aliases
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30300:
URL: https://github.com/apache/spark/pull/30300#discussion_r520213847
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -48,13 +42,13 @@ trait AliasAwareOutputExpression extends UnaryExecNode {
*/
trait AliasAwareOutputPartitioning extends AliasAwareOutputExpression {
final override def outputPartitioning: Partitioning = {
- if (hasAlias) {
- child.outputPartitioning match {
- case h: HashPartitioning => h.copy(expressions = replaceAliases(h.expressions))
Review comment:
If we generalize this logic, I think its better to add tests for all the partitioning cases where possible.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -48,13 +42,13 @@ trait AliasAwareOutputExpression extends UnaryExecNode {
*/
trait AliasAwareOutputPartitioning extends AliasAwareOutputExpression {
final override def outputPartitioning: Partitioning = {
- if (hasAlias) {
- child.outputPartitioning match {
- case h: HashPartitioning => h.copy(expressions = replaceAliases(h.expressions))
- case other => other
- }
- } else {
- child.outputPartitioning
+ child.outputPartitioning match {
+ case e: Expression if hasAlias =>
+ val normalizedExp = e.transformDown {
Review comment:
nit: `normalizedExp` -> `normalizedExpr` and `transformDown` -> `transform`
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala
##########
@@ -895,6 +895,73 @@ class PlannerSuite extends SharedSparkSession with AdaptiveSparkPlanHelper {
}
}
+ test("No extra exchanges in case of [Inner Join -> Project with aliases -> Inner join]") {
+ withSQLConf(SQLConf.AUTO_BROADCASTJOIN_THRESHOLD.key -> "-1") {
+ withSQLConf(SQLConf.CONSTRAINT_PROPAGATION_ENABLED.key -> "false") {
Review comment:
Why we need to turn off the constant propagation?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -25,20 +25,14 @@ import org.apache.spark.sql.catalyst.plans.physical.{HashPartitioning, Partition
trait AliasAwareOutputExpression extends UnaryExecNode {
protected def outputExpressions: Seq[NamedExpression]
- protected def hasAlias: Boolean = outputExpressions.collectFirst { case _: Alias => }.isDefined
+ lazy val aliasMap = AttributeMap(outputExpressions.collect {
+ case a @ Alias(child: AttributeReference, _) => (child, a.toAttribute)
+ })
- protected def replaceAliases(exprs: Seq[Expression]): Seq[Expression] = {
- exprs.map {
- case a: AttributeReference => replaceAlias(a).getOrElse(a)
- case other => other
- }
- }
+ protected def hasAlias: Boolean = aliasMap.nonEmpty
protected def replaceAlias(attr: AttributeReference): Option[Attribute] = {
Review comment:
Looks `AliasHelper` has the similar func and could you check if we can reuse it?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -25,20 +25,14 @@ import org.apache.spark.sql.catalyst.plans.physical.{HashPartitioning, Partition
trait AliasAwareOutputExpression extends UnaryExecNode {
protected def outputExpressions: Seq[NamedExpression]
- protected def hasAlias: Boolean = outputExpressions.collectFirst { case _: Alias => }.isDefined
+ lazy val aliasMap = AttributeMap(outputExpressions.collect {
Review comment:
`private`?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -25,20 +25,14 @@ import org.apache.spark.sql.catalyst.plans.physical.{HashPartitioning, Partition
trait AliasAwareOutputExpression extends UnaryExecNode {
protected def outputExpressions: Seq[NamedExpression]
- protected def hasAlias: Boolean = outputExpressions.collectFirst { case _: Alias => }.isDefined
+ lazy val aliasMap = AttributeMap(outputExpressions.collect {
Review comment:
Also, could you use `AliasHelper`?
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/AliasAwareOutputExpression.scala
##########
@@ -48,13 +42,13 @@ trait AliasAwareOutputExpression extends UnaryExecNode {
*/
trait AliasAwareOutputPartitioning extends AliasAwareOutputExpression {
final override def outputPartitioning: Partitioning = {
- if (hasAlias) {
- child.outputPartitioning match {
- case h: HashPartitioning => h.copy(expressions = replaceAliases(h.expressions))
- case other => other
- }
- } else {
- child.outputPartitioning
+ child.outputPartitioning match {
+ case e: Expression if hasAlias =>
+ val normalizedExp = e.transformDown {
Review comment:
btw, we need `transform` here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-729405241
NOTE: It seems this update makes TPCDS(sf=20) q95 much faster (176324ms->129644ms). Nice.
https://docs.google.com/spreadsheets/d/1V8xoKR9ElU-rOXMH84gb5BbLEw0XAPTJY8c8aZeIqus/edit?usp=sharing
--
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on pull request #30300: [SPARK-33399][SQL] Normalize output partitioning and sortorder with respect to aliases to avoid unneeded exchange/sort nodes
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #30300:
URL: https://github.com/apache/spark/pull/30300#issuecomment-728629017
Thanks! Merged to master.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org