[GitHub] spark pull request #19874: [SPARK-22675] [SQL] Refactoring PropagateTypes in...
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...
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...
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...
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...
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...
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...
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