fix comments

Author: Zhenhua Wang

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/EstimationUtils.scala

@@ -214,24 +214,25 @@ object EstimationUtils {
   }
 
   /**
-   * Returns overlapped ranges between two histograms, in the given value range [newMin, newMax].
+   * Returns overlapped ranges between two histograms, in the given value range
+   * [lowerBound, upperBound].
    */
   def getOverlappedRanges(
     leftHistogram: Histogram,
     rightHistogram: Histogram,
-    newMin: Double,
-    newMax: Double): Seq[OverlappedRange] = {
+    lowerBound: Double,
+    upperBound: Double): Seq[OverlappedRange] = {
     val overlappedRanges = new ArrayBuffer[OverlappedRange]()
-    // Only bins whose range intersect [newMin, newMax] have join possibility.
+    // Only bins whose range intersect [lowerBound, upperBound] have join possibility.
     val leftBins = leftHistogram.bins
-      .filter(b => b.lo <= newMax && b.hi >= newMin)
+      .filter(b => b.lo <= upperBound && b.hi >= lowerBound)
     val rightBins = rightHistogram.bins
-      .filter(b => b.lo <= newMax && b.hi >= newMin)
+      .filter(b => b.lo <= upperBound && b.hi >= lowerBound)
 
     leftBins.foreach { lb =>
       rightBins.foreach { rb =>
-        val (left, leftHeight) = trimBin(lb, leftHistogram.height, newMin, newMax)
-        val (right, rightHeight) = trimBin(rb, rightHistogram.height, newMin, newMax)
+        val (left, leftHeight) = trimBin(lb, leftHistogram.height, lowerBound, upperBound)
+        val (right, rightHeight) = trimBin(rb, rightHistogram.height, lowerBound, upperBound)
         // Only collect overlapped ranges.
         if (left.lo <= right.hi && left.hi >= right.lo) {
           // Collect overlapped ranges.
@@ -259,9 +260,7 @@ object EstimationUtils {
             // Case3: the left bin is "smaller" than the right bin
             //      left.lo            right.lo     left.hi          right.hi
             // --------+------------------+------------+----------------+------->
-            val leftRatio = (left.hi - right.lo) / (left.hi - left.lo)
-            val rightRatio = (left.hi - right.lo) / (right.hi - right.lo)
-            if (leftRatio == 0) {
+            if (left.hi == right.lo) {
               // The overlapped range has only one value.
               OverlappedRange(
                 lo = right.lo,
@@ -272,6 +271,8 @@ object EstimationUtils {
                 rightNumRows = rightHeight / right.ndv
               )
             } else {
+              val leftRatio = (left.hi - right.lo) / (left.hi - left.lo)
+              val rightRatio = (left.hi - right.lo) / (right.hi - right.lo)
               OverlappedRange(
                 lo = right.lo,
                 hi = left.hi,
@@ -285,9 +286,7 @@ object EstimationUtils {
             // Case4: the left bin is "larger" than the right bin
             //      right.lo           left.lo      right.hi         left.hi
             // --------+------------------+------------+----------------+------->
-            val leftRatio = (right.hi - left.lo) / (left.hi - left.lo)
-            val rightRatio = (right.hi - left.lo) / (right.hi - right.lo)
-            if (leftRatio == 0) {
+            if (right.hi == left.lo) {
               // The overlapped range has only one value.
               OverlappedRange(
                 lo = right.hi,
@@ -298,6 +297,8 @@ object EstimationUtils {
                 rightNumRows = rightHeight / right.ndv
               )
             } else {
+              val leftRatio = (right.hi - left.lo) / (left.hi - left.lo)
+              val rightRatio = (right.hi - left.lo) / (right.hi - right.lo)
               OverlappedRange(
                 lo = left.lo,
                 hi = right.hi,
@@ -343,24 +344,26 @@ object EstimationUtils {
   }
 
   /**
-   * Given an original bin and a value range [min, max], returns the trimmed bin and its number of
-   * rows.
+   * Given an original bin and a value range [lowerBound, upperBound], returns the trimmed part
+   * of the bin in that range and its number of rows.
    */
-  def trimBin(bin: HistogramBin, height: Double, min: Double, max: Double)
+  def trimBin(bin: HistogramBin, height: Double, lowerBound: Double, upperBound: Double)
   : (HistogramBin, Double) = {
-    val (lo, hi) = if (bin.lo <= min && bin.hi >= max) {
-      //       bin.lo              min          max          bin.hi
+    val (lo, hi) = if (bin.lo <= lowerBound && bin.hi >= upperBound) {
+      //       bin.lo          lowerBound     upperBound      bin.hi
+      // --------+------------------+------------+-------------+------->
+      (lowerBound, upperBound)
+    } else if (bin.lo <= lowerBound && bin.hi >= lowerBound) {
+      //       bin.lo          lowerBound      bin.hi      upperBound
       // --------+------------------+------------+-------------+------->
-      (min, max)
-    } else if (bin.lo <= min && bin.hi >= min) {
-      //       bin.lo              min        bin.hi
-      // --------+------------------+-----------+------->
-      (min, bin.hi)
-    } else if (bin.lo <= max && bin.hi >= max) {
-      //        bin.lo             max        bin.hi
-      // --------+------------------+-----------+------->
-      (bin.lo, max)
+      (lowerBound, bin.hi)
+    } else if (bin.lo <= upperBound && bin.hi >= upperBound) {
+      //    lowerBound            bin.lo     upperBound       bin.hi
+      // --------+------------------+------------+-------------+------->
+      (bin.lo, upperBound)
     } else {
+      //    lowerBound            bin.lo        bin.hi     upperBound
+      // --------+------------------+------------+-------------+------->
       (bin.lo, bin.hi)
     }
 

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/JoinEstimation.scala

@@ -245,8 +245,8 @@ case class JoinEstimation(join: Join) extends Logging {
       leftHistogram = leftHistogram,
       rightHistogram = rightHistogram,
       // Only numeric values have equi-height histograms.
-      newMin = newMin.get.toString.toDouble,
-      newMax = newMax.get.toString.toDouble)
+      lowerBound = newMin.get.toString.toDouble,
+      upperBound = newMax.get.toString.toDouble)
 
     var card: BigDecimal = 0
     var totalNdv: Double = 0

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/statsEstimation/JoinEstimationSuite.scala

@@ -129,9 +129,9 @@ class JoinEstimationSuite extends StatsEstimationTestBase {
     val histogram2 = Histogram(height = 100, Array(
       HistogramBin(lo = 0, hi = 50, ndv = 50), HistogramBin(lo = 50, hi = 100, ndv = 40)))
     // test bin trimming
-    val (t0, h0) = trimBin(histogram2.bins(0), height = 100, min = 10, max = 60)
+    val (t0, h0) = trimBin(histogram2.bins(0), height = 100, lowerBound = 10, upperBound = 60)
     assert(t0 == HistogramBin(lo = 10, hi = 50, ndv = 40) && h0 == 80)
-    val (t1, h1) = trimBin(histogram2.bins(1), height = 100, min = 10, max = 60)
+    val (t1, h1) = trimBin(histogram2.bins(1), height = 100, lowerBound = 10, upperBound = 60)
     assert(t1 == HistogramBin(lo = 50, hi = 60, ndv = 8) && h1 == 20)
 
     val expectedRanges = Seq(
@@ -143,7 +143,7 @@ class JoinEstimationSuite extends StatsEstimationTestBase {
       OverlappedRange(50, 60, 30*1/3, 8, 300*1/3, 20)
     )
     assert(expectedRanges.equals(
-      getOverlappedRanges(histogram1, histogram2, newMin = 10D, newMax = 60D)))
+      getOverlappedRanges(histogram1, histogram2, lowerBound = 10D, upperBound = 60D)))
 
     estimateByHistogram(
       leftHistogram = histogram1,
@@ -162,9 +162,9 @@ class JoinEstimationSuite extends StatsEstimationTestBase {
     val histogram2 = Histogram(height = 100, Array(
       HistogramBin(lo = 0, hi = 50, ndv = 50), HistogramBin(lo = 50, hi = 100, ndv = 40)))
     // test bin trimming
-    val (t0, h0) = trimBin(histogram2.bins(0), height = 100, min = 50, max = 75)
+    val (t0, h0) = trimBin(histogram2.bins(0), height = 100, lowerBound = 50, upperBound = 75)
     assert(t0 == HistogramBin(lo = 50, hi = 50, ndv = 1) && h0 == 2)
-    val (t1, h1) = trimBin(histogram2.bins(1), height = 100, min = 50, max = 75)
+    val (t1, h1) = trimBin(histogram2.bins(1), height = 100, lowerBound = 50, upperBound = 75)
     assert(t1 == HistogramBin(lo = 50, hi = 75, ndv = 20) && h1 == 50)
 
     val expectedRanges = Seq(
@@ -176,7 +176,7 @@ class JoinEstimationSuite extends StatsEstimationTestBase {
       OverlappedRange(60, 75, 3, 20*15/25, 300, 50*15/25)
     )
     assert(expectedRanges.equals(
-      getOverlappedRanges(histogram1, histogram2, newMin = 50D, newMax = 75D)))
+      getOverlappedRanges(histogram1, histogram2, lowerBound = 50D, upperBound = 75D)))
 
     estimateByHistogram(
       leftHistogram = histogram1,
@@ -197,9 +197,9 @@ class JoinEstimationSuite extends StatsEstimationTestBase {
     val histogram2 = Histogram(height = 100, Array(
       HistogramBin(lo = 0, hi = 50, ndv = 50), HistogramBin(lo = 50, hi = 100, ndv = 40)))
     // test bin trimming
-    val (t0, h0) = trimBin(histogram2.bins(0), height = 100, min = 30, max = 60)
+    val (t0, h0) = trimBin(histogram2.bins(0), height = 100, lowerBound = 30, upperBound = 60)
     assert(t0 == HistogramBin(lo = 30, hi = 50, ndv = 20) && h0 == 40)
-    val (t1, h1) = trimBin(histogram2.bins(1), height = 100, min = 30, max = 60)
+    val (t1, h1) = trimBin(histogram2.bins(1), height = 100, lowerBound = 30, upperBound = 60)
     assert(t1 ==HistogramBin(lo = 50, hi = 60, ndv = 8) && h1 == 20)
 
     val expectedRanges = Seq(
@@ -209,7 +209,7 @@ class JoinEstimationSuite extends StatsEstimationTestBase {
       OverlappedRange(50, 60, 30*1/3, 8, 300*1/3, 20)
     )
     assert(expectedRanges.equals(
-      getOverlappedRanges(histogram1, histogram2, newMin = 30D, newMax = 60D)))
+      getOverlappedRanges(histogram1, histogram2, lowerBound = 30D, upperBound = 60D)))
 
     estimateByHistogram(
       leftHistogram = histogram1,
@@ -228,9 +228,9 @@ class JoinEstimationSuite extends StatsEstimationTestBase {
     val histogram2 = Histogram(height = 100, Array(
       HistogramBin(lo = 0, hi = 50, ndv = 50), HistogramBin(lo = 50, hi = 50, ndv = 1)))
     // test bin trimming
-    val (t0, h0) = trimBin(histogram1.bins(1), height = 300, min = 30, max = 50)
+    val (t0, h0) = trimBin(histogram1.bins(1), height = 300, lowerBound = 30, upperBound = 50)
     assert(t0 == HistogramBin(lo = 30, hi = 50, ndv = 20) && h0 == 200)
-    val (t1, h1) = trimBin(histogram2.bins(0), height = 100, min = 30, max = 50)
+    val (t1, h1) = trimBin(histogram2.bins(0), height = 100, lowerBound = 30, upperBound = 50)
     assert(t1 == HistogramBin(lo = 30, hi = 50, ndv = 20) && h1 == 40)
 
     val expectedRanges = Seq(
@@ -239,7 +239,7 @@ class JoinEstimationSuite extends StatsEstimationTestBase {
       OverlappedRange(50, 50, 1, 1, 200/20, 100)
     )
     assert(expectedRanges.equals(
-      getOverlappedRanges(histogram1, histogram2, newMin = 30D, newMax = 50D)))
+      getOverlappedRanges(histogram1, histogram2, lowerBound = 30D, upperBound = 50D)))
 
     estimateByHistogram(
       leftHistogram = histogram1,
@@ -258,9 +258,9 @@ class JoinEstimationSuite extends StatsEstimationTestBase {
     val histogram2 = Histogram(height = 150, Array(
       HistogramBin(lo = 0, hi = 30, ndv = 30), HistogramBin(lo = 30, hi = 30, ndv = 1)))
     // test bin trimming
-    val (t0, h0) = trimBin(histogram1.bins(1), height = 300, min = 30, max = 30)
+    val (t0, h0) = trimBin(histogram1.bins(1), height = 300, lowerBound = 30, upperBound = 30)
     assert(t0 == HistogramBin(lo = 30, hi = 30, ndv = 1) && h0 == 10)
-    val (t1, h1) = trimBin(histogram2.bins(0), height = 150, min = 30, max = 30)
+    val (t1, h1) = trimBin(histogram2.bins(0), height = 150, lowerBound = 30, upperBound = 30)
     assert(t1 == HistogramBin(lo = 30, hi = 30, ndv = 1) && h1 == 5)
 
     val expectedRanges = Seq(
@@ -270,7 +270,7 @@ class JoinEstimationSuite extends StatsEstimationTestBase {
       OverlappedRange(30, 30, 1, 1, 10, 150)
     )
     assert(expectedRanges.equals(
-      getOverlappedRanges(histogram1, histogram2, newMin = 30D, newMax = 30D)))
+      getOverlappedRanges(histogram1, histogram2, lowerBound = 30D, upperBound = 30D)))
 
     estimateByHistogram(
       leftHistogram = histogram1,