From 048a0ecc65763c6feaa939938e2dec6f4040d939 Mon Sep 17 00:00:00 2001 From: Yuming Wang Date: Fri, 17 Jan 2020 14:29:09 +0800 Subject: [PATCH 01/10] Constraints should be inferred from cast equality constraint --- .../plans/logical/QueryPlanConstraints.scala | 11 ++++++++--- .../InferFiltersFromConstraintsSuite.scala | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala index 1355003358b9f..ea7e85fdc6826 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala @@ -62,11 +62,16 @@ trait ConstraintHelper { */ def inferAdditionalConstraints(constraints: Set[Expression]): Set[Expression] = { var inferredConstraints = Set.empty[Expression] - constraints.foreach { + val binaryComparisons = constraints.filter(_.isInstanceOf[BinaryComparison]) + binaryComparisons.foreach { case eq @ EqualTo(l: Attribute, r: Attribute) => - val candidateConstraints = constraints - eq + val candidateConstraints = binaryComparisons - eq inferredConstraints ++= replaceConstraints(candidateConstraints, l, r) inferredConstraints ++= replaceConstraints(candidateConstraints, r, l) + case eq @ EqualTo(l: Cast, r: Attribute) => + inferredConstraints ++= replaceConstraints(binaryComparisons - eq, r, l) + case eq @ EqualTo(l: Attribute, r: Cast) => + inferredConstraints ++= replaceConstraints(binaryComparisons - eq, l, r) case _ => // No inference } inferredConstraints -- constraints @@ -75,7 +80,7 @@ trait ConstraintHelper { private def replaceConstraints( constraints: Set[Expression], source: Expression, - destination: Attribute): Set[Expression] = constraints.map(_ transform { + destination: Expression): Set[Expression] = constraints.map(_ transform { case e: Expression if e.semanticEquals(source) => destination }) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala index 974bc781d36ab..05cb032ea089c 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala @@ -24,6 +24,7 @@ import org.apache.spark.sql.catalyst.plans._ import org.apache.spark.sql.catalyst.plans.logical._ import org.apache.spark.sql.catalyst.rules._ import org.apache.spark.sql.internal.SQLConf +import org.apache.spark.sql.types.LongType class InferFiltersFromConstraintsSuite extends PlanTest { @@ -263,4 +264,21 @@ class InferFiltersFromConstraintsSuite extends PlanTest { val y = testRelation.subquery('y) testConstraintsAfterJoin(x, y, x.where(IsNotNull('a)), y, RightOuter) } + + test("Constraints should be inferred from cast equality constraint") { + val testRelation1 = LocalRelation('a.int) + val testRelation2 = LocalRelation('b.long) + val originalLeft = testRelation1.subquery('left) + val originalRight = testRelation2.where('b === 1L).subquery('right) + + val left = testRelation1.where(IsNotNull('a) && 'a.cast(LongType) === 1L).subquery('left) + val right = testRelation2.where(IsNotNull('b) && 'b === 1L).subquery('right) + + Seq(Some("left.a".attr.cast(LongType) === "right.b".attr), + Some("right.b".attr === "left.a".attr.cast(LongType))).foreach { condition => + val optimized = Optimize.execute(originalLeft.join(originalRight, Inner, condition).analyze) + val correctAnswer = left.join(right, Inner, condition).analyze + comparePlans(optimized, correctAnswer) + } + } } From 34c7001e9f751ceda750fe38fa1c950aa18c36d8 Mon Sep 17 00:00:00 2001 From: Yuming Wang Date: Sun, 19 Jan 2020 15:25:18 +0800 Subject: [PATCH 02/10] Support filter at lower data type --- .../plans/logical/QueryPlanConstraints.scala | 39 +++++++++++++++---- .../InferFiltersFromConstraintsSuite.scala | 27 +++++++++---- 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala index ea7e85fdc6826..c62afeadaf416 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala @@ -68,21 +68,46 @@ trait ConstraintHelper { val candidateConstraints = binaryComparisons - eq inferredConstraints ++= replaceConstraints(candidateConstraints, l, r) inferredConstraints ++= replaceConstraints(candidateConstraints, r, l) - case eq @ EqualTo(l: Cast, r: Attribute) => - inferredConstraints ++= replaceConstraints(binaryComparisons - eq, r, l) - case eq @ EqualTo(l: Attribute, r: Cast) => - inferredConstraints ++= replaceConstraints(binaryComparisons - eq, l, r) + case eq @ EqualTo(l @ Cast(lc: Attribute, _, tz), r: Attribute) => + val candidateConstraints = binaryComparisons - eq + val bridge = Cast(r, lc.dataType, tz) + inferredConstraints ++= replaceConstraints(candidateConstraints, r, l) + inferredConstraints ++= replaceConstraints(candidateConstraints, lc, bridge) + case eq @ EqualTo(l: Attribute, r @ Cast(rc: Attribute, _, tz)) => + val candidateConstraints = binaryComparisons - eq + val bridge = Cast(l, rc.dataType, tz) + inferredConstraints ++= replaceConstraints(candidateConstraints, l, r) + inferredConstraints ++= replaceConstraints(candidateConstraints, rc, bridge) case _ => // No inference } inferredConstraints -- constraints } + private def replaceConstraint( + constraint: Expression, + source: Expression, + destination: Expression): Expression = constraint transform { + case e: Expression if e.semanticEquals(source) => destination + } + private def replaceConstraints( constraints: Set[Expression], source: Expression, - destination: Expression): Set[Expression] = constraints.map(_ transform { - case e: Expression if e.semanticEquals(source) => destination - }) + dest: Expression): Set[Expression] = { + constraints.map { + case b @ BinaryComparison(left, right) => + (replaceConstraint(left, source, dest), replaceConstraint(right, source, dest)) match { + case (Cast(Cast(child, _, _), dt, _), replacedRight) + if dt == child.dataType && child.dataType == replacedRight.dataType => + b.makeCopy(Array(child, replacedRight)) + case (replacedLeft, Cast(Cast(child, _, _), dt, _)) + if dt == child.dataType && child.dataType == replacedLeft.dataType => + b.makeCopy(Array(replacedLeft, child)) + case (replacedLeft, replacedRight) => + b.makeCopy(Array(replacedLeft, replacedRight)) + } + } + } /** * Infers a set of `isNotNull` constraints from null intolerant expressions as well as diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala index 05cb032ea089c..ad25a6752562a 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala @@ -24,7 +24,7 @@ import org.apache.spark.sql.catalyst.plans._ import org.apache.spark.sql.catalyst.plans.logical._ import org.apache.spark.sql.catalyst.rules._ import org.apache.spark.sql.internal.SQLConf -import org.apache.spark.sql.types.LongType +import org.apache.spark.sql.types.{IntegerType, LongType} class InferFiltersFromConstraintsSuite extends PlanTest { @@ -47,8 +47,8 @@ class InferFiltersFromConstraintsSuite extends PlanTest { y: LogicalPlan, expectedLeft: LogicalPlan, expectedRight: LogicalPlan, - joinType: JoinType) = { - val condition = Some("x.a".attr === "y.a".attr) + joinType: JoinType, + condition: Option[Expression] = Some("x.a".attr === "y.a".attr)) = { val originalQuery = x.join(y, joinType, condition).analyze val correctAnswer = expectedLeft.join(expectedRight, joinType, condition).analyze val optimized = Optimize.execute(originalQuery) @@ -265,7 +265,22 @@ class InferFiltersFromConstraintsSuite extends PlanTest { testConstraintsAfterJoin(x, y, x.where(IsNotNull('a)), y, RightOuter) } - test("Constraints should be inferred from cast equality constraint") { + test("Constraints should be inferred from cast equality constraint(filter at lower data type)") { + val testRelation1 = LocalRelation('a.int) + val testRelation2 = LocalRelation('b.long) + val originalLeft = testRelation1.where('a === 1).subquery('left) + val originalRight = testRelation2.subquery('right) + + val left = testRelation1.where(IsNotNull('a) && 'a === 1).subquery('left) + val right = testRelation2.where(IsNotNull('b) && 'b.cast(IntegerType) === 1).subquery('right) + + Seq(Some("left.a".attr.cast(LongType) === "right.b".attr), + Some("right.b".attr === "left.a".attr.cast(LongType))).foreach { condition => + testConstraintsAfterJoin(originalLeft, originalRight, left, right, Inner, condition) + } + } + + test("Constraints should be inferred from cast equality constraint(filter at higher data type)") { val testRelation1 = LocalRelation('a.int) val testRelation2 = LocalRelation('b.long) val originalLeft = testRelation1.subquery('left) @@ -276,9 +291,7 @@ class InferFiltersFromConstraintsSuite extends PlanTest { Seq(Some("left.a".attr.cast(LongType) === "right.b".attr), Some("right.b".attr === "left.a".attr.cast(LongType))).foreach { condition => - val optimized = Optimize.execute(originalLeft.join(originalRight, Inner, condition).analyze) - val correctAnswer = left.join(right, Inner, condition).analyze - comparePlans(optimized, correctAnswer) + testConstraintsAfterJoin(originalLeft, originalRight, left, right, Inner, condition) } } } From 7dcfe915087dbe274b470928600197745a645f5e Mon Sep 17 00:00:00 2001 From: Yuming Wang Date: Mon, 20 Jan 2020 14:28:41 +0800 Subject: [PATCH 03/10] fix test error --- .../plans/logical/QueryPlanConstraints.scala | 27 ++++--------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala index c62afeadaf416..3d74f58cfb8dc 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala @@ -83,31 +83,14 @@ trait ConstraintHelper { inferredConstraints -- constraints } - private def replaceConstraint( - constraint: Expression, - source: Expression, - destination: Expression): Expression = constraint transform { - case e: Expression if e.semanticEquals(source) => destination - } - private def replaceConstraints( constraints: Set[Expression], source: Expression, - dest: Expression): Set[Expression] = { - constraints.map { - case b @ BinaryComparison(left, right) => - (replaceConstraint(left, source, dest), replaceConstraint(right, source, dest)) match { - case (Cast(Cast(child, _, _), dt, _), replacedRight) - if dt == child.dataType && child.dataType == replacedRight.dataType => - b.makeCopy(Array(child, replacedRight)) - case (replacedLeft, Cast(Cast(child, _, _), dt, _)) - if dt == child.dataType && child.dataType == replacedLeft.dataType => - b.makeCopy(Array(replacedLeft, child)) - case (replacedLeft, replacedRight) => - b.makeCopy(Array(replacedLeft, replacedRight)) - } - } - } + destination: Expression): Set[Expression] = constraints.map(_ transform { + case e: Expression if e.semanticEquals(source) => destination + }).map(_ transform { + case Cast(e @ Cast(child, _, _), dt, _) if e == destination && dt == child.dataType => child + }) /** * Infers a set of `isNotNull` constraints from null intolerant expressions as well as From 2060190d319c9008371ae9af8aabb2ebc757dde6 Mon Sep 17 00:00:00 2001 From: Yuming Wang Date: Fri, 31 Jan 2020 13:58:20 +0800 Subject: [PATCH 04/10] Only support filter higher data type --- .../plans/logical/QueryPlanConstraints.scala | 18 +++++------------ .../InferFiltersFromConstraintsSuite.scala | 20 +++++++++---------- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala index 3d74f58cfb8dc..ce88fd7efeebf 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala @@ -63,21 +63,15 @@ trait ConstraintHelper { def inferAdditionalConstraints(constraints: Set[Expression]): Set[Expression] = { var inferredConstraints = Set.empty[Expression] val binaryComparisons = constraints.filter(_.isInstanceOf[BinaryComparison]) - binaryComparisons.foreach { + constraints.foreach { case eq @ EqualTo(l: Attribute, r: Attribute) => val candidateConstraints = binaryComparisons - eq inferredConstraints ++= replaceConstraints(candidateConstraints, l, r) inferredConstraints ++= replaceConstraints(candidateConstraints, r, l) - case eq @ EqualTo(l @ Cast(lc: Attribute, _, tz), r: Attribute) => - val candidateConstraints = binaryComparisons - eq - val bridge = Cast(r, lc.dataType, tz) - inferredConstraints ++= replaceConstraints(candidateConstraints, r, l) - inferredConstraints ++= replaceConstraints(candidateConstraints, lc, bridge) - case eq @ EqualTo(l: Attribute, r @ Cast(rc: Attribute, _, tz)) => - val candidateConstraints = binaryComparisons - eq - val bridge = Cast(l, rc.dataType, tz) - inferredConstraints ++= replaceConstraints(candidateConstraints, l, r) - inferredConstraints ++= replaceConstraints(candidateConstraints, rc, bridge) + case eq @ EqualTo(l @ Cast(_: Attribute, _, _), r: Attribute) => + inferredConstraints ++= replaceConstraints(binaryComparisons - eq, r, l) + case eq @ EqualTo(l: Attribute, r @ Cast(_: Attribute, _, _)) => + inferredConstraints ++= replaceConstraints(binaryComparisons - eq, l, r) case _ => // No inference } inferredConstraints -- constraints @@ -88,8 +82,6 @@ trait ConstraintHelper { source: Expression, destination: Expression): Set[Expression] = constraints.map(_ transform { case e: Expression if e.semanticEquals(source) => destination - }).map(_ transform { - case Cast(e @ Cast(child, _, _), dt, _) if e == destination && dt == child.dataType => child }) /** diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala index ad25a6752562a..7ab80c71ea2e1 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala @@ -265,14 +265,14 @@ class InferFiltersFromConstraintsSuite extends PlanTest { testConstraintsAfterJoin(x, y, x.where(IsNotNull('a)), y, RightOuter) } - test("Constraints should be inferred from cast equality constraint(filter at lower data type)") { + test("Constraints should be inferred from cast equality constraint(filter higher data type)") { val testRelation1 = LocalRelation('a.int) val testRelation2 = LocalRelation('b.long) - val originalLeft = testRelation1.where('a === 1).subquery('left) - val originalRight = testRelation2.subquery('right) + val originalLeft = testRelation1.subquery('left) + val originalRight = testRelation2.where('b === 1L).subquery('right) - val left = testRelation1.where(IsNotNull('a) && 'a === 1).subquery('left) - val right = testRelation2.where(IsNotNull('b) && 'b.cast(IntegerType) === 1).subquery('right) + val left = testRelation1.where(IsNotNull('a) && 'a.cast(LongType) === 1L).subquery('left) + val right = testRelation2.where(IsNotNull('b) && 'b === 1L).subquery('right) Seq(Some("left.a".attr.cast(LongType) === "right.b".attr), Some("right.b".attr === "left.a".attr.cast(LongType))).foreach { condition => @@ -280,14 +280,14 @@ class InferFiltersFromConstraintsSuite extends PlanTest { } } - test("Constraints should be inferred from cast equality constraint(filter at higher data type)") { + test("Constraints shouldn't be inferred from cast equality constraint(filter lower data type)") { val testRelation1 = LocalRelation('a.int) val testRelation2 = LocalRelation('b.long) - val originalLeft = testRelation1.subquery('left) - val originalRight = testRelation2.where('b === 1L).subquery('right) + val originalLeft = testRelation1.where('a === 1).subquery('left) + val originalRight = testRelation2.subquery('right) - val left = testRelation1.where(IsNotNull('a) && 'a.cast(LongType) === 1L).subquery('left) - val right = testRelation2.where(IsNotNull('b) && 'b === 1L).subquery('right) + val left = testRelation1.where(IsNotNull('a) && 'a === 1).subquery('left) + val right = testRelation2.where(IsNotNull('b)).subquery('right) Seq(Some("left.a".attr.cast(LongType) === "right.b".attr), Some("right.b".attr === "left.a".attr.cast(LongType))).foreach { condition => From 420058bf35371ddb49c67ff3c893add3281aeba9 Mon Sep 17 00:00:00 2001 From: Yuming Wang Date: Tue, 4 Feb 2020 11:21:43 +0800 Subject: [PATCH 05/10] IsNotNull should be constructed by `constructIsNotNullConstraints`. --- .../catalyst/plans/logical/QueryPlanConstraints.scala | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala index ce88fd7efeebf..2c4d183bb4df4 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala @@ -62,16 +62,17 @@ trait ConstraintHelper { */ def inferAdditionalConstraints(constraints: Set[Expression]): Set[Expression] = { var inferredConstraints = Set.empty[Expression] - val binaryComparisons = constraints.filter(_.isInstanceOf[BinaryComparison]) + // IsNotNull should be constructed by `constructIsNotNullConstraints`. + val predicates = constraints.filterNot(_.isInstanceOf[IsNotNull]) constraints.foreach { case eq @ EqualTo(l: Attribute, r: Attribute) => - val candidateConstraints = binaryComparisons - eq + val candidateConstraints = predicates - eq inferredConstraints ++= replaceConstraints(candidateConstraints, l, r) inferredConstraints ++= replaceConstraints(candidateConstraints, r, l) case eq @ EqualTo(l @ Cast(_: Attribute, _, _), r: Attribute) => - inferredConstraints ++= replaceConstraints(binaryComparisons - eq, r, l) + inferredConstraints ++= replaceConstraints(predicates - eq, r, l) case eq @ EqualTo(l: Attribute, r @ Cast(_: Attribute, _, _)) => - inferredConstraints ++= replaceConstraints(binaryComparisons - eq, l, r) + inferredConstraints ++= replaceConstraints(predicates - eq, l, r) case _ => // No inference } inferredConstraints -- constraints From 8dcd076ba1843289b5775abe002ff0d88f235d8f Mon Sep 17 00:00:00 2001 From: Yuming Wang Date: Tue, 11 Feb 2020 12:59:44 +0800 Subject: [PATCH 06/10] Address comment --- .../optimizer/InferFiltersFromConstraintsSuite.scala | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala index 7ab80c71ea2e1..670c2e6ba0d74 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala @@ -283,11 +283,13 @@ class InferFiltersFromConstraintsSuite extends PlanTest { test("Constraints shouldn't be inferred from cast equality constraint(filter lower data type)") { val testRelation1 = LocalRelation('a.int) val testRelation2 = LocalRelation('b.long) - val originalLeft = testRelation1.where('a === 1).subquery('left) - val originalRight = testRelation2.subquery('right) + val originalLeft = testRelation1.where('a > 1).subquery('left) + val originalRight = testRelation2.where('b < 10).subquery('right) - val left = testRelation1.where(IsNotNull('a) && 'a === 1).subquery('left) - val right = testRelation2.where(IsNotNull('b)).subquery('right) + val left = testRelation1.where( + IsNotNull('a) && 'a > 1 && 'a.cast(LongType) < Literal(10).cast(LongType)).subquery('left) + val right = testRelation2.where( + IsNotNull('b) && 'b < Literal(10).cast(LongType)).subquery('right) Seq(Some("left.a".attr.cast(LongType) === "right.b".attr), Some("right.b".attr === "left.a".attr.cast(LongType))).foreach { condition => From 47eadf4bbd35aa853175d28527278194c4f7225d Mon Sep 17 00:00:00 2001 From: Yuming Wang Date: Wed, 12 Feb 2020 09:53:51 +0800 Subject: [PATCH 07/10] Address comment --- .../InferFiltersFromConstraintsSuite.scala | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala index 670c2e6ba0d74..d2816fb09ae84 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala @@ -283,17 +283,23 @@ class InferFiltersFromConstraintsSuite extends PlanTest { test("Constraints shouldn't be inferred from cast equality constraint(filter lower data type)") { val testRelation1 = LocalRelation('a.int) val testRelation2 = LocalRelation('b.long) - val originalLeft = testRelation1.where('a > 1).subquery('left) - val originalRight = testRelation2.where('b < 10).subquery('right) + val originalLeft = testRelation1.where('a === 1).subquery('left) + val originalRight = testRelation2.subquery('right) - val left = testRelation1.where( - IsNotNull('a) && 'a > 1 && 'a.cast(LongType) < Literal(10).cast(LongType)).subquery('left) - val right = testRelation2.where( - IsNotNull('b) && 'b < Literal(10).cast(LongType)).subquery('right) + val left = testRelation1.where(IsNotNull('a) && 'a === 1).subquery('left) + val right = testRelation2.where(IsNotNull('b)).subquery('right) Seq(Some("left.a".attr.cast(LongType) === "right.b".attr), Some("right.b".attr === "left.a".attr.cast(LongType))).foreach { condition => testConstraintsAfterJoin(originalLeft, originalRight, left, right, Inner, condition) } + + testConstraintsAfterJoin( + originalLeft, + originalRight, + left, + testRelation2.where(IsNotNull('b) && 'b.attr.cast(IntegerType) === 1).subquery('right), + Inner, + Some("left.a".attr === "right.b".attr.cast(IntegerType))) } } From d283028b09e4de0c46b72bff7617f04c824d3198 Mon Sep 17 00:00:00 2001 From: Yuming Wang Date: Wed, 12 Feb 2020 10:55:37 +0800 Subject: [PATCH 08/10] constraints -> predicates --- .../spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala index 2c4d183bb4df4..4c4ec000d0930 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/QueryPlanConstraints.scala @@ -64,7 +64,7 @@ trait ConstraintHelper { var inferredConstraints = Set.empty[Expression] // IsNotNull should be constructed by `constructIsNotNullConstraints`. val predicates = constraints.filterNot(_.isInstanceOf[IsNotNull]) - constraints.foreach { + predicates.foreach { case eq @ EqualTo(l: Attribute, r: Attribute) => val candidateConstraints = predicates - eq inferredConstraints ++= replaceConstraints(candidateConstraints, l, r) From d055eba5fb7aa1e2ba874a641093aa40c1fad0d2 Mon Sep 17 00:00:00 2001 From: Yuming Wang Date: Wed, 12 Feb 2020 11:43:58 +0800 Subject: [PATCH 09/10] Improve test coverage --- .../InferFiltersFromConstraintsSuite.scala | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala index d2816fb09ae84..a25f8a2261754 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala @@ -294,12 +294,15 @@ class InferFiltersFromConstraintsSuite extends PlanTest { testConstraintsAfterJoin(originalLeft, originalRight, left, right, Inner, condition) } - testConstraintsAfterJoin( - originalLeft, - originalRight, - left, - testRelation2.where(IsNotNull('b) && 'b.attr.cast(IntegerType) === 1).subquery('right), - Inner, - Some("left.a".attr === "right.b".attr.cast(IntegerType))) + Seq(Some("left.a".attr === "right.b".attr.cast(IntegerType)), + Some("right.b".attr.cast(IntegerType) === "left.a".attr)).foreach { condition => + testConstraintsAfterJoin( + originalLeft, + originalRight, + left, + testRelation2.where(IsNotNull('b) && 'b.attr.cast(IntegerType) === 1).subquery('right), + Inner, + condition) + } } } From 4b14b3f619af56babc981e729f43e165369cad01 Mon Sep 17 00:00:00 2001 From: Yuming Wang Date: Thu, 13 Feb 2020 00:32:37 +0800 Subject: [PATCH 10/10] Improve test coverage --- .../optimizer/InferFiltersFromConstraintsSuite.scala | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala index a25f8a2261754..79bd573f1d84a 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/InferFiltersFromConstraintsSuite.scala @@ -278,6 +278,17 @@ class InferFiltersFromConstraintsSuite extends PlanTest { Some("right.b".attr === "left.a".attr.cast(LongType))).foreach { condition => testConstraintsAfterJoin(originalLeft, originalRight, left, right, Inner, condition) } + + Seq(Some("left.a".attr === "right.b".attr.cast(IntegerType)), + Some("right.b".attr.cast(IntegerType) === "left.a".attr)).foreach { condition => + testConstraintsAfterJoin( + originalLeft, + originalRight, + testRelation1.where(IsNotNull('a)).subquery('left), + right, + Inner, + condition) + } } test("Constraints shouldn't be inferred from cast equality constraint(filter lower data type)") {