[SPARK-21322][SQL] support histogram in filter cardinality estimation #19783

ron8hu · 2017-11-19T19:59:32Z

What changes were proposed in this pull request?

Histogram is effective in dealing with skewed distribution. After we generate histogram information for column statistics, we need to adjust filter estimation based on histogram data structure.

How was this patch tested?

We revised all the unit test cases by including histogram data structure.

Please review http://spark.apache.org/contributing.html before opening a pull request.

SparkQA · 2017-11-19T22:52:37Z

Test build #84003 has finished for PR 19783 at commit dd5b975.

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

ron8hu · 2017-11-20T01:31:49Z

cc @sameeragarwal @cloud-fan @gatorsmile @wzhfy

wzhfy · 2017-11-20T02:13:36Z

...yst/src/test/scala/org/apache/spark/sql/catalyst/statsEstimation/FilterEstimationSuite.scala

@@ -158,8 +196,8 @@ class FilterEstimationSuite extends StatsEstimationTestBase {
    val condition = Not(And(LessThan(attrInt, Literal(3)), Literal(null, IntegerType)))
    validateEstimatedStats(
      Filter(condition, childStatsTestPlan(Seq(attrInt), 10L)),
-      Seq(attrInt -> colStatInt.copy(distinctCount = 8)),
-      expectedRowCount = 8)
+      Seq(attrInt -> colStatInt.copy(distinctCount = 7)),


Shall we add new test cases for filter estimation based on histogram, instead of modifying existing test results?

SparkQA · 2017-11-26T03:07:22Z

Test build #84190 has finished for PR 19783 at commit 8e5d04e.

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

SparkQA · 2017-11-28T04:17:55Z

Test build #84236 has finished for PR 19783 at commit 052d111.

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

wzhfy · 2017-11-29T03:25:46Z

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

+   * @return the number of the first bin/bucket into which a column values falls.
+   */
+
+  def findFirstBucketForValue(value: Double, histogram: Histogram): Int = {


Shall we unify all names to bin/bins in code and comments?

We had bucket(s) and bin(s) used interchangeably. To avoid confusion, I will unify them to use only bin/bins.

SparkQA · 2017-11-30T01:12:26Z

Test build #84317 has finished for PR 19783 at commit 241089c.

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

wzhfy · 2017-11-30T02:17:04Z

...ain/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala

-      Some(1.0 / BigDecimal(ndv))
-    } else {
+      // We compute filter selectivity using Histogram information
+      attr.dataType match {


use if (colStat.histogram.isEmpty) to seperate the logic of basic stats (Some(1.0 / BigDecimal(ndv))) and histogram computation.

wzhfy · 2017-11-30T02:17:54Z

...ain/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala

@@ -332,8 +332,45 @@ case class FilterEstimation(plan: Filter) extends Logging {
        colStatsMap.update(attr, newStats)
      }

-      Some(1.0 / BigDecimal(ndv))
-    } else {
+      // We compute filter selectivity using Histogram information


move this comment where the histogram computation really starts

wzhfy · 2017-11-30T02:30:44Z

...ain/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala

+          // returns 1/ndv if there is no histogram
+          if (colStat.histogram.isEmpty) return Some(1.0 / BigDecimal(ndv))
+
+          // We traverse histogram bins to locate the literal value


This comment is not accurate, here we want to get the bins occupied by the literal value, because if the value is skewed, it can occupy multiple bins.

wzhfy · 2017-11-30T02:31:13Z

...ain/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala

+          // We traverse histogram bins to locate the literal value
+          val hgmBins = colStat.histogram.get.bins
+          val datum = EstimationUtils.toDecimal(literal.value, literal.dataType).toDouble
+          // find the interval where this datum locates


we can remove this comment, it's explained above

wzhfy · 2017-11-30T02:32:33Z

...ain/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala

+          // find the interval where this datum locates
+          var lowerId, higherId = -1
+          for (i <- hgmBins.indices) {
+            // if datum > upperBound, just move to next bin


please remove the comment, it does not match the logic at next line (there's no "move" logic)

wzhfy · 2017-11-30T02:35:39Z

...ain/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala

+              (1.0 / hgmBins.length) / math.max(lowerBinNdv, 1) +
+              (1.0 / hgmBins.length) / math.max(higherBinNdv, 1)
+          }
+          Some(percent)


How about simplifying the above logic as:

val occupiedBins = if (lowerId == higherId) { 1.0 / lowerBinNdv } else { (higherId - lowerId - 1) + 1.0 / lowerBinNdv + 1.0 / higherBinNdv } Some(occupiedBins / hgmBins.length)

Good point.

wzhfy · 2017-11-30T03:26:17Z

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

+        binId += 1
+      }
+      if ((value == histogram.bins(i).hi) && (i < histogram.bins.length - 1)) {
+        if (value == histogram.bins(i + 1).lo) {


merge two ifs: if ((value == histogram.bins(i).hi) && (value == histogram.bins(i + 1).lo) && (i < histogram.bins.length - 1))

I used two statements instead of one statement is because, when i points to the last bin, this condition "value == histogram.bins(i + 1).lo" may be out of bound. By separating the conditions into two statements, we can be sure that the out-of-bound error will not happen.

By "out of bound", do you mean it exceeds 100 length limit? You can just switch new line after &&

No. I meant the upper bound for the array of bins in a histogram. The default length of the histogram bin array is 254. When i is equal to 253 (the last bin), then i+1 is 254 leading to out-of-bound error.

just move this condition after the length check:

if ((value == histogram.bins(i).hi) && (i < histogram.bins.length - 1) && (value == histogram.bins(i + 1).lo))

wzhfy · 2017-11-30T03:35:30Z

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

+   * @param histogram a numeric equi-height histogram
+   * @return the number of the first bin into which a column values falls.
+   */
+


could you remove the empty line between method comment and its definition?
same for other methods here.

wzhfy · 2017-11-30T03:39:35Z

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

+      histogram: Histogram): Double = {
+    // find bins where current min and max locate
+    val minBinId = findFirstBinForValue(lowerEnd, histogram)
+    val maxBinId = findLastBinForValue(higherEnd, histogram)


how about lowerBinId, higherBinId?

wzhfy · 2017-11-30T03:42:20Z

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

+      1.0
+    } else if (binId == 0 && curBin.hi != curBin.lo) {
+      if (higherValue == lowerValue) {
+        // in the case curBin.binNdv == 0, current bin is occupied by one value, which


binNdv will never be zero

wzhfy · 2017-11-30T03:44:07Z

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

+    } else if (lowerId == higherId) {
+      getOccupation(lowerId, higherEnd, lowerEnd, histogram) * histogram.bins(lowerId).ndv
+    } else {
+      // compute how much lowerEnd/higherEnd occupy its bin


typo: occupies its bin

SparkQA · 2017-12-01T05:25:32Z

Test build #84365 has finished for PR 19783 at commit 6e6c49b.

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

SparkQA · 2017-12-02T05:33:26Z

Test build #84385 has finished for PR 19783 at commit 9d2a463.

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

cloud-fan · 2017-12-05T05:02:15Z

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

+  def findFirstBinForValue(value: Double, bins: Array[HistogramBin]): Int = {
+    var binId = 0
+    bins.foreach { bin =>
+      if (value > bin.hi) binId += 1


this looks more like a while loop pattern, can we use while loop here?

Good point. Actually while loop is better because it can exit early when the condition no longer qualifies.

cloud-fan · 2017-12-05T05:04:45Z

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

+   * @param bins an array of bins for a given numeric equi-height histogram
+   * @return the number of the last bin into which a column values falls.
+   */
+  def findLastBinForValue(value: Double, bins: Array[HistogramBin]): Int = {


Why is this method so different from findFirstBinForValue? It looks like we just need to reverse the iteration order, i.e. from bins.length to 0.

Good point. We can simplify the logic by iterating from bins.length-1 to 0.

cloud-fan · 2017-12-05T05:09:44Z

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

+        1.0 / curBin.ndv.toDouble
+      } else {
+        // Use proration since the range falls inside this bin.
+        (higherValue - lowerValue) / (curBin.hi - curBin.lo)


this is the only branch we need to specialize for binId=0.

why do we need to specialize it?

cloud-fan · 2017-12-05T05:35:37Z

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

+   * @param higherEnd a given upper bound value of a specified column value range
+   * @param lowerEnd a given lower bound value of a specified column value range
+   * @param histogram a numeric equi-height histogram
+   * @return the selectivity percentage for column values in [lowerEnd, higherEnd].


this doesn't match the java doc: Returns the number of bins...

SparkQA · 2017-12-06T04:36:22Z

Test build #84517 has finished for PR 19783 at commit d068888.

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

cloud-fan · 2017-12-06T13:10:42Z

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

+   *
+   * @param value a literal value of a column
+   * @param bins an array of bins for a given numeric equi-height histogram
+   * @return the number of the first bin into which a column values falls.


nit: the id of the first bin into which the given value falls.

cloud-fan · 2017-12-06T13:11:03Z

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

+  def findFirstBinForValue(value: Double, bins: Array[HistogramBin]): Int = {
+    var i = 0
+    while ((i < bins.length) && (value > bins(i).hi)) {
+      i +=1


nit: s space after +=

cloud-fan · 2017-12-06T13:11:32Z

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

@@ -114,4 +114,171 @@ object EstimationUtils {
    }
  }

+  /**
+   * Returns the number of the first bin into which a column values falls for a specified


cloud-fan · 2017-12-06T13:11:52Z

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

+  }
+
+  /**
+   * Returns the number of the last bin into which a column values falls for a specified


cloud-fan · 2017-12-06T13:11:59Z

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

+   *
+   * @param value a literal value of a column
+   * @param bins an array of bins for a given numeric equi-height histogram
+   * @return the number of the last bin into which a column values falls.


cloud-fan · 2017-12-06T13:12:12Z

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

+  def findLastBinForValue(value: Double, bins: Array[HistogramBin]): Int = {
+    var i = bins.length - 1
+    while ((i >= 0) && (value < bins(i).lo)) {
+      i -=1


a space after -=

cloud-fan · 2017-12-06T13:15:05Z

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

+      binId: Int,
+      higherValue: Double,
+      lowerValue: Double,
+      histogram: Histogram): Double = {


the method signature looks weird, shouldn't it be

private def getOccupation( higherValue: Double, lowerValue: Double, bin: HistogramBin)

cloud-fan · 2017-12-06T13:17:40Z

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

+    val curBin = histogram.bins(binId)
+    if (curBin.hi == curBin.lo) {
+      // the entire bin is covered in the range
+      1.0


I don't get it, shouldn't we check lowerValue <= curBin.lo <= higherValue here?

cloud-fan · 2017-12-06T13:18:48Z

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

+      1.0
+    } else if (higherValue == lowerValue) {
+      // set percentage to 1/NDV
+      1.0 / curBin.ndv.toDouble


shouldn't we check the lowerValue/higherValues fits in the bin value range?

cloud-fan · 2017-12-07T15:51:53Z

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

+      // of next bin is equal to the hi value of the previous bin.  We bump up
+      // ndv value only if the hi values of two consecutive bins are different.
+      var middleNdv: Long = 0
+      for (i <- histogram.bins.indices) {


again this is a typical while loop pattern.

cloud-fan · 2017-12-07T15:52:55Z

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

+      var middleNdv: Long = 0
+      for (i <- histogram.bins.indices) {
+        val bin = histogram.bins(i)
+        if (bin.hi != bin.lo && i >= lowerId + 1 && i <= higherId - 1) {


var i = lowerId + 1 while (i < higherId) { ... i += 1 }

cloud-fan · 2017-12-07T15:53:54Z

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

+      // The total ndv is minPartNdv + maxPartNdv + Ndvs between them.
+      // In order to avoid counting same distinct value twice, we check if the upperBound value
+      // of next bin is equal to the hi value of the previous bin.  We bump up
+      // ndv value only if the hi values of two consecutive bins are different.


this doesn't match the code, the actual logic is: only if the lo and hi values of the bin are different

I will change the comment so that it matches with the code. Actually my original comment means the same thing as your comment. This is because the hi value of a bin is equal to the lo value of the next bin.

cloud-fan · 2017-12-07T15:55:02Z

...ain/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala

+        // Here we traverse histogram bins to locate the range of bins the literal values falls
+        // into.  For skewed distribution, a literal value can occupy multiple bins.
+        val hgmBins = colStat.histogram.get.bins
+        val datum = EstimationUtils.toDecimal(literal.value, literal.dataType).toDouble


cc @wzhfy , you would refactor this part to always use Double for CBO computing?

yes, I'll refactor this part.

cloud-fan · 2017-12-07T16:04:53Z

...ain/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala

+              ((datum == hgmBins(i).hi) && (datum < hgmBins(i + 1).hi))) {
+              higherId = i
+            }
+           }


how about

var lowerId = -1 var highIdFound = false var i = 0 while (i < hgmBins.length || highIdFound) { if (datum <= hgmBins(i).hi && lowerId < 0) lowerId = i if (datum >= hgmBins(i).lo) highIdFound = true } val highId = i

cloud-fan · 2017-12-07T16:28:37Z

LGTM overall

wzhfy · 2017-12-08T02:21:36Z

...ain/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala

+
+    // compute how many bins the column's current valid range [min, max] occupies.
+    // Note that a column's [min, max] range may vary after we apply some filter conditions.
+    val minToMaxLength = EstimationUtils.getOccupationBins(maxBinId, minBinId, max,


Personally I prefer to have this method unit-tested, because it's the core part of filter estimation. We can do this in follow-up anyway.

SparkQA · 2017-12-11T01:38:03Z

Test build #84700 has finished for PR 19783 at commit be1e7ba.

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

ron8hu · 2017-12-11T19:09:42Z

retest this please.

SparkQA · 2017-12-11T21:59:18Z

Test build #84725 has finished for PR 19783 at commit be1e7ba.

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

gatorsmile · 2017-12-11T22:41:06Z

retest this please

SparkQA · 2017-12-12T01:34:41Z

Test build #84732 has finished for PR 19783 at commit be1e7ba.

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

ron8hu · 2017-12-12T01:43:33Z

For the past 2 test builds #84725 and #84732, I checked the test result on the web. Actually there were no failures. See https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84725/testReport/. It appears that there is a bug in the jenkins test system.

ron8hu · 2017-12-12T03:36:00Z

retest this please.

SparkQA · 2017-12-12T06:33:54Z

Test build #84751 has finished for PR 19783 at commit be1e7ba.

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

cloud-fan

LGTM except some code style issue, we can address them later

cloud-fan · 2017-12-12T06:38:00Z

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

@@ -114,4 +114,99 @@ object EstimationUtils {
    }
  }

+  /**
+   * Returns the number of the first bin into which a column value falls for a specified


nit: number -> index?

cloud-fan · 2017-12-12T06:38:49Z

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

+   *
+   * @param value a literal value of a column
+   * @param bins an array of bins for a given numeric equi-height histogram
+   * @return the id of the first bin into which a column value falls.


Seems this is redundant, shall we remove it?

cloud-fan · 2017-12-12T06:39:02Z

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

+  }
+
+  /**
+   * Returns the number of the last bin into which a column value falls for a specified


cloud-fan · 2017-12-12T06:39:08Z

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

+   *
+   * @param value a literal value of a column
+   * @param bins an array of bins for a given numeric equi-height histogram
+   * @return the id of the last bin into which a column value falls.


cloud-fan · 2017-12-12T06:39:36Z

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

+   * @param higherValue a given upper bound value of a specified column value range
+   * @param lowerValue a given lower bound value of a specified column value range
+   * @param bin a single histogram bin
+   * @return the percentage of a single bin holding values in [lowerValue, higherValue].


cloud-fan · 2017-12-12T06:43:00Z

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

+   * double value is because we may return a portion of a bin. For example, a predicate
+   * "column = 8" may return the number of bins 0.2 if the holding bin has 5 distinct values.
+   *
+   * @param higherId id of the high end bin holding the high end value of a column range


nit: higherIndex

cloud-fan · 2017-12-12T06:45:12Z

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

+    } else {
+      // compute how much lowerEnd/higherEnd occupies its bin
+      val lowerCurBin = histogram.bins(lowerId)
+      val lowerPart = getOccupation(lowerCurBin.hi, lowerEnd, lowerCurBin)


shall we assert that lowerBin.lo <= lowerEnd

cloud-fan · 2017-12-12T06:46:09Z

...ain/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala

+        // returns 1/ndv if there is no histogram
+        Some(1.0 / BigDecimal(ndv))
+      } else {
+        // We compute filter selectivity using Histogram information.


did you create a new method?

cloud-fan · 2017-12-12T06:47:10Z

...ain/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala

+        // range may change due to another condition applied earlier.
+        val min = EstimationUtils.toDecimal(colStat.min.get, literal.dataType).toDouble
+        val max = EstimationUtils.toDecimal(colStat.max.get, literal.dataType).toDouble
+        val minBinId = EstimationUtils.findFirstBinForValue(min, hgmBins)


nit: minBinIndex

cloud-fan · 2017-12-12T06:50:39Z

...ain/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/FilterEstimation.scala

+        val lowerBinNdv = hgmBins(lowerBinId).ndv
+        val higherBinNdv = hgmBins(higherBinId).ndv
+        // assume uniform distribution in each bin
+        val occupiedBins = if (lowerBinId == higherBinId) {


is this just EstimationUtils.getOccupationBins(higherBinId, lowerBinId, datum, datum, histogram)?

cloud-fan · 2017-12-12T07:05:05Z

thanks, merging to master!

ron8hu changed the title ~~support histogram in filter cardinality estimation~~ [SPARK-21322][SQL] support histogram in filter cardinality estimation Nov 19, 2017

wzhfy reviewed Nov 20, 2017

View reviewed changes

ron8hu force-pushed the supportHistogram branch from dd5b975 to 8e5d04e Compare November 26, 2017 00:15

wzhfy reviewed Nov 29, 2017

View reviewed changes

ron8hu force-pushed the supportHistogram branch from 052d111 to 241089c Compare November 29, 2017 22:19

wzhfy reviewed Nov 30, 2017

View reviewed changes

ron8hu force-pushed the supportHistogram branch from 6e6c49b to 9d2a463 Compare December 2, 2017 02:40

cloud-fan reviewed Dec 5, 2017

View reviewed changes

ron8hu force-pushed the supportHistogram branch from 9d2a463 to d068888 Compare December 6, 2017 01:50

cloud-fan reviewed Dec 6, 2017

View reviewed changes

cloud-fan reviewed Dec 7, 2017

View reviewed changes

wzhfy reviewed Dec 8, 2017

View reviewed changes

ron8hu added 10 commits December 10, 2017 15:20

support histogram in filter cardinality estimation

4a39bda

add histogram test cases without modifying the existing tests

f326feb

use a skewed distribution for histogram test cases

776d45d

change bucket to bin in code and comment

af39604

refactor code based on comments from wzhfy

53e4979

add histogram test cases for non-skewed distribution

5d2e505

simplify the logic in findLastBinForValue method

5d97ad3

simplify the logic in getOccupation method

4158392

refactor getOccupation method

a7d23e1

refactor evaluateEquality method

be1e7ba

ron8hu force-pushed the supportHistogram branch from c9538b8 to be1e7ba Compare December 10, 2017 23:29

cloud-fan approved these changes Dec 12, 2017

View reviewed changes

asfgit closed this in ecc179e Dec 12, 2017

[SPARK-21322][SQL] support histogram in filter cardinality estimation #19783

[SPARK-21322][SQL] support histogram in filter cardinality estimation #19783

Conversation

ron8hu commented Nov 19, 2017

What changes were proposed in this pull request?

How was this patch tested?

SparkQA commented Nov 19, 2017

ron8hu commented Nov 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Nov 26, 2017

SparkQA commented Nov 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Nov 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 1, 2017

SparkQA commented Dec 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloud-fan commented Dec 7, 2017

Choose a reason for hiding this comment

SparkQA commented Dec 11, 2017

ron8hu commented Dec 11, 2017

SparkQA commented Dec 11, 2017

gatorsmile commented Dec 11, 2017

SparkQA commented Dec 12, 2017

ron8hu commented Dec 12, 2017

ron8hu commented Dec 12, 2017

SparkQA commented Dec 12, 2017

cloud-fan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloud-fan commented Dec 12, 2017