[GitHub] spark pull request #19874: [SPARK-22675] [SQL] Refactoring PropagateTypes in...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/19874


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19874: [SPARK-22675] [SQL] Refactoring PropagateTypes in...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/19874#discussion_r154855820
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/DecimalPrecision.scala
 ---
@@ -58,7 +58,7 @@ import org.apache.spark.sql.types._
  * - FLOAT and DOUBLE cause fixed-length decimals to turn into DOUBLE
  */
 // scalastyle:on
-object DecimalPrecision extends Rule[LogicalPlan] {
+object DecimalPrecision extends Rule[LogicalPlan] with TypePropagation {
--- End diff --

one benefit of naming it `TypeCoercionRule` is, we can just do `object 
DecimalPrecision extends TypeCoercionRule`


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19874: [SPARK-22675] [SQL] Refactoring PropagateTypes in...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/19874#discussion_r154854338
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/TPCDSQuerySuite.scala ---
@@ -39,6 +40,8 @@ class TPCDSQuerySuite extends QueryTest with 
SharedSQLContext with BeforeAndAfte
*/
   protected override def afterAll(): Unit = {
 try {
+  // For debugging dump some statistics about how much time was spent 
in various optimizer rules
+  logWarning(RuleExecutor.dumpTimeSpent())
--- End diff --

Let me change all the other places. 


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19874: [SPARK-22675] [SQL] Refactoring PropagateTypes in...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/19874#discussion_r154854356
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ---
@@ -850,3 +834,45 @@ object TypeCoercion {
 }
   }
 }
+
+trait TypePropagation extends Logging {
--- End diff --

Sure. 


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19874: [SPARK-22675] [SQL] Refactoring PropagateTypes in...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/19874#discussion_r154853933
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/TPCDSQuerySuite.scala ---
@@ -39,6 +40,8 @@ class TPCDSQuerySuite extends QueryTest with 
SharedSQLContext with BeforeAndAfte
*/
   protected override def afterAll(): Unit = {
 try {
+  // For debugging dump some statistics about how much time was spent 
in various optimizer rules
+  logWarning(RuleExecutor.dumpTimeSpent())
--- End diff --

use `logInfo`?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19874: [SPARK-22675] [SQL] Refactoring PropagateTypes in...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/19874#discussion_r154853773
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ---
@@ -850,3 +834,45 @@ object TypeCoercion {
 }
   }
 }
+
+trait TypePropagation extends Logging {
--- End diff --

how about
```
trait TypeCoercionRule extends Rule[LogicalPlan] {
  protected def coerciveType(plan: LogicalPlan): LogicalPlan

  def apply(plan: LogicalPlan): LogicalPlan = {
val newPlan = coerciveType(plan)
...
  }
}
```


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #19874: [SPARK-22675] [SQL] Refactoring PropagateTypes in...

[gatorsmile]

GitHub user gatorsmile opened a pull request:

https://github.com/apache/spark/pull/19874

[SPARK-22675] [SQL] Refactoring PropagateTypes in TypeCoercion

## What changes were proposed in this pull request?
PropagateTypes are called twice in TypeCoercion. We do not need to call it 
twice. Instead, we should call it after each change on the types. 

## How was this patch tested?
The existing tests

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/gatorsmile/spark deduplicatePropagateTypes

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/19874.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #19874


commit fc3a24e039c905fdd263d2ece2088ce152887fc7
Author: gatorsmile 
Date:   2017-12-03T05:55:00Z

clean

commit 657cf88bbf4259d1a823f93f16eaccc2fbe78667
Author: gatorsmile 
Date:   2017-12-04T07:40:26Z

refactor




---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org