[SPARK-27604][SQL] Enhance constant propagation #24553

peter-toth · 2019-05-08T07:02:25Z

What changes were proposed in this pull request?

This PR improves ConstantPropagation rule, the new implementation:

Collects constants into a map directly, instead of a list that is then transformed into a map.

Supports substituting in conflicting equalities:
e.g. a = 1 AND a = 2 => a = 1 AND 1 = 2
Before this PR this unsatisfiable filter was not reduced:

== Physical Plan ==
*(1) Project [value#219 AS a#222]
+- *(1) Filter ((isnotnull(value#219) AND (value#219 = 1)) AND (value#219 = 2))
   +- *(1) LocalTableScan [value#219]

But after this PR it is:

== Physical Plan ==
LocalTableScan <empty>, [a#222]

Extends propagation from attribute => constant mapping to deterministic expression => constant mapping:
e.g. abs(a) = 5 AND b = abs(a) + 3 => abs(a) = 5 AND b = 8
Allows substitution in other than EqualTo and EqualNullSafe expressions:
e.g. a = 5 AND b < a + 3 => a = 5 AND b < 8
Allows propagation of constant non-nullable expressions in Project and other nodes (except for Join):
e.g. SELECT a = 5 AND b = a + 3 => SELECT a = 5 AND b = 8
Before this PR only Filter was supported.
Allows deep constant propagation:
e.g. ... WHERE IF(..., a = 5 AND b = a + 3, ...) => ... WHERE IF(..., a = 5 AND b = 8, ...)
Before this PR only top level And/Or/Not nodes were supported.
During expression tree traversal tracks a boolean context that controls if constant propagation of a nullable expression can be safely applied.
- E.g. in the case of ... WHERE a = c AND f(a) or IF(a = c AND f(a), ..., ...) where a is a nullable expression and c is a constant the null result of a = c AND f(a) means the same as if it resulted false therefore constant propagation can be safely applied (a = c AND f(a) => a = c AND f(c)).
- In the case of SELECT a = c AND f(a) the null result really means null. In this context constant propagation can't be applied safely.
- There is also a 3rd context due to an enclosing Not in which the context flips. E.g. constant propagation can't be applied on WHERE NOT(a = c AND f(a)) but can be again on WHERE NOT(IF(..., NOT(a = c AND f(a)), ...).

Why are the changes needed?

Improve performance.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

New UTs.

peter-toth · 2019-05-08T07:06:28Z

This PR is extracted from #24495 as @dongjoon-hyun suggested to make review easier.
Please note that new UTs added in #24495 depend on this change.

mgaido91

@peter-toth may you please also add some UTs? eg. the example you added in the scaladoc may be also formalized with a UT... thanks.

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

peter-toth · 2019-05-08T13:27:12Z

@mgaido91, I've moved the relevant UTs from the other PR here.

dongjoon-hyun · 2019-05-08T18:35:35Z

ok to test

HyukjinKwon · 2019-05-09T04:56:45Z

ok to test

SparkQA · 2019-05-09T07:05:01Z

Test build #105274 has finished for PR 24553 at commit 7a29f09.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

mgaido91 · 2019-05-09T08:12:43Z

retest this please

...core/src/test/scala/org/apache/spark/sql/execution/datasources/FileSourceStrategySuite.scala

SparkQA · 2019-05-09T10:16:13Z

Test build #105275 has finished for PR 24553 at commit 7a29f09.

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

SparkQA · 2019-05-09T12:06:02Z

Test build #105278 has finished for PR 24553 at commit 96e54a7.

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

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

SparkQA · 2019-05-09T15:23:48Z

Test build #105283 has finished for PR 24553 at commit 8be22b6.

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

peter-toth · 2019-05-13T12:46:34Z

Any comments or suggestions @dongjoon-hyun or @gatorsmile ?

peter-toth · 2019-05-23T20:44:30Z

@dongjoon-hyun @gatorsmile @mgaido91 @viirya @HyukjinKwon any feedback or suggestion is very welcome.

mgaido91

I think the change makes sense in general, just some questions on the tests. Thanks @peter-toth

...core/src/test/scala/org/apache/spark/sql/execution/datasources/FileSourceStrategySuite.scala

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ConstantPropagationSuite.scala

peter-toth · 2019-05-24T19:49:30Z

Thanks so much @mgaido91 for the review. Please let me know if you have any more questions.

SparkQA · 2019-05-24T22:49:02Z

Test build #105769 has finished for PR 24553 at commit 6bccd84.

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

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ConstantPropagationSuite.scala

...core/src/test/scala/org/apache/spark/sql/execution/datasources/FileSourceStrategySuite.scala

SparkQA · 2019-05-25T16:03:56Z

Test build #105780 has finished for PR 24553 at commit 827cfe5.

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

mgaido91 · 2019-05-25T16:42:04Z

LGTM, thanks!

peter-toth · 2019-05-31T17:01:06Z

@dongjoon-hyun @gatorsmile @cloud-fan @viirya @HyukjinKwon could you please help me to review this PR?

HyukjinKwon

From a cursory look, seems making sense but I guess @hvanhovell should take a look as well.

HyukjinKwon · 2019-06-14T00:49:21Z

retest this please

SparkQA · 2019-06-14T03:58:01Z

Test build #106493 has finished for PR 24553 at commit 827cfe5.

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

SparkQA · 2019-06-22T19:31:05Z

Test build #106794 has finished for PR 24553 at commit 054624e.

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

peter-toth · 2019-12-26T08:55:19Z

retest this please

peter-toth · 2020-01-14T18:28:32Z

retest this please

SparkQA · 2020-01-14T20:17:28Z

Test build #116717 has finished for PR 24553 at commit 39c563d.

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

SparkQA · 2020-01-14T22:35:24Z

Test build #116721 has finished for PR 24553 at commit 39c563d.

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

SparkQA · 2020-01-15T22:58:59Z

Test build #116792 has finished for PR 24553 at commit 85d5430.

This patch fails from timeout after a configured wait of 400m.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-01-16T20:07:44Z

Test build #116861 has finished for PR 24553 at commit 0c227c3.

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

SparkQA · 2020-01-16T21:15:46Z

Test build #116866 has finished for PR 24553 at commit 48802b6.

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

SparkQA · 2020-01-18T18:36:35Z

Test build #116980 has finished for PR 24553 at commit 4f0a89a.

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

SparkQA · 2020-01-24T16:55:14Z

Test build #117354 has finished for PR 24553 at commit 5d67ffd.

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

SparkQA · 2020-01-29T20:25:39Z

Test build #117514 has finished for PR 24553 at commit 767233e.

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

SparkQA · 2020-01-31T15:19:20Z

Test build #117661 has finished for PR 24553 at commit 6c33c71.

This patch fails Scala style tests.
This patch does not merge cleanly.
This patch adds no public classes.

SparkQA · 2020-01-31T19:59:59Z

Test build #117663 has finished for PR 24553 at commit 63ea687.

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

peter-toth · 2020-02-03T07:39:54Z

@cloud-fan, @dongjoon-hyun, @maropu I improved ConstantPropagation a bit in this PR. Do you think you could review it?

SparkQA · 2020-02-13T15:48:52Z

Test build #118355 has finished for PR 24553 at commit 6d994ab.

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

github-actions · 2020-05-24T00:27:22Z

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

peter-toth mentioned this pull request May 8, 2019

[SPARK-27604][SQL] Add filter reduction #24495

Closed

mgaido91 reviewed May 8, 2019

View reviewed changes

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

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

peter-toth commented May 9, 2019

View reviewed changes

...core/src/test/scala/org/apache/spark/sql/execution/datasources/FileSourceStrategySuite.scala Show resolved Hide resolved

peter-toth commented May 9, 2019

View reviewed changes

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

mgaido91 reviewed May 24, 2019

View reviewed changes

mgaido91 reviewed May 25, 2019

View reviewed changes

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ConstantPropagationSuite.scala Outdated Show resolved Hide resolved

...core/src/test/scala/org/apache/spark/sql/execution/datasources/FileSourceStrategySuite.scala Show resolved Hide resolved

HyukjinKwon reviewed Jun 14, 2019

View reviewed changes

dongjoon-hyun added the SQL label Jun 14, 2019

HyukjinKwon mentioned this pull request Jun 20, 2019

[SPARK-27692][SQL] Add new optimizer rule to evaluate the deterministic scala udf only once if all inputs are literals #24593

Closed

peter-toth force-pushed the SPARK-27604-enhanced-constant-propagation branch from 827cfe5 to 054624e Compare June 22, 2019 16:26

peter-toth changed the title ~~[WIP][SPARK-27604][SQL] Enhance constant propagation~~ [SPARK-27604][SQL] Enhance constant propagation Jan 15, 2020

peter-toth changed the title ~~[SPARK-27604][SQL] Enhance constant propagation~~ [WIP][SPARK-27604][SQL] Enhance constant propagation Jan 15, 2020

peter-toth force-pushed the SPARK-27604-enhanced-constant-propagation branch from 39c563d to 85d5430 Compare January 15, 2020 16:15

peter-toth mentioned this pull request Jan 22, 2020

[SPARK-30598][SQL] Detect equijoins better #27309

Closed

peter-toth force-pushed the SPARK-27604-enhanced-constant-propagation branch from 4f0a89a to 5d67ffd Compare January 24, 2020 12:55

peter-toth force-pushed the SPARK-27604-enhanced-constant-propagation branch from 5d67ffd to 767233e Compare January 29, 2020 16:18

peter-toth force-pushed the SPARK-27604-enhanced-constant-propagation branch from 767233e to 6c33c71 Compare January 31, 2020 15:12

[SPARK-27604][SQL] Enhance constant propagation

63ea687

peter-toth force-pushed the SPARK-27604-enhanced-constant-propagation branch from 6c33c71 to 63ea687 Compare January 31, 2020 15:30

peter-toth changed the title ~~[WIP][SPARK-27604][SQL] Enhance constant propagation~~ [SPARK-27604][SQL] Enhance constant propagation Feb 3, 2020

peter-toth mentioned this pull request Feb 11, 2020

[SPARK-30768][SQL] Constraints inferred from inequality attributes #27518

Closed

fix UTs

6d994ab

github-actions bot added the Stale label May 24, 2020

github-actions bot closed this May 25, 2020

wangyum mentioned this pull request Feb 28, 2023

[SPARK-42500][SQL] ConstantPropagation supports more cases #40093

Closed

peter-toth mentioned this pull request Mar 10, 2023

[SPARK-42500][SQL] ConstantPropagation support more cases #40268

Closed

[SPARK-27604][SQL] Enhance constant propagation #24553

[SPARK-27604][SQL] Enhance constant propagation #24553

Uh oh!

Conversation

peter-toth commented May 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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?

Uh oh!

peter-toth commented May 8, 2019

Uh oh!

mgaido91 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

peter-toth commented May 8, 2019

Uh oh!

dongjoon-hyun commented May 8, 2019

Uh oh!

HyukjinKwon commented May 9, 2019

Uh oh!

SparkQA commented May 9, 2019

Uh oh!

mgaido91 commented May 9, 2019

Uh oh!

Uh oh!

SparkQA commented May 9, 2019

Uh oh!

SparkQA commented May 9, 2019

Uh oh!

Uh oh!

SparkQA commented May 9, 2019

Uh oh!

peter-toth commented May 13, 2019

Uh oh!

peter-toth commented May 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mgaido91 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

peter-toth commented May 24, 2019

Uh oh!

SparkQA commented May 24, 2019

Uh oh!

Uh oh!

Uh oh!

SparkQA commented May 25, 2019

Uh oh!

mgaido91 commented May 25, 2019

Uh oh!

peter-toth commented May 31, 2019

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon commented Jun 14, 2019

Uh oh!

SparkQA commented Jun 14, 2019

Uh oh!

SparkQA commented Jun 22, 2019

Uh oh!

peter-toth commented Dec 26, 2019

Uh oh!

peter-toth commented Jan 14, 2020

Uh oh!

SparkQA commented Jan 14, 2020

Uh oh!

SparkQA commented Jan 14, 2020

Uh oh!

SparkQA commented Jan 15, 2020

Uh oh!

SparkQA commented Jan 16, 2020

Uh oh!

peter-toth commented May 8, 2019 •

edited

Loading

peter-toth commented May 23, 2019 •

edited

Loading