[SPARK-32635][SQL] Fix foldable propagation #29771

peter-toth · 2020-09-16T11:37:38Z

What changes were proposed in this pull request?

This PR rewrites FoldablePropagation rule to replace attribute references in a node with foldables coming only from the node's children.

Before this PR in the case of this example (with settingspark.sql.optimizer.excludedRules=org.apache.spark.sql.catalyst.optimizer.ConvertToLocalRelation):

val a = Seq("1").toDF("col1").withColumn("col2", lit("1"))
val b = Seq("2").toDF("col1").withColumn("col2", lit("2"))
val aub = a.union(b)
val c = aub.filter($"col1" === "2").cache()
val d = Seq("2").toDF( "col4")
val r = d.join(aub, $"col2" === $"col4").select("col4")
val l = c.select("col2")
val df = l.join(r, $"col2" === $"col4", "LeftOuter")
df.show()

foldable propagation happens incorrectly:

 Join LeftOuter, (col2#6 = col4#34)                                                              Join LeftOuter, (col2#6 = col4#34)
!:- Project [col2#6]                                                                             :- Project [1 AS col2#6]
 :  +- InMemoryRelation [col1#4, col2#6], StorageLevel(disk, memory, deserialized, 1 replicas)   :  +- InMemoryRelation [col1#4, col2#6], StorageLevel(disk, memory, deserialized, 1 replicas)
 :        +- Union                                                                               :        +- Union
 :           :- *(1) Project [value#1 AS col1#4, 1 AS col2#6]                                    :           :- *(1) Project [value#1 AS col1#4, 1 AS col2#6]
 :           :  +- *(1) Filter (isnotnull(value#1) AND (value#1 = 2))                            :           :  +- *(1) Filter (isnotnull(value#1) AND (value#1 = 2))
 :           :     +- *(1) LocalTableScan [value#1]                                              :           :     +- *(1) LocalTableScan [value#1]
 :           +- *(2) Project [value#10 AS col1#13, 2 AS col2#15]                                 :           +- *(2) Project [value#10 AS col1#13, 2 AS col2#15]
 :              +- *(2) Filter (isnotnull(value#10) AND (value#10 = 2))                          :              +- *(2) Filter (isnotnull(value#10) AND (value#10 = 2))
 :                 +- *(2) LocalTableScan [value#10]                                             :                 +- *(2) LocalTableScan [value#10]
 +- Project [col4#34]                                                                            +- Project [col4#34]
    +- Join Inner, (col2#6 = col4#34)                                                               +- Join Inner, (col2#6 = col4#34)
       :- Project [value#31 AS col4#34]                                                                :- Project [value#31 AS col4#34]
       :  +- LocalRelation [value#31]                                                                  :  +- LocalRelation [value#31]
       +- Project [col2#6]                                                                             +- Project [col2#6]
          +- Union false, false                                                                           +- Union false, false
             :- Project [1 AS col2#6]                                                                        :- Project [1 AS col2#6]
             :  +- LocalRelation [value#1]                                                                   :  +- LocalRelation [value#1]
             +- Project [2 AS col2#15]                                                                       +- Project [2 AS col2#15]
                +- LocalRelation [value#10]                                                                     +- LocalRelation [value#10]

and so the result is wrong:

+----+----+
|col2|col4|
+----+----+
|   1|null|
+----+----+

After this PR foldable propagation will not happen incorrectly and the result is correct:

+----+----+
|col2|col4|
+----+----+
|   2|   2|
+----+----+

Why are the changes needed?

To fix a correctness issue.

Does this PR introduce any user-facing change?

Yes, fixes a correctness issue.

How was this patch tested?

Existing and new UTs.

SparkQA · 2020-09-16T18:05:22Z

Test build #128764 has finished for PR 29771 at commit bc433e8.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

peter-toth · 2020-09-16T18:43:07Z

cc @cloud-fan, @maropu, @viirya

sql/catalyst/src/main/scala-2.12/org/apache/spark/sql/catalyst/expressions/AttributeMap.scala

cloud-fan

good catch!

maropu · 2020-09-17T06:53:48Z

sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala

@@ -2555,6 +2555,18 @@ class DataFrameSuite extends QueryTest
    val df = Seq(0.0 -> -0.0).toDF("pos", "neg")
    checkAnswer(df.select($"pos" > $"neg"), Row(false))
  }
+
+  test("SPARK-32635: Fix foldable propagation") {


Could you make the title clearer? e.g., Replace references with foldables coming only from the node's children

Sure, fixed.

maropu · 2020-09-17T06:58:41Z

Looks ok otherwise. Nice fix.

peter-toth · 2020-09-17T09:10:29Z

last commit is just a minor improvement

rednaxelafx · 2020-09-17T09:25:25Z

LGTM (not a Reviewer). Eliminating var FTW!

I'd like to ask about a few things though:

The fix involved a bit of refactoring. Is the bug caused by "leaking" the foldableMap from a sibling node, such that it's not properly just propagating along the child->parent flow? And that the refactoring eliminated the var the threaded the propagation via returns in a recursive depth-first traversal to make sure that it's always only propagating along the child->parent flow, and otherwise the existing logic is unchanged? If so, this looks like a nice refactoring.
TreeNode's transformXXX functions take care of the case where a new node is actually equal to the old one, so that after transformation the logically unchanged nodes will stay with their original identity. It looks like with the explicit recursion after this PR, that it might introduce more, unnecessary copies of the logically unchanged nodes that end up staying in the transformed tree. Is that the case? If so, can we make it better?

Thanks!

SparkQA · 2020-09-17T11:08:41Z

Test build #128812 has finished for PR 29771 at commit 5f7bbfb.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-09-17T11:16:24Z

that it might introduce more, unnecessary copies of the logically unchanged nodes

Good point. mapChildren takes care of it and we should always use mapChildren instead of node.copy(child = newChild)

peter-toth · 2020-09-17T11:45:12Z

The fix involved a bit of refactoring. Is the bug caused by "leaking" the foldableMap from a sibling node, such that it's not properly just propagating along the child->parent flow? And that the refactoring eliminated the var the threaded the propagation via returns in a recursive depth-first traversal to make sure that it's always only propagating along the child->parent flow, and otherwise the existing logic is unchanged? If so, this looks like a nice refactoring.

This is exactly what happens without this PR. Basically foldableMap is filled up with all foldables in the beginning and then we start the preorder traversal and replacing references from the map. Sometimes we narrow down the map when traversing through particular nodes, but this is still incorrect.
With this PR we do a proper fill-up and child->parent propagation of the map.

TreeNode's transformXXX functions take care of the case where a new node is actually equal to the old one, so that after transformation the logically unchanged nodes will stay with their original identity. It looks like with the explicit recursion after this PR, that it might introduce more, unnecessary copies of the logically unchanged nodes that end up staying in the transformed tree. Is that the case? If so, can we make it better?

I think this is a very good point actually. mapChildren() in the "other" case seems to be ok as that method takes care of the comparison. withNewChildren() also takes care of it in UnaryNode and Join. The replaceFoldable() call after withNewChildren() is also ok because it uses transformExpressions. Finally Project and Expand, well, they can add unnecessary copies as you pointed out. But I think if there is a parent "other"/UnaryNode/Join node above these 2 in the tree then there won't be any copies in the transformed tree in the end. But let me change the copy() to avoid issues in other cases, though.

peter-toth · 2020-09-17T13:04:17Z

Thanks @rednaxelafx, latest commit contains the fix.

SparkQA · 2020-09-17T14:20:54Z

Test build #128808 has finished for PR 29771 at commit 55575de.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-09-17T14:52:55Z

Test build #128810 has finished for PR 29771 at commit 0c0a115.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2020-09-17T16:03:34Z

sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala

+    val b = Seq("2").toDF("col1").withColumn("col2", lit("2"))
+    val aub = a.union(b)
+    val c = aub.filter($"col1" === "2").cache()
+    val d = Seq("2").toDF( "col4")


nit: extra space before "col4"

Thanks, fixed.

viirya

good catch!

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

gatorsmile · 2020-09-17T16:50:26Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

@@ -624,67 +624,89 @@ object NullPropagation extends Rule[LogicalPlan] {
 */
 object FoldablePropagation extends Rule[LogicalPlan] {
  def apply(plan: LogicalPlan): LogicalPlan = {
-    var foldableMap = AttributeMap(plan.flatMap {
-      case Project(projectList, _) => projectList.collect {


How about Aggregate? Why we are not handling Aggregate in the original rule?

This ticket and PR just focuses on the correctness issue found. But I think we can extend the rule to handle Aggregate in another ticket if that is ok with you.

I had the same question when I first looked at the PR, but since this PR might need to be hotfixed to released branches, it's a good idea to keep it simple and don't add any new features. So I'm +1 for handling Aggregate in a separate PR.

+1 for @peter-toth and @rednaxelafx 's opinion.

dongjoon-hyun · 2020-09-17T18:24:45Z

sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala

@@ -2555,6 +2555,18 @@ class DataFrameSuite extends QueryTest
    val df = Seq(0.0 -> -0.0).toDF("pos", "neg")
    checkAnswer(df.select($"pos" > $"neg"), Row(false))
  }
+
+  test("SPARK-32635: Replace references with foldables coming only from the node's children") {


Although this end-to-end test is also useful, we had better have a narrow-downed test coverage at FoldablePropagationSuite in catalyst module, too. Could you add one please? If then, we can easily detect regression in catalyst module test instead of running full sql tests.

Ok, I will try to add a test case to FoldablePropagationSuite tomorrow.

dongjoon-hyun

+1, LGTM. Thanks, @peter-toth and all.
I guess most comments are already addressed. I have only one minor test related comment.

dongjoon-hyun · 2020-09-17T18:30:53Z

Ur, @peter-toth . Could you check the PR description once more?

scala> spark.version
res1: String = 3.0.1

scala> :paste
// Entering paste mode (ctrl-D to finish)

val a = Seq("1").toDF("col1").withColumn("col2", lit("1"))
val b = Seq("2").toDF("col1").withColumn("col2", lit("2"))
val aub = a.union(b)
val c = aub.filter($"col1" === "2").cache()
val d = Seq("2").toDF( "col4")
val r = d.join(aub, $"col2" === $"col4").select("col4")
val l = c.select("col2")
val df = l.join(r, $"col2" === $"col4", "LeftOuter")
df.show()

// Exiting paste mode, now interpreting.

+----+----+
|col2|col4|
+----+----+
|   2|   2|
+----+----+

scala> spark.version
res0: String = 2.4.7

scala> :paste
// Entering paste mode (ctrl-D to finish)

val a = Seq("1").toDF("col1").withColumn("col2", lit("1"))
val b = Seq("2").toDF("col1").withColumn("col2", lit("2"))
val aub = a.union(b)
val c = aub.filter($"col1" === "2").cache()
val d = Seq("2").toDF( "col4")
val r = d.join(aub, $"col2" === $"col4").select("col4")
val l = c.select("col2")
val df = l.join(r, $"col2" === $"col4", "LeftOuter")
df.show()

// Exiting paste mode, now interpreting.

+----+----+
|col2|col4|
+----+----+
|   2|   2|
+----+----+

peter-toth · 2020-09-17T18:44:05Z

Ur, @peter-toth . Could you check the PR description once more?

@dongjoon-hyun, please run the shell with --conf "spark.sql.optimizer.excludedRules=org.apache.spark.sql.catalyst.optimizer.ConvertToLocalRelation" as that test suite does.

dongjoon-hyun · 2020-09-17T18:44:28Z

Got it. Thanks, @peter-toth ! Please add that conf into PR description and JIRA description.

SparkQA · 2020-09-17T19:59:53Z

Test build #128825 has finished for PR 29771 at commit 54f41fa.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-09-17T21:22:30Z

Test build #128835 has finished for PR 29771 at commit 75d7a4c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-09-17T22:19:11Z

Test build #128837 has finished for PR 29771 at commit 61548c7.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2020-09-17T23:12:47Z

Nice suggestion, @rednaxelafx ! I checked the latest commits again and they looks okay.

### What changes were proposed in this pull request? This PR rewrites `FoldablePropagation` rule to replace attribute references in a node with foldables coming only from the node's children. Before this PR in the case of this example (with setting`spark.sql.optimizer.excludedRules=org.apache.spark.sql.catalyst.optimizer.ConvertToLocalRelation`): ```scala val a = Seq("1").toDF("col1").withColumn("col2", lit("1")) val b = Seq("2").toDF("col1").withColumn("col2", lit("2")) val aub = a.union(b) val c = aub.filter($"col1" === "2").cache() val d = Seq("2").toDF( "col4") val r = d.join(aub, $"col2" === $"col4").select("col4") val l = c.select("col2") val df = l.join(r, $"col2" === $"col4", "LeftOuter") df.show() ``` foldable propagation happens incorrectly: ``` Join LeftOuter, (col2#6 = col4#34) Join LeftOuter, (col2#6 = col4#34) !:- Project [col2#6] :- Project [1 AS col2#6] : +- InMemoryRelation [col1#4, col2#6], StorageLevel(disk, memory, deserialized, 1 replicas) : +- InMemoryRelation [col1#4, col2#6], StorageLevel(disk, memory, deserialized, 1 replicas) : +- Union : +- Union : :- *(1) Project [value#1 AS col1#4, 1 AS col2#6] : :- *(1) Project [value#1 AS col1#4, 1 AS col2#6] : : +- *(1) Filter (isnotnull(value#1) AND (value#1 = 2)) : : +- *(1) Filter (isnotnull(value#1) AND (value#1 = 2)) : : +- *(1) LocalTableScan [value#1] : : +- *(1) LocalTableScan [value#1] : +- *(2) Project [value#10 AS col1#13, 2 AS col2#15] : +- *(2) Project [value#10 AS col1#13, 2 AS col2#15] : +- *(2) Filter (isnotnull(value#10) AND (value#10 = 2)) : +- *(2) Filter (isnotnull(value#10) AND (value#10 = 2)) : +- *(2) LocalTableScan [value#10] : +- *(2) LocalTableScan [value#10] +- Project [col4#34] +- Project [col4#34] +- Join Inner, (col2#6 = col4#34) +- Join Inner, (col2#6 = col4#34) :- Project [value#31 AS col4#34] :- Project [value#31 AS col4#34] : +- LocalRelation [value#31] : +- LocalRelation [value#31] +- Project [col2#6] +- Project [col2#6] +- Union false, false +- Union false, false :- Project [1 AS col2#6] :- Project [1 AS col2#6] : +- LocalRelation [value#1] : +- LocalRelation [value#1] +- Project [2 AS col2#15] +- Project [2 AS col2#15] +- LocalRelation [value#10] +- LocalRelation [value#10] ``` and so the result is wrong: ``` +----+----+ |col2|col4| +----+----+ | 1|null| +----+----+ ``` After this PR foldable propagation will not happen incorrectly and the result is correct: ``` +----+----+ |col2|col4| +----+----+ | 2| 2| +----+----+ ``` ### Why are the changes needed? To fix a correctness issue. ### Does this PR introduce _any_ user-facing change? Yes, fixes a correctness issue. ### How was this patch tested? Existing and new UTs. Closes #29771 from peter-toth/SPARK-32635-fix-foldable-propagation. Authored-by: Peter Toth <peter.toth@gmail.com> Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org> (cherry picked from commit 4ced588) Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>

maropu · 2020-09-17T23:19:22Z

Thanks! Merged to master/3.0. Could you backport it into branch-2.4, @peter-toth ?

maropu · 2020-09-17T23:21:48Z

Also, do you want to make a PR for the @gatorsmile comment above #29771 (comment)?

dongjoon-hyun · 2020-09-18T01:48:38Z

@maropu and @peter-toth . Please don't forget #29771 (comment) . That should be backported together.

maropu · 2020-09-18T01:50:31Z

Oh, I missed it. Could you fix it first in follow-up @peter-toth ?

peter-toth · 2020-09-18T08:43:21Z

Thanks all for the review!

I've opened a follow-up PR: #29802 to add a new test case to FoldablePropagationSuite.

I will open a 2.4 backport PR soon and I'm happy to extend propagation to Aggregate (#29771 (comment)) as well in a separate ticket.

### What changes were proposed in this pull request? This is a follow-up PR to #29771 and just adds a new test case. ### Why are the changes needed? To have better test coverage. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? New UT. Closes #29802 from peter-toth/SPARK-32635-fix-foldable-propagation-followup. Authored-by: Peter Toth <peter.toth@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

### What changes were proposed in this pull request? This is a follow-up PR to #29771 and just adds a new test case. ### Why are the changes needed? To have better test coverage. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? New UT. Closes #29802 from peter-toth/SPARK-32635-fix-foldable-propagation-followup. Authored-by: Peter Toth <peter.toth@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit 3309a2b) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

HyukjinKwon · 2020-09-20T01:42:19Z

Nice @peter-toth, LGTM too! cc @vinodkc FYI

### What changes were proposed in this pull request? This PR adds foldable propagation from `Aggregate` as per: #29771 (comment) ### Why are the changes needed? This is an improvement as `Aggregate`'s `aggregateExpressions` can contain foldables that can be propagated up. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? New UT. Closes #29816 from peter-toth/SPARK-32951-foldable-propagation-from-aggregate. Authored-by: Peter Toth <peter.toth@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

### What changes were proposed in this pull request? This PR rewrites `FoldablePropagation` rule to replace attribute references in a node with foldables coming only from the node's children. Before this PR in the case of this example (with setting`spark.sql.optimizer.excludedRules=org.apache.spark.sql.catalyst.optimizer.ConvertToLocalRelation`): ```scala val a = Seq("1").toDF("col1").withColumn("col2", lit("1")) val b = Seq("2").toDF("col1").withColumn("col2", lit("2")) val aub = a.union(b) val c = aub.filter($"col1" === "2").cache() val d = Seq("2").toDF( "col4") val r = d.join(aub, $"col2" === $"col4").select("col4") val l = c.select("col2") val df = l.join(r, $"col2" === $"col4", "LeftOuter") df.show() ``` foldable propagation happens incorrectly: ``` Join LeftOuter, (col2#6 = col4#34) Join LeftOuter, (col2#6 = col4#34) !:- Project [col2#6] :- Project [1 AS col2#6] : +- InMemoryRelation [col1#4, col2#6], StorageLevel(disk, memory, deserialized, 1 replicas) : +- InMemoryRelation [col1#4, col2#6], StorageLevel(disk, memory, deserialized, 1 replicas) : +- Union : +- Union : :- *(1) Project [value#1 AS col1#4, 1 AS col2#6] : :- *(1) Project [value#1 AS col1#4, 1 AS col2#6] : : +- *(1) Filter (isnotnull(value#1) AND (value#1 = 2)) : : +- *(1) Filter (isnotnull(value#1) AND (value#1 = 2)) : : +- *(1) LocalTableScan [value#1] : : +- *(1) LocalTableScan [value#1] : +- *(2) Project [value#10 AS col1#13, 2 AS col2#15] : +- *(2) Project [value#10 AS col1#13, 2 AS col2#15] : +- *(2) Filter (isnotnull(value#10) AND (value#10 = 2)) : +- *(2) Filter (isnotnull(value#10) AND (value#10 = 2)) : +- *(2) LocalTableScan [value#10] : +- *(2) LocalTableScan [value#10] +- Project [col4#34] +- Project [col4#34] +- Join Inner, (col2#6 = col4#34) +- Join Inner, (col2#6 = col4#34) :- Project [value#31 AS col4#34] :- Project [value#31 AS col4#34] : +- LocalRelation [value#31] : +- LocalRelation [value#31] +- Project [col2#6] +- Project [col2#6] +- Union false, false +- Union false, false :- Project [1 AS col2#6] :- Project [1 AS col2#6] : +- LocalRelation [value#1] : +- LocalRelation [value#1] +- Project [2 AS col2#15] +- Project [2 AS col2#15] +- LocalRelation [value#10] +- LocalRelation [value#10] ``` and so the result is wrong: ``` +----+----+ |col2|col4| +----+----+ | 1|null| +----+----+ ``` After this PR foldable propagation will not happen incorrectly and the result is correct: ``` +----+----+ |col2|col4| +----+----+ | 2| 2| +----+----+ ``` ### Why are the changes needed? To fix a correctness issue. ### Does this PR introduce _any_ user-facing change? Yes, fixes a correctness issue. ### How was this patch tested? Existing and new UTs. Closes apache#29771 from peter-toth/SPARK-32635-fix-foldable-propagation. Authored-by: Peter Toth <peter.toth@gmail.com> Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org> (cherry picked from commit 4ced588) Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>

### What changes were proposed in this pull request? This is a follow-up PR to apache#29771 and just adds a new test case. ### Why are the changes needed? To have better test coverage. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? New UT. Closes apache#29802 from peter-toth/SPARK-32635-fix-foldable-propagation-followup. Authored-by: Peter Toth <peter.toth@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit 3309a2b) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

[SPARK-32635][SQL] Fix foldable propagation

bc433e8

probot-autolabeler bot added the SQL label Sep 16, 2020

cloud-fan reviewed Sep 17, 2020

View reviewed changes

sql/catalyst/src/main/scala-2.12/org/apache/spark/sql/catalyst/expressions/AttributeMap.scala Show resolved Hide resolved

cloud-fan approved these changes Sep 17, 2020

View reviewed changes

maropu reviewed Sep 17, 2020

View reviewed changes

fix review findings

55575de

maropu approved these changes Sep 17, 2020

View reviewed changes

minor improvement

5f7bbfb

peter-toth force-pushed the SPARK-32635-fix-foldable-propagation branch from 0c0a115 to 5f7bbfb Compare September 17, 2020 09:07

avoid copy with new children

54f41fa

viirya reviewed Sep 17, 2020

View reviewed changes

viirya approved these changes Sep 17, 2020

View reviewed changes

remove extra space

75d7a4c

gatorsmile reviewed Sep 17, 2020

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala Show resolved Hide resolved

gatorsmile reviewed Sep 17, 2020

View reviewed changes

comment about foldable propagation in Project

61548c7

dongjoon-hyun reviewed Sep 17, 2020

View reviewed changes

dongjoon-hyun approved these changes Sep 17, 2020

View reviewed changes

maropu closed this in 4ced588 Sep 17, 2020

peter-toth mentioned this pull request Sep 18, 2020

[SPARK-32635][SQL][FOLLOW-UP] Add a new test case in catalyst module #29802

Closed

peter-toth mentioned this pull request Sep 18, 2020

[SPARK-32635][SQL][2.4] Fix foldable propagation #29805

Closed

peter-toth mentioned this pull request Sep 21, 2020

[SPARK-32951][SQL] Foldable propagation from Aggregate #29816

Closed

[SPARK-32635][SQL] Fix foldable propagation #29771

[SPARK-32635][SQL] Fix foldable propagation #29771

Conversation

peter-toth commented Sep 16, 2020 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

SparkQA commented Sep 16, 2020

peter-toth commented Sep 16, 2020 • edited Loading

cloud-fan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maropu commented Sep 17, 2020

peter-toth commented Sep 17, 2020

rednaxelafx commented Sep 17, 2020

SparkQA commented Sep 17, 2020

cloud-fan commented Sep 17, 2020

peter-toth commented Sep 17, 2020 • edited Loading

peter-toth commented Sep 17, 2020

SparkQA commented Sep 17, 2020

SparkQA commented Sep 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viirya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dongjoon-hyun Sep 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dongjoon-hyun left a comment

Choose a reason for hiding this comment

dongjoon-hyun commented Sep 17, 2020 • edited Loading

peter-toth commented Sep 17, 2020 • edited Loading

dongjoon-hyun commented Sep 17, 2020 • edited Loading

SparkQA commented Sep 17, 2020

SparkQA commented Sep 17, 2020

SparkQA commented Sep 17, 2020

maropu commented Sep 17, 2020

maropu commented Sep 17, 2020

maropu commented Sep 17, 2020

dongjoon-hyun commented Sep 18, 2020

maropu commented Sep 18, 2020

peter-toth commented Sep 18, 2020

HyukjinKwon commented Sep 20, 2020

peter-toth commented Sep 16, 2020 •

edited

Loading

peter-toth commented Sep 16, 2020 •

edited

Loading

peter-toth commented Sep 17, 2020 •

edited

Loading

dongjoon-hyun Sep 17, 2020 •

edited

Loading

dongjoon-hyun commented Sep 17, 2020 •

edited

Loading

peter-toth commented Sep 17, 2020 •

edited

Loading

dongjoon-hyun commented Sep 17, 2020 •

edited

Loading