[SPARK-21351][SQL] Update nullability based on children's output #18576

maropu · 2017-07-09T06:22:33Z

What changes were proposed in this pull request?

This pr added a new optimizer rule UpdateNullabilityInAttributeReferences to update the nullability that Filter changes when having IsNotNull. In the master, optimized plans do not respect the nullability when Filter has IsNotNull. This wrongly generates unnecessary code. For example:

scala> val df = Seq((Some(1), Some(2))).toDF("a", "b")
scala> val bIsNotNull = df.where($"b" =!= 2).select($"b")
scala> val targetQuery = bIsNotNull.distinct
scala> val targetQuery.queryExecution.optimizedPlan.output(0).nullable
res5: Boolean = true

scala> targetQuery.debugCodegen
Found 2 WholeStageCodegen subtrees.
== Subtree 1 / 2 ==
*HashAggregate(keys=[b#19], functions=[], output=[b#19])
+- Exchange hashpartitioning(b#19, 200)
   +- *HashAggregate(keys=[b#19], functions=[], output=[b#19])
      +- *Project [_2#16 AS b#19]
         +- *Filter isnotnull(_2#16)
            +- LocalTableScan [_1#15, _2#16]

Generated code:
...
/* 124 */   protected void processNext() throws java.io.IOException {
...
/* 132 */     // output the result
/* 133 */
/* 134 */     while (agg_mapIter.next()) {
/* 135 */       wholestagecodegen_numOutputRows.add(1);
/* 136 */       UnsafeRow agg_aggKey = (UnsafeRow) agg_mapIter.getKey();
/* 137 */       UnsafeRow agg_aggBuffer = (UnsafeRow) agg_mapIter.getValue();
/* 138 */
/* 139 */       boolean agg_isNull4 = agg_aggKey.isNullAt(0);
/* 140 */       int agg_value4 = agg_isNull4 ? -1 : (agg_aggKey.getInt(0));
/* 141 */       agg_rowWriter1.zeroOutNullBytes();
/* 142 */
                // We don't need this NULL check because NULL is filtered out in `$"b" =!=2`
/* 143 */       if (agg_isNull4) {
/* 144 */         agg_rowWriter1.setNullAt(0);
/* 145 */       } else {
/* 146 */         agg_rowWriter1.write(0, agg_value4);
/* 147 */       }
/* 148 */       append(agg_result1);
/* 149 */
/* 150 */       if (shouldStop()) return;
/* 151 */     }
/* 152 */
/* 153 */     agg_mapIter.close();
/* 154 */     if (agg_sorter == null) {
/* 155 */       agg_hashMap.free();
/* 156 */     }
/* 157 */   }
/* 158 */
/* 159 */ }

In the line 143, we don't need this NULL check because NULL is filtered out in $"b" =!=2.
This pr could remove this NULL check;

scala> val targetQuery.queryExecution.optimizedPlan.output(0).nullable
res5: Boolean = false

scala> targetQuery.debugCodegen
...
Generated code:
...
/* 144 */   protected void processNext() throws java.io.IOException {
...
/* 152 */     // output the result
/* 153 */
/* 154 */     while (agg_mapIter.next()) {
/* 155 */       wholestagecodegen_numOutputRows.add(1);
/* 156 */       UnsafeRow agg_aggKey = (UnsafeRow) agg_mapIter.getKey();
/* 157 */       UnsafeRow agg_aggBuffer = (UnsafeRow) agg_mapIter.getValue();
/* 158 */
/* 159 */       int agg_value4 = agg_aggKey.getInt(0);
/* 160 */       agg_rowWriter1.write(0, agg_value4);
/* 161 */       append(agg_result1);
/* 162 */
/* 163 */       if (shouldStop()) return;
/* 164 */     }
/* 165 */
/* 166 */     agg_mapIter.close();
/* 167 */     if (agg_sorter == null) {
/* 168 */       agg_hashMap.free();
/* 169 */     }
/* 170 */   }

How was this patch tested?

Added UpdateNullabilityInAttributeReferencesSuite for unit tests.

maropu · 2017-07-09T06:25:26Z

@gatorsmile I probably missed something though...) Is it an expected design to ignore this kind of nullability (I saw your tests related to this issue here)?

SparkQA · 2017-07-09T07:04:55Z

Test build #79401 has finished for PR 18576 at commit fe5ecff.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2017-07-09T15:16:35Z

Jenkins, retest this please.

SparkQA · 2017-07-09T16:52:08Z

Test build #79423 has finished for PR 18576 at commit fe5ecff.

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

SparkQA · 2017-07-10T05:05:57Z

Test build #79430 has finished for PR 18576 at commit d79f976.

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

kiszk · 2017-07-10T05:53:06Z

...la/org/apache/spark/sql/catalyst/optimizer/UpdateNullabilityInAttributeReferencesSuite.scala

+    try {
+      SQLConf.get.setConf(SQLConf.CONSTRAINT_PROPAGATION_ENABLED, true)
+      val logicalPlan = testRelation.where('a =!= 2).select('a).analyze
+      var expectedSchema = new StructType().add("a", "INT", nullable = true)


nit: Can we use val by using a different variable at line 47?

@gatorsmile suggested we better do so for multiple use in assertion.

kiszk · 2017-07-10T06:00:21Z

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

@@ -132,7 +132,9 @@ abstract class Optimizer(sessionCatalog: SessionCatalog)
      OptimizeCodegen) ::
    Batch("RewriteSubquery", Once,
      RewritePredicateSubquery,
-      CollapseProject) :: Nil
+      CollapseProject) ::
+    Batch("UpdateAttributeReferences", Once,


Can we apply this optimization earlier to encourage other optimizations that uses information on nullable?
For example, before Operator Optimizations?

Or, is there any reason to put this optimization at the end of the optimizations?

The original motivation is to fix nullability inconsistency in logical plans and I couldn't find concrete examples to make the optimization better by using this new rule. Any idea?

ueshin · 2017-07-10T06:24:21Z

...la/org/apache/spark/sql/catalyst/optimizer/UpdateNullabilityInAttributeReferencesSuite.scala

+  test("update nullability when inferred constraints applied")  {
+    val testRelation = LocalRelation('a.int, 'b.int)
+    try {
+      SQLConf.get.setConf(SQLConf.CONSTRAINT_PROPAGATION_ENABLED, true)


nit: should we use PlanTest.withSQLConf here?

I'll update. Thanks!

SparkQA · 2017-07-10T07:04:56Z

Test build #79445 has finished for PR 18576 at commit 841179f.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2017-07-10T07:12:07Z

Jenkins, retest this please.

SparkQA · 2017-07-10T09:39:49Z

Test build #79449 has finished for PR 18576 at commit 841179f.

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

maropu · 2017-07-10T10:48:19Z

Jenkins, retest this please.

SparkQA · 2017-07-10T13:10:55Z

Test build #79463 has finished for PR 18576 at commit 841179f.

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

maropu · 2017-07-20T02:31:17Z

ping

maropu · 2017-08-06T01:43:24Z

@gatorsmile If you get time, could you also check this? Thanks!

gatorsmile · 2017-08-06T03:11:38Z

If we can update the nullability in Optimizer rules, do we still need to do it in FilterExec?

maropu · 2017-08-06T03:32:16Z

Yea, if we can do so, I feel it might be the best. I'll check if we can remove nullability update in FilterExec.

SparkQA · 2017-08-06T05:22:17Z

Test build #80293 has finished for PR 18576 at commit aba4eba.

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

SparkQA · 2017-08-06T12:54:12Z

Test build #80303 has finished for PR 18576 at commit be174d1.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class FilterExec(condition: Expression, child: SparkPlan, override val output: Seq[Attribute])

SparkQA · 2017-08-06T12:59:23Z

Test build #80304 has finished for PR 18576 at commit e92a9b8.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class FilterExec(condition: Expression, child: SparkPlan, output: Seq[Attribute])

SparkQA · 2017-08-06T15:02:43Z

Test build #80305 has finished for PR 18576 at commit a6d2743.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class FilterExec(condition: Expression, child: SparkPlan, output: Seq[Attribute])

maropu · 2017-08-06T17:15:43Z

I'm looking into the failure reason...

SparkQA · 2017-08-07T04:22:46Z

Test build #80315 has finished for PR 18576 at commit 5d2fd6d.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class FilterExec(condition: Expression, child: SparkPlan, outputAttrs: Seq[Attribute])

maropu · 2017-08-07T05:04:44Z

sql/core/src/main/scala/org/apache/spark/sql/execution/basicPhysicalOperators.scala

-        a
-      }
-    }
+    child.output.map { attr => outputAttrs.find(_.exprId == attr.exprId).getOrElse(attr) }
  }


I simply tried to drop updating nullability and reuse output attributes outputAttrs in an optimized logical plan here though, some python tests failed (all the scala tests passed). I checked and I found this; in the planner path of python, we have some cases changing operator's output from the optimized logical plan to a physical plan.
For example;

sql("""SELECT strlen(a) FROM test WHERE strlen(a) > 1""") // pyspark >>> spark.sql("SELECT strlen(a) FROM test WHERE strlen(a) > 1").explain(True) ... == Optimized Logical Plan == Project [strlen(a#0) AS strlen(a)#30] +- Filter (strlen(a#0) > 1) +- LogicalRDD [a#0] == Physical Plan == *Project [pythonUDF0#34 AS strlen(a)#30] +- BatchEvalPython [strlen(a#0)], [a#0, pythonUDF0#34] +- *Filter (pythonUDF0#33 > 1), [a#0] +- BatchEvalPython [strlen(a#0)], [a#0, pythonUDF0#33] +- Scan ExistingRDD[a#0]

So, I added code to check a difference between outputAttrs and child.output here.
Could you give me insight on this? @gatorsmile

maropu · 2018-03-28T05:41:25Z

ok, I'll do.

maropu · 2018-03-28T06:04:09Z

An alternative solution is here and this is based on the suggestion.

SparkQA · 2018-03-28T06:09:35Z

Test build #88653 has finished for PR 18576 at commit b9c4d98.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class FilterExec(condition: Expression, child: SparkPlan, outputAttrs: Seq[Attribute])

cloud-fan · 2018-03-28T08:23:22Z

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

+      val nullabilityMap = AttributeMap(p.children.flatMap(_.output).map { x => x -> x.nullable })
+      p transformExpressions {
+        case ar: AttributeReference =>
+          nullabilityMap.get(ar).filterNot(_ == ar.nullable).map { nullable =>


can't we just write

case a: AttributeReference if nullabilityMap.contains(a) => a.withNullability(nullabilityMap(a))

cloud-fan · 2018-03-28T08:32:45Z

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

+ * Updates nullability in [[AttributeReference]]s if nullability is different between
+ * non-leaf plan's expressions and the children output.
+ */
+object UpdateNullabilityInAttributeReferences extends Rule[LogicalPlan] {


this rule itself is useful without the filter optimization.

In general, I don't think it's useful to optimize filter using the IsNotNull expression, as it's not a common case. I think we can update Join.output to specify nullability for join keys, which seems easier and a more common case.

ok, I'll make this pr simpler (I'll drop some changes for filters).

I dropped the changes of execution.FilterExec though, you suggested we would drop the changes of logical.Filter, too? https://github.com/apache/spark/pull/18576/files#diff-72917e7b68f0311b2fb42990e0dc616dR139

I basically agree that the Join.output modification is more simple/important, but is it okay to ignore nullability in logical.Filter? For example, in the current master, QueryPlanConstraints.inferIsNotNullConstraints appends non-nullable constraints in logical.Filter and this constraints aren't correctly propagated into upper plan nodes now. So, I think it'd be better to respect nullability in both logical.Join and logical.Filter.

If we think more about how these IsNotNull constraints are generated, they mostly come from join. I don't think it's a common case users put IsNotNull in filter.

ok, I'll drop it in this pr. just a sec.

SparkQA · 2018-03-28T10:07:13Z

Test build #88657 has finished for PR 18576 at commit 5cd41d3.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class FilterExec(condition: Expression, child: SparkPlan, outputAttrs: Seq[Attribute])

maropu · 2018-03-28T10:14:56Z

retest this please

SparkQA · 2018-03-28T12:43:52Z

Test build #88662 has finished for PR 18576 at commit 5cd41d3.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class FilterExec(condition: Expression, child: SparkPlan, outputAttrs: Seq[Attribute])

SparkQA · 2018-03-28T16:28:10Z

Test build #88665 has finished for PR 18576 at commit 5b7de7f.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class FilterExec(condition: Expression, child: SparkPlan)

SparkQA · 2018-03-31T03:47:18Z

Test build #88772 has finished for PR 18576 at commit 65ee6f7.

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

maropu · 2018-03-31T03:52:43Z

@cloud-fan I’m ganna to fix the join stuffs described in your comment in following prs, ok?

maropu · 2018-04-02T08:11:43Z

I checked if we could fix the join issue: c4ca4f3

maropu · 2018-04-04T06:11:32Z

ping

cloud-fan · 2018-04-04T06:39:51Z

thanks, merging to master!

## What changes were proposed in this pull request? This pr added a new optimizer rule `UpdateNullabilityInAttributeReferences ` to update the nullability that `Filter` changes when having `IsNotNull`. In the master, optimized plans do not respect the nullability when `Filter` has `IsNotNull`. This wrongly generates unnecessary code. For example: ``` scala> val df = Seq((Some(1), Some(2))).toDF("a", "b") scala> val bIsNotNull = df.where($"b" =!= 2).select($"b") scala> val targetQuery = bIsNotNull.distinct scala> val targetQuery.queryExecution.optimizedPlan.output(0).nullable res5: Boolean = true scala> targetQuery.debugCodegen Found 2 WholeStageCodegen subtrees. == Subtree 1 / 2 == *HashAggregate(keys=[b#19], functions=[], output=[b#19]) +- Exchange hashpartitioning(b#19, 200) +- *HashAggregate(keys=[b#19], functions=[], output=[b#19]) +- *Project [_2#16 AS b#19] +- *Filter isnotnull(_2#16) +- LocalTableScan [_1#15, _2#16] Generated code: ... /* 124 */ protected void processNext() throws java.io.IOException { ... /* 132 */ // output the result /* 133 */ /* 134 */ while (agg_mapIter.next()) { /* 135 */ wholestagecodegen_numOutputRows.add(1); /* 136 */ UnsafeRow agg_aggKey = (UnsafeRow) agg_mapIter.getKey(); /* 137 */ UnsafeRow agg_aggBuffer = (UnsafeRow) agg_mapIter.getValue(); /* 138 */ /* 139 */ boolean agg_isNull4 = agg_aggKey.isNullAt(0); /* 140 */ int agg_value4 = agg_isNull4 ? -1 : (agg_aggKey.getInt(0)); /* 141 */ agg_rowWriter1.zeroOutNullBytes(); /* 142 */ // We don't need this NULL check because NULL is filtered out in `$"b" =!=2` /* 143 */ if (agg_isNull4) { /* 144 */ agg_rowWriter1.setNullAt(0); /* 145 */ } else { /* 146 */ agg_rowWriter1.write(0, agg_value4); /* 147 */ } /* 148 */ append(agg_result1); /* 149 */ /* 150 */ if (shouldStop()) return; /* 151 */ } /* 152 */ /* 153 */ agg_mapIter.close(); /* 154 */ if (agg_sorter == null) { /* 155 */ agg_hashMap.free(); /* 156 */ } /* 157 */ } /* 158 */ /* 159 */ } ``` In the line 143, we don't need this NULL check because NULL is filtered out in `$"b" =!=2`. This pr could remove this NULL check; ``` scala> val targetQuery.queryExecution.optimizedPlan.output(0).nullable res5: Boolean = false scala> targetQuery.debugCodegen ... Generated code: ... /* 144 */ protected void processNext() throws java.io.IOException { ... /* 152 */ // output the result /* 153 */ /* 154 */ while (agg_mapIter.next()) { /* 155 */ wholestagecodegen_numOutputRows.add(1); /* 156 */ UnsafeRow agg_aggKey = (UnsafeRow) agg_mapIter.getKey(); /* 157 */ UnsafeRow agg_aggBuffer = (UnsafeRow) agg_mapIter.getValue(); /* 158 */ /* 159 */ int agg_value4 = agg_aggKey.getInt(0); /* 160 */ agg_rowWriter1.write(0, agg_value4); /* 161 */ append(agg_result1); /* 162 */ /* 163 */ if (shouldStop()) return; /* 164 */ } /* 165 */ /* 166 */ agg_mapIter.close(); /* 167 */ if (agg_sorter == null) { /* 168 */ agg_hashMap.free(); /* 169 */ } /* 170 */ } ``` ## How was this patch tested? Added `UpdateNullabilityInAttributeReferencesSuite` for unit tests. Author: Takeshi Yamamuro <[email protected]> Closes apache#18576 from maropu/SPARK-21351.

cloud-fan · 2018-12-26T15:06:08Z

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

-      RemoveRedundantProject)
+      RemoveRedundantProject) :+
+    Batch("UpdateAttributeReferences", Once,
+      UpdateNullabilityInAttributeReferences)


I noticed that this rule is very similar to FixNullability. Do we have to keep both of them? cc @maropu @gatorsmile

UpdateNullabilityInAttributeReferences is an optimizer rule that was added after we introduced the analyzer rule FixNullability. Do we have any end-to-end test case for showing the benefit of this optimizer rule?

yea, I have the same opinion with @gatorsmile; FixNullability is related to the correctness, but UpdateNullabilityInAttributeReferences is not (just for optimization).

@gatorsmile I think we haven't checked actual performance benefits (e.g., wall time) from the rule now. I just assume that this rule could generate better code from the code generator.

But these 2 rules are almost same, except that FixNullability skips resolved plan.

Yea, I see... But, I have no idea about the way to resolve the two issue (the correctness issue and the optimization issue) simultaneously in a single place. I think there is one greedy & temporary solution that merges the current two rule into one, and then put it in the two places: the analyzer and the optimizer.

…h FixNullability ## What changes were proposed in this pull request? This is a followup of #18576 The newly added rule `UpdateNullabilityInAttributeReferences` does the same thing the `FixNullability` does, we only need to keep one of them. This PR removes `UpdateNullabilityInAttributeReferences`, and use `FixNullability` to replace it. Also rename it to `UpdateAttributeNullability` ## How was this patch tested? existing tests Closes #23390 from cloud-fan/nullable. Authored-by: Wenchen Fan <[email protected]> Signed-off-by: Takeshi Yamamuro <[email protected]>

…h FixNullability ## What changes were proposed in this pull request? This is a followup of apache#18576 The newly added rule `UpdateNullabilityInAttributeReferences` does the same thing the `FixNullability` does, we only need to keep one of them. This PR removes `UpdateNullabilityInAttributeReferences`, and use `FixNullability` to replace it. Also rename it to `UpdateAttributeNullability` ## How was this patch tested? existing tests Closes apache#23390 from cloud-fan/nullable. Authored-by: Wenchen Fan <[email protected]> Signed-off-by: Takeshi Yamamuro <[email protected]>

maropu force-pushed the SPARK-21351 branch from fe5ecff to d79f976 Compare July 10, 2017 02:41

kiszk reviewed Jul 10, 2017

View reviewed changes

ueshin reviewed Jul 10, 2017

View reviewed changes

maropu force-pushed the SPARK-21351 branch from aba4eba to be174d1 Compare August 6, 2017 12:51

maropu force-pushed the SPARK-21351 branch from be174d1 to e92a9b8 Compare August 6, 2017 12:56

maropu force-pushed the SPARK-21351 branch from e92a9b8 to a6d2743 Compare August 6, 2017 13:01

maropu force-pushed the SPARK-21351 branch 2 times, most recently from c552a6e to 5d2fd6d Compare August 7, 2017 01:46

maropu commented Aug 7, 2017

View reviewed changes

maropu force-pushed the SPARK-21351 branch from 84d32ef to b9c4d98 Compare March 28, 2018 06:01

maropu force-pushed the SPARK-21351 branch from b9c4d98 to 5cd41d3 Compare March 28, 2018 07:29

cloud-fan reviewed Mar 28, 2018

View reviewed changes

maropu added 3 commits March 31, 2018 08:08

Update nullability based on children's output

0f90ae4

Fix

12a88e8

Fix

65ee6f7

maropu force-pushed the SPARK-21351 branch from 5b7de7f to 65ee6f7 Compare March 31, 2018 00:19

asfgit closed this in 5197562 Apr 4, 2018

maropu mentioned this pull request Apr 25, 2018

[SPARK-24079][SQL] Update the nullability of Join output based on inferred predicates #21148

Closed

cloud-fan reviewed Dec 26, 2018

View reviewed changes

cloud-fan mentioned this pull request Dec 27, 2018

[SPARK-26459][SQL] replace UpdateNullabilityInAttributeReferences with FixNullability #23390

Closed

maropu mentioned this pull request Jul 16, 2019

[SPARK-27915][SQL][WIP] Update logical Filter's output nullability based on IsNotNull conditions #24765

Closed

[SPARK-21351][SQL] Update nullability based on children's output #18576

[SPARK-21351][SQL] Update nullability based on children's output #18576

Conversation

maropu commented Jul 9, 2017

What changes were proposed in this pull request?

How was this patch tested?

maropu commented Jul 9, 2017

SparkQA commented Jul 9, 2017

maropu commented Jul 9, 2017

SparkQA commented Jul 9, 2017

SparkQA commented Jul 10, 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

maropu Jul 10, 2017 • edited Loading

Choose a reason for hiding this comment

SparkQA commented Jul 10, 2017

maropu commented Jul 10, 2017

SparkQA commented Jul 10, 2017

maropu commented Jul 10, 2017

SparkQA commented Jul 10, 2017

maropu commented Jul 20, 2017

maropu commented Aug 6, 2017

gatorsmile commented Aug 6, 2017

maropu commented Aug 6, 2017

SparkQA commented Aug 6, 2017

SparkQA commented Aug 6, 2017

SparkQA commented Aug 6, 2017

SparkQA commented Aug 6, 2017

maropu commented Aug 6, 2017

SparkQA commented Aug 7, 2017

maropu Aug 7, 2017 • edited Loading

Choose a reason for hiding this comment

maropu commented Mar 28, 2018

maropu commented Mar 28, 2018

SparkQA commented Mar 28, 2018

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 Mar 28, 2018

maropu commented Mar 28, 2018

SparkQA commented Mar 28, 2018

SparkQA commented Mar 28, 2018

SparkQA commented Mar 31, 2018

maropu commented Mar 31, 2018

maropu commented Apr 2, 2018

maropu commented Apr 4, 2018

cloud-fan commented Apr 4, 2018

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

maropu Jul 10, 2017 •

edited

Loading

maropu Aug 7, 2017 •

edited

Loading