[SPARK-33798][SQL] Add new rule to push down the foldable expressions through CaseWhen/If #30790

wangyum · 2020-12-16T01:35:35Z

What changes were proposed in this pull request?

This pr add a new rule(PushFoldableIntoBranches) to push down the foldable expressions through CaseWhen/If. This is a real case from production:

create table t1 using parquet as select * from range(100);
create table t2 using parquet as select * from range(200);

create temp view v1 as                                                             
select 'a' as event_type, * from t1                                                
union all                                                                          
select CASE WHEN id = 1 THEN 'b' WHEN id = 3 THEN 'c' end as event_type, * from t2 

explain select * from v1 where event_type = 'a';

Before this PR:

== Physical Plan ==
Union
:- *(1) Project [a AS event_type#30533, id#30535L]
:  +- *(1) ColumnarToRow
:     +- FileScan parquet default.t1[id#30535L] Batched: true, DataFilters: [], Format: Parquet
+- *(2) Project [CASE WHEN (id#30536L = 1) THEN b WHEN (id#30536L = 3) THEN c END AS event_type#30534, id#30536L]
   +- *(2) Filter (CASE WHEN (id#30536L = 1) THEN b WHEN (id#30536L = 3) THEN c END = a)
      +- *(2) ColumnarToRow
         +- FileScan parquet default.t2[id#30536L] Batched: true, DataFilters: [(CASE WHEN (id#30536L = 1) THEN b WHEN (id#30536L = 3) THEN c END = a)], Format: Parquet

After this PR:

== Physical Plan ==
*(1) Project [a AS event_type#8, id#4L]
+- *(1) ColumnarToRow
   +- FileScan parquet default.t1[id#4L] Batched: true, DataFilters: [], Format: Parquet

Why are the changes needed?

Improve query performance.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Unit test.

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

wangyum · 2020-12-16T01:38:46Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

+    assertEquivalent(
+      EqualTo(CaseWhen(Seq((a, Literal(null, IntegerType))), Some(Literal(1))), Literal(2)),
+      FalseLiteral)
+    assertEquivalent(
+      EqualTo(CaseWhen(Seq((a, Literal(1))), Some(Literal(2))), Literal(null, IntegerType)),
+      FalseLiteral)


SPARK-33798 will change these behaviors as expected.

wangyum · 2020-12-16T01:38:57Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

+    assertEquivalent(
+      EqualTo(CaseWhen(Seq((a, Literal(null, IntegerType))), Some(Literal(1))), Literal(1)),
+      EqualTo(CaseWhen(Seq((a, Literal(null, IntegerType))), Some(Literal(1))), Literal(1)))
+    assertEquivalent(
+      EqualTo(CaseWhen(Seq((a, Literal(null, IntegerType))), Some(Literal(null, IntegerType))),
+        Literal(1)),
+      Literal(null, BooleanType))
+    assertEquivalent(
+      EqualTo(CaseWhen(Seq((a, Literal(null, IntegerType))), Some(Literal(null, IntegerType))),
+        Literal(null, IntegerType)),
+      Literal(null, BooleanType))


SPARK-33798 will not change these behaviors.

SparkQA · 2020-12-16T02:46:30Z

Test build #132846 has finished for PR 30790 at commit 19b0a83.

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

cloud-fan · 2020-12-16T02:58:49Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

+        FalseLiteral
+
+      case EqualTo(c @ CaseWhen(branches, elseValue), right: Literal) if c.deterministic &&
+          (branches.map(_._2) ++ elseValue).forall(_.isInstanceOf[Literal]) &&


we can do the "is literal check" inside isAlwaysFalse, to shorten the code.

cloud-fan · 2020-12-16T03:04:00Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

+    assertEquivalent(EqualTo(nonFoldable, Literal(1)), EqualTo(nonFoldable, Literal(1)))
+
+    // Do not simplify if it contains non-deterministic expressions.
+    val nonDeterministic = If(LessThan(Rand(1), Literal(0.5)), Literal(1), Literal(1))


This is a case that we should optimize. But we need to distinguish between non-deterministic and has-side-effect. While we can skip running non-deterministic expressions, we can't skip running has-side-effect expressions.

This is not related to this PR, but is something worth considering. cc @viirya @dbtsai @maropu @rednaxelafx

cloud-fan · 2020-12-16T03:13:22Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

+
+    // null check, SPARK-33798 will not change these behaviors.
+    assertEquivalent(
+      EqualTo(If(FalseLiteral, Literal(null, IntegerType), Literal(1)), Literal(1)),


The if condition is a literal, does it really go through the new branch? will it be optimized earlier by some other logics?

I'd prefer to still use a === Literal(1) as the if conditionn, and make sure the expression is not optimized if there are null values.

SparkQA · 2020-12-16T03:14:56Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37448/

SparkQA · 2020-12-16T03:50:43Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37448/

SparkQA · 2020-12-16T07:04:30Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37463/

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

SparkQA · 2020-12-16T07:47:11Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37463/

cloud-fan · 2020-12-16T08:20:40Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

@@ -470,6 +470,11 @@ object SimplifyConditionals extends Rule[LogicalPlan] with PredicateHelper {
    case _ => false
  }

+  private def isAlwaysFalse(exps: Seq[Expression], equalTo: Literal): Boolean = {
+    exps.forall(_.isInstanceOf[Literal]) &&
+      exps.forall(!EqualTo(_, equalTo).eval(EmptyRow).asInstanceOf[Boolean])


EqualTo may return null, we need to take care of it, because null is not false in SQL.

Yes. Null value should be handled by NullPropagation.

dongjoon-hyun

Thank you for making a new one, @wangyum .

cloud-fan · 2020-12-16T09:00:02Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

@@ -523,6 +529,17 @@ object SimplifyConditionals extends Rule[LogicalPlan] with PredicateHelper {
        } else {
          e.copy(branches = branches.take(i).map(branch => (branch._1, elseValue)))
        }
+
+      // Null value should be handled by NullPropagation.


It's better to make optimizer rules orthogonal and don't rely on each other. Let's make isAlwaysFalse stricter:

exps.forall { e => val res = EqualTo(_, equalTo).eval(EmptyRow) res != null && !res.asInstanceOf[Boolean] }

How about:

exps.forall { case l: Literal => val res = EqualTo(l, equalTo).eval(EmptyRow) res != null && !res.asInstanceOf[Boolean] case _ => false }

otherwise this test will fail:

val nonFoldable = CaseWhen(Seq(normalBranch, (a, b)), None) assertEquivalent(EqualTo(nonFoldable, Literal(1)), EqualTo(nonFoldable, Literal(1)))

SparkQA · 2020-12-16T10:02:10Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37479/

SparkQA · 2020-12-16T10:32:42Z

Test build #132861 has finished for PR 30790 at commit 831bf66.

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

SparkQA · 2020-12-16T10:35:30Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37479/

SparkQA · 2020-12-16T11:56:51Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37485/

SparkQA · 2020-12-16T12:25:53Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37485/

SparkQA · 2020-12-16T13:24:39Z

Test build #132878 has finished for PR 30790 at commit 859893d.

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

cloud-fan · 2020-12-16T13:28:43Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

+  test("SPARK-33798: simplify EqualTo(If, Literal) always false") {
+    val a = EqualTo(UnresolvedAttribute("a"), Literal(100))
+    val b = UnresolvedAttribute("b")
+    val ifExp = If(a === Literal(1), Literal(2), Literal(3))


Let's make sure the test expression is valid. a is an EqualTo, comparing boolean and Literal(1) is invalid.

cloud-fan · 2020-12-16T14:29:40Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

+
+    assertEquivalent(EqualTo(ifExp, Literal(4)), FalseLiteral)
+    assertEquivalent(EqualTo(ifExp, Literal(3)), EqualTo(ifExp, Literal(3)))
+    assertEquivalent(EqualTo(ifExp, Literal("4")), FalseLiteral)


ditto, let's don't test invalid expressions, ifExp.dataType is int, not string.

cloud-fan · 2020-12-16T14:33:12Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

+    assertEquivalent(
+      EqualTo(If(a, b, Literal(2)), Literal(3)),
+      EqualTo(If(a, b, Literal(2)), Literal(3)))
+    val nonFoldable = If(NonFoldableLiteral(true), Literal(1), Literal(2))


NonFoldableLiteral is nothing special comparing to 'a == 100, as they are both non-foldable. I think we don't need to test with NonFoldableLiteral

maropu · 2020-12-17T07:47:20Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

+ * Push the foldable expression into (if / case) branches.
+ */
+object PushFoldableIntoBranches extends Rule[LogicalPlan] with PredicateHelper {
+  def apply(plan: LogicalPlan): LogicalPlan = plan transform {


nit: we cannot use transformAllExpressions here?

This copied from

spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

Line 474 in 8ccc3c1

case q: LogicalPlan => q transformExpressionsUp {

Ah, I see. I think its okay to simply use transformAllExpressions here.

SparkQA · 2020-12-17T08:00:26Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37542/

SparkQA · 2020-12-17T08:28:53Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37542/

SparkQA · 2020-12-17T10:45:22Z

Test build #132935 has finished for PR 30790 at commit f9f622f.

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

SparkQA · 2020-12-17T11:16:23Z

Test build #132939 has finished for PR 30790 at commit 8ccc3c1.

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

cloud-fan · 2020-12-17T18:25:35Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

+          falseValue = b.makeCopy(Array(falseValue, right)))
+
+      case b @ BinaryComparison(c @ CaseWhen(branches, elseValue), right) if c.deterministic &&
+          right.foldable && (branches.map(_._2) ++ elseValue).forall(_.foldable) =>


some more ideas:

I think it's helpful to pushdown foldables into branches as long as there is at lease one foldable branche. To be conservative, we can do the pushdown if the IF/CASE WHEN has at most one non-foldable branch, so that we never increase the expression tree size, so is the generated code size.

I think it's also useful to pushdown expressions like Add, e.g. IF(cond, 1, 2) +1 -> IF(cond, 2, 3). We can use BinnaryExpression instead of BinaryComparison.

dongjoon-hyun · 2020-12-18T00:30:53Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

+/**
+ * Push the foldable expression into (if / case) branches.
+ */
+object PushFoldableIntoBranches extends Rule[LogicalPlan] with PredicateHelper {


HyukjinKwon · 2020-12-18T04:02:27Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

+  def apply(plan: LogicalPlan): LogicalPlan = plan transform {
+    case q: LogicalPlan => q transformExpressionsUp {
+      case b @ BinaryExpression(i @ If(_, trueValue, falseValue), right) if i.deterministic &&
+          right.foldable && (trueValue.foldable || falseValue.foldable) =>


Sorry if I missed something but can you explain why we need this foldable condition?

Shouldn't bin_op((if cond: a else: b), c) be same as if cond: bin_op(a, c) else: bin_op(b, c) as long as the results are deterministic?

Ohh, okay but we just want to push down foldables only that can have benefits. gotya.

SparkQA · 2020-12-18T04:08:22Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37583/

SparkQA · 2020-12-18T04:36:13Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37583/

cloud-fan · 2020-12-18T04:54:30Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

+          trueValue = b.makeCopy(Array(trueValue, right)),
+          falseValue = b.makeCopy(Array(falseValue, right)))
+
+      case b @ BinaryExpression(left, i @ If(_, trueValue, falseValue)) if i.deterministic &&


Do we care about deterministic here? This optimization doesn't change the execution order of the branch conditions and/or branch values.

cloud-fan · 2020-12-18T04:58:45Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

+          falseValue = b.makeCopy(Array(falseValue, right)))
+
+      case b @ BinaryExpression(left, i @ If(_, trueValue, falseValue)) if i.deterministic &&
+          left.foldable && (trueValue.foldable || falseValue.foldable) =>


let's create a method to count foldables, with comments to explain why

// be conservative here: it's only a guaranteed win if all but at most only one branch // end up being not foldable. private def atMostOneUnfoldable(exprs: Seq[Expression]): Boolean = ...

then here left.foldable && atMostOneUnfoldable(Seq(trueValue, falseValue))

Do we need to push down this case, only one branch and non foldable:

assertEquivalent(EqualTo(CaseWhen(Seq((a, b)), None), Literal(1)), CaseWhen(Seq((a, EqualTo(b, Literal(1)))), None))

How about?

// To be conservative here: it's only a guaranteed win if all but at most only one branch // end up being not foldable. private def atMostOneUnfoldable(exprs: Seq[Expression]): Boolean = { val (foldables, others) = exprs.partition(_.foldable) foldables.nonEmpty && others.length < 2 }

cloud-fan · 2020-12-18T05:02:00Z

...t/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PushFoldableIntoBranchesSuite.scala

+    comparePlans(actual, correctAnswer)
+  }
+
+  test("SPARK-33798: Push down EqualTo through If") {


nit: when creating a new test suite, we don't need to add the JIRA ID as prefix.

cloud-fan · 2020-12-18T05:03:16Z

...t/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PushFoldableIntoBranchesSuite.scala

+
+  test("SPARK-33798: Push down EqualTo through If") {
+    assertEquivalent(EqualTo(ifExp, Literal(4)), FalseLiteral)
+    assertEquivalent(EqualTo(ifExp, Literal(3)), If(a, FalseLiteral, TrueLiteral))


If(a, FalseLiteral, TrueLiteral) can be turned into !a. We can optimize it in followups.

Yes. we can optimize it:
https://github.com/apache/spark/pull/30849/files#diff-d43484d56a4d9991066b5c00d12ec2465c75131e055fc02ee7fb6dfd45b5006fR478-R479

cloud-fan · 2020-12-18T05:03:58Z

...t/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PushFoldableIntoBranchesSuite.scala

+  test("SPARK-33798: Push down EqualTo through If") {
+    assertEquivalent(EqualTo(ifExp, Literal(4)), FalseLiteral)
+    assertEquivalent(EqualTo(ifExp, Literal(3)), If(a, FalseLiteral, TrueLiteral))
+    assertEquivalent(EqualTo(ifExp, Literal("4")), FalseLiteral)


let's not test invalid expressions, EqualTo can't compare int and string.

cloud-fan · 2020-12-18T05:05:53Z

...t/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PushFoldableIntoBranchesSuite.scala

+    assertEquivalent(EqualTo(caseWhen, Literal(4)), FalseLiteral)
+    assertEquivalent(EqualTo(caseWhen, Literal(3)),
+      CaseWhen(Seq((a, FalseLiteral), (c, FalseLiteral)), Some(TrueLiteral)))
+    assertEquivalent(EqualTo(caseWhen, Literal("4")), FalseLiteral)


ditto, don't test invalid expressions.

cloud-fan · 2020-12-18T05:10:48Z

...t/src/test/scala/org/apache/spark/sql/catalyst/optimizer/PushFoldableIntoBranchesSuite.scala

+  private val b = UnresolvedAttribute("b")
+  private val c = EqualTo(UnresolvedAttribute("c"), Literal(true))
+  private val ifExp = If(a, Literal(2), Literal(3))
+  private val caseWhen = CaseWhen(Seq((a, Literal(1)), (c, Literal(2))), Some(Literal(3)))


without the else value, seems like we can't optimize CASE WHEN cond1 THEN false WHEN cond2 THEN false into false. We should fix it in followups.

viirya · 2020-12-18T06:30:44Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

+  def apply(plan: LogicalPlan): LogicalPlan = plan transform {
+    case q: LogicalPlan => q transformExpressionsUp {
+      case b @ BinaryExpression(i @ If(_, trueValue, falseValue), right) if i.deterministic &&
+          right.foldable && (trueValue.foldable || falseValue.foldable) =>


This is good idea when both trueValue and falseValue are foldable like the example in the description.

But if only trueValue is foldable? For example, from If(_, 'b', col1) = 'a', we get If(_, 'b' = 'a', col1 = 'a'), are some benefits here too?

The differences in generated code:

Hmm, I'm not sure if the difference means obvious benefits. Looks like it should be very close but I think I'm okay with it.

SparkQA · 2020-12-18T07:37:56Z

Test build #132982 has finished for PR 30790 at commit 45b56fc.

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

SparkQA · 2020-12-18T09:07:52Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37604/

SparkQA · 2020-12-18T09:37:51Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37604/

SparkQA · 2020-12-18T13:18:26Z

Test build #133005 has finished for PR 30790 at commit cfac0e8.

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

cloud-fan · 2020-12-18T13:20:56Z

thanks, merging to master!

Simplify EqualTo(CaseWhen/If, Literal) always false

19b0a83

wangyum commented Dec 16, 2020

View reviewed changes

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala Outdated Show resolved Hide resolved

wangyum commented Dec 16, 2020

View reviewed changes

github-actions bot added the SQL label Dec 16, 2020

cloud-fan reviewed Dec 16, 2020

View reviewed changes

Address comments

831bf66

cloud-fan reviewed Dec 16, 2020

View reviewed changes

...talyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Dec 16, 2020

View reviewed changes

dongjoon-hyun reviewed Dec 16, 2020

View reviewed changes

Null value should be handled by NullPropagation.

859893d

cloud-fan reviewed Dec 16, 2020

View reviewed changes

Handle Null values

db584ef

cloud-fan reviewed Dec 16, 2020

View reviewed changes

address comments

55f4528

cloud-fan reviewed Dec 16, 2020

View reviewed changes

maropu reviewed Dec 17, 2020

View reviewed changes

cloud-fan reviewed Dec 17, 2020

View reviewed changes

dongjoon-hyun reviewed Dec 18, 2020

View reviewed changes

fix

45b56fc

HyukjinKwon reviewed Dec 18, 2020

View reviewed changes

HyukjinKwon approved these changes Dec 18, 2020

View reviewed changes

cloud-fan reviewed Dec 18, 2020

View reviewed changes

viirya reviewed Dec 18, 2020

View reviewed changes

viirya approved these changes Dec 18, 2020

View reviewed changes

fix

cfac0e8

cloud-fan approved these changes Dec 18, 2020

View reviewed changes

cloud-fan closed this in 06b1bbb Dec 18, 2020

wangyum deleted the SPARK-33798 branch December 18, 2020 16:15

[SPARK-33798][SQL] Add new rule to push down the foldable expressions through CaseWhen/If #30790

[SPARK-33798][SQL] Add new rule to push down the foldable expressions through CaseWhen/If #30790

Conversation

wangyum commented Dec 16, 2020 • edited Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 16, 2020

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 16, 2020

SparkQA commented Dec 16, 2020

SparkQA commented Dec 16, 2020

SparkQA commented Dec 16, 2020

cloud-fan Dec 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dongjoon-hyun left a 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 16, 2020

SparkQA commented Dec 16, 2020

SparkQA commented Dec 16, 2020

SparkQA commented Dec 16, 2020

SparkQA commented Dec 16, 2020

SparkQA commented Dec 16, 2020

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 17, 2020

SparkQA commented Dec 17, 2020

SparkQA commented Dec 17, 2020

SparkQA commented Dec 17, 2020

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 18, 2020

SparkQA commented Dec 18, 2020

Choose a reason for hiding this comment

cloud-fan Dec 18, 2020 • edited Loading

Choose a reason for hiding this comment

wangyum Dec 18, 2020 • edited Loading

Choose a reason for hiding this comment

wangyum Dec 18, 2020 • edited Loading

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

This comment was marked as spam.

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 18, 2020

SparkQA commented Dec 18, 2020

SparkQA commented Dec 18, 2020

SparkQA commented Dec 18, 2020

cloud-fan commented Dec 18, 2020

wangyum commented Dec 16, 2020 •

edited

Loading

cloud-fan Dec 16, 2020 •

edited

Loading

cloud-fan Dec 18, 2020 •

edited

Loading

wangyum Dec 18, 2020 •

edited

Loading

wangyum Dec 18, 2020 •

edited

Loading