Add disjunctive predicate for filtered aggregates #11425

atris · 2018-09-05T18:07:28Z

Filtered aggregates do not make an explicit predicate of the filter clause
that is present. This limits the ability of the optimizer to potentially
push down the predicate to table scan node. This commit adds a filter node
with a disjunctive predicate formed from all the filter clauses present in
the aggregation node's aggregates

kokosing

looks good % minor comments

kokosing · 2018-09-05T19:07:05Z

...com/facebook/presto/sql/planner/iterative/rule/TestAddPredicateBelowFilteredAggregation.java

+    public void rewriteAddFilterWithMultipleFilters()
+    {
+        assertPlan(
+                "SELECT sum(totalprice) FILTER(WHERE totalprice > 0), sum(custkey) FILTER(WHERE custkey > 0) FROM orders",


Please add also a test with:

SELECT sum(totalprice) FILTER(WHERE totalprice > 0), sum(custkey) FROM orders

kokosing · 2018-09-05T19:07:36Z

...com/facebook/presto/sql/planner/iterative/rule/TestAddPredicateBelowFilteredAggregation.java

+    @Test
+    public void rewriteAddFilter()
+    {
+        assertPlan(


can you please also assert the results of these test queries?

kokosing · 2018-09-05T19:08:15Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

                                context.getIdAllocator().getNextId(),
-                                aggregation.getSource(),
-                                newAssignments.build()),
+                                projectNode,


kokosing · 2018-09-05T19:10:05Z

...com/facebook/presto/sql/planner/iterative/rule/TestAddPredicateBelowFilteredAggregation.java

+import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.project;
+import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.tableScan;
+
+public class TestAddPredicateBelowFilteredAggregation


Please rename to TestImplementFilteredAggregations

TestImplementFilteredAggregations exists today, these tests are specifically for filter clause pushdown

No one will find this class. And filter clause pushdown is a part of ImplementFilteredAggregations.

kokosing · 2018-09-05T19:11:22Z

...com/facebook/presto/sql/planner/iterative/rule/TestAddPredicateBelowFilteredAggregation.java

+import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.tableScan;
+
+public class TestAddPredicateBelowFilteredAggregation
+        extends BasePlanTest


The best would be if you could extend from BaseRuleTest

kokosing · 2018-09-06T08:19:24Z

Notice that this optimization works only for default aggregations.
Consider the case: SELECT count(a) FILTER (WHERE b > 0) FROM t GROUP BY c, in case of none of the rows matches the condition then we still need to output row per each group c with null output for aggregate function count(a)

kokosing · 2018-09-10T06:05:13Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

@@ -20,6 +20,8 @@
 import com.facebook.presto.sql.planner.plan.AggregationNode;


Please add to the commit message information about rule application space, that this only work only for default aggregation.

kokosing · 2018-09-10T06:06:56Z

...com/facebook/presto/sql/planner/iterative/rule/TestAddPredicateBelowFilteredAggregation.java

+import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.project;
+import static com.facebook.presto.sql.planner.assertions.PlanMatchPattern.tableScan;
+
+public class TestAddPredicateBelowFilteredAggregation


No one will find this class. And filter clause pushdown is a part of ImplementFilteredAggregations.

kokosing · 2018-09-10T06:09:22Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

@@ -92,7 +97,17 @@ public Result apply(AggregationNode aggregation, Captures captures, Context cont
                verify(!mask.isPresent(), "Expected aggregation without mask symbols, see Rule pattern");
                newAssignments.put(symbol, filter);
                mask = Optional.of(symbol);
+
+                filterExpressions.add(filter);


Instead of modifying this loop, I would introduce another method that would extract the predicate that you could pushdown or BooleanLiteral.TRUE if it is not possible extract such a predicate.

kokosing · 2018-09-10T06:10:11Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

+                        context.getIdAllocator().getNextId(),
+                        aggregationNode.getSource(),
+                        assignments),
+                combineDisjuncts(filterExpressions));


What if combineDisjuncts(filterExpressions) evaluates to True or False?

Use combineDisjunctsWithDefault(filterExpressions, TRUE)

kokosing · 2018-09-10T06:11:06Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

+            Assignments assignments,
+            boolean doNotOptimizeForFilterPredicates)
+    {
+        if (doNotOptimizeForFilterPredicates) {


I do not think this flag is needed

kokosing · 2018-09-10T06:12:14Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

+
+                filterExpressions.add(filter);
+
+                if (aggregation.hasNonEmptyGroupingSet()) {


If would change it to:

if (aggregation.hasNonEmptyGroupingSet()) { filterExpressions.add(TRUE); else { filterExpressions.add(filter); }

kokosing · 2018-09-10T06:16:20Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

+            boolean doNotOptimizeForFilterPredicates)
+    {
+        if (doNotOptimizeForFilterPredicates) {
+            return new ProjectNode(


extract this as variable and reuse below

kokosing

Almost there

kokosing · 2018-09-11T04:45:47Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

@@ -92,7 +100,11 @@ public Result apply(AggregationNode aggregation, Captures captures, Context cont
                verify(!mask.isPresent(), "Expected aggregation without mask symbols, see Rule pattern");
                newAssignments.put(symbol, filter);
                mask = Optional.of(symbol);
+


undo, such empty line can cause the build failure

kokosing · 2018-09-11T04:46:14Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

@@ -78,6 +84,8 @@ public Result apply(AggregationNode aggregation, Captures captures, Context cont
    {
        Assignments.Builder newAssignments = Assignments.builder();
        ImmutableMap.Builder<Symbol, Aggregation> aggregations = ImmutableMap.builder();
+        ImmutableList.Builder<Expression> filterExpressions = ImmutableList.builder();
+        boolean doNotOptimizeForFilterPredicate = false;


remove this flag

kokosing · 2018-09-11T04:47:47Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

                        aggregations.build(),
                        aggregation.getGroupingSets(),
                        ImmutableList.of(),
                        aggregation.getStep(),
                        aggregation.getHashSymbol(),
                        aggregation.getGroupIdSymbol()));
    }
+
+    private Expression getFilterPredicateToAdd(AggregationNode node, FunctionCall aggregate)


s/getFilterPredicateToAdd/getFilterPredicate

kokosing · 2018-09-11T04:50:36Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

+                                        context.getIdAllocator().getNextId(),
+                                        aggregation.getSource(),
+                                        newAssignments.build()),
+                                combineDisjunctsWithDefault(filterExpressions.build(), TRUE_LITERAL)),


let's do not add FilterNode when combineDisjunctsWithDefault(filterExpressions.build(), TRUE_LITERAL) evaluates to TRUE and please add returnValuesNode when evaluates to FALSE. Make sure you covered this cases with unit tests (checking the query output and plan).

As discussed offline, I will add tests only in this PR and let the upper optimiser rules handle this.

kokosing · 2018-09-12T08:37:13Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+    }
+
+    @Test
+    public void testDoNotRewriteIfNonFilteredAggregateIsPresent()


this is a wrong test name, as rule always rewrite filtered aggregations, but here it does not push down predicates

kokosing · 2018-09-12T08:38:45Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+                                        "sum_13", functionCall("sum", ImmutableList.of("custkey"))),
+                                        project(
+                                                tableScan("orders", ImmutableMap.of("totalprice", "totalprice",
+                                                        "custkey", "custkey"))))))));


if you split line with arguments, please put each argument in separate line.

This is the last time I comment this, next time I will write just eh.

Sorry about that

kokosing · 2018-09-12T08:39:38Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+    }
+
+    @Test
+    public void testRewriteLiteralFilterToValuesNode()


update test name, say something about predicate push donw

kokosing · 2018-09-12T08:40:03Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+                "SELECT sum(totalprice) FILTER(WHERE FALSE) FROM orders",
+                anyTree(
+                        aggregation(ImmutableMap.of("sum_6", functionCall("sum", ImmutableList.of("totalprice"))),
+                                        project(


assert that predicate was pushed down

Since the expected plan tests for the presence of ValuesNode with same projections as original source, shouldn't that rewrite be a check that the FILTER with constant predicate was pushed down?

kokosing · 2018-09-12T08:40:43Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+                anyTree(
+                        aggregation(ImmutableMap.of("sum_2", functionCall("sum", ImmutableList.of("sum_13")),
+                                "sum", functionCall("sum", ImmutableList.of("sum_12"))),
+                                exchange(aggregation(ImmutableMap.of("sum_12", functionCall("sum", ImmutableList.of("totalprice")),


I do not see any assert that you check if FilterNode has TRUE predicate

kokosing · 2018-09-12T08:41:30Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+
+    @Test
+    public void testDoNotRewriteIfNonFilteredAggregateIsPresent()
+    {


add test like SELECT sum(totalprice) FILTER(WHERE totalprice > 0) FROM orders ORDER BY x

kokosing · 2018-09-13T05:57:20Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+    public void testAddPredicateForFilterClauses()
+    {
+        assertions.assertQuery(
+                "SELECT sum(x) FILTER(WHERE x > 0) FROM (VALUES 1, 1, 0, 2, 3, 3) t(x)",


Please test also SELECT sum(x) FILTER(WHERE x > 0) FROM (VALUES 1, 1, 0, 2, 3, 3) t(x) GROUP BY x

I would ask , what does t(x) mean ? I have never seen this kind of way of expression. Could you tell me more ?

t(x) is a pseudo representation of a table with a single column x and rows 1, 1, 0, 2, 3, 3

kokosing · 2018-09-13T05:58:48Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+                                        "sum_15", functionCall("sum", ImmutableList.of("custkey"))),
+                                        project(
+                                                filter(
+                                                        "(\"totalprice\" > 0E0 OR \"custkey\" > BIGINT '0')",


why 0E0 and not DOUBLE '0'?

The test does not pass with DOUBLE '0'

kokosing · 2018-09-13T05:59:27Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+                                        aggregation(
+                                                ImmutableMap.of("sum_14", functionCall("sum", ImmutableList.of("totalprice")),
+                                        "sum_15", functionCall("sum", ImmutableList.of("custkey"))),
+                                        project(


I would put all the above as anyTree

kokosing · 2018-09-13T06:06:24Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+
+        assertPlan(
+                "SELECT sum(totalprice) FILTER(WHERE TRUE) FROM orders",
+                anyTree(


instead you can use something like: com.facebook.presto.sql.planner.TestLogicalPlanner#assertPlanContainsNoApplyOrAnyJoin to see if there is no filter node

kokosing · 2018-09-13T06:06:51Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+                anyTree(
+                        aggregation(
+                                ImmutableMap.of("sum_6", functionCall("sum", ImmutableList.of("totalprice"))),
+                                        project(


I would wrap all the pattern above as anyTree

kokosing · 2018-09-13T06:07:32Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+    {
+        assertPlan(
+                "SELECT sum(totalprice) FILTER(WHERE totalprice > 0), sum(custkey) FROM orders",
+                anyTree(


instead you can use something like: com.facebook.presto.sql.planner.TestLogicalPlanner#assertPlanContainsNoApplyOrAnyJoin to see if there is no filter node

Why you haven't used assertPlanContainsNoFilter here?

kokosing · 2018-09-13T06:08:30Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+    public void testDoNotPushdownPredicateIfNonFilteredAggregateIsPresent()
+    {
+        assertPlan(
+                "SELECT sum(totalprice) FILTER(WHERE totalprice > 0), sum(custkey) FROM orders",


Test also a plan for SELECT sum(totalprice) FILTER(WHERE totalprice > 0) FROM orders GROUP BY totalprice

kokosing

% minor things

kokosing · 2018-09-14T06:18:48Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+
+        assertPlanContainsNoFilter("SELECT sum(totalprice) FILTER(WHERE TRUE) FROM orders");
+
+        assertPlanContainsNoFilter("SELECT sum(x) FILTER(WHERE x > 0) FROM (VALUES 1, 1, 0, 2, 3, 3) t(x) GROUP BY x");


this does not push down any filter predicate, so it does not belong to testPushDownConstantFilterPredicate, same with assertion below

Good point. I have added a new test, testNoFilterAddedForConstantValueFilters and added the two tests.

kokosing · 2018-09-14T06:20:31Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+    {
+        assertPlan(
+                "SELECT sum(totalprice) FILTER(WHERE totalprice > 0), sum(custkey) FROM orders",
+                anyTree(


Why you haven't used assertPlanContainsNoFilter here?

atris · 2018-09-14T06:40:07Z

@kokosing I wanted the plan to be more readable for the case when one aggregate has a filter and other does not, might be just me though. I have changed it now.

kokosing · 2018-09-14T06:44:34Z

I wanted the plan to be more readable for the case when one aggregate has a filter and other does not

I see, but plan pattern are expensive to maintain, so we if there is a way to avoid without losing coverage them let's avoid them.

kokosing

minor comments

CC: @sopel39 would you like to take a look?

kokosing · 2018-09-14T11:04:07Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+    }
+
+    @Test
+    public void testNoFilterAddedForConstantValueFilters()


s/testNoFilterAddedForConstantValueFilters/testNoFilterAddedFoNonDefaultAggregation

kokosing · 2018-09-14T11:04:45Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

+    @Test
+    public void testPushDownConstantFilterPredicate()
+    {
+        assertPlanContainsNoFilter("SELECT sum(totalprice) FILTER(WHERE FALSE) FROM orders");


add a comment that filter node was optimized

kokosing · 2018-09-14T11:05:44Z

presto-main/src/test/java/com/facebook/presto/sql/query/TestFilteredAggregations.java

@@ -34,6 +46,22 @@ public void teardown()
        assertions = null;
    }

+    @Test
+    public void testAddPredicateForFilterClauses()


s/testNoFilterAddedForConstantValueFilters/testFilterdAggregationPredicatePushdown

sopel39

Update ImplementFilteredAggregations javadoc (to include filter)

sopel39 · 2018-09-14T14:14:08Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

@@ -78,6 +81,7 @@ public Result apply(AggregationNode aggregation, Captures captures, Context cont
    {
        Assignments.Builder newAssignments = Assignments.builder();
        ImmutableMap.Builder<Symbol, Aggregation> aggregations = ImmutableMap.builder();
+        ImmutableList.Builder<Expression> filterExpressions = ImmutableList.builder();


Collect mask symbols instead:

ImmutableList.Builder<Expression> maskSymbols = ...

sopel39 · 2018-09-14T14:14:16Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

@@ -93,6 +97,9 @@ public Result apply(AggregationNode aggregation, Captures captures, Context cont
                newAssignments.put(symbol, filter);
                mask = Optional.of(symbol);
            }
+
+            filterExpressions.add(getFilterPredicate(aggregation, call));


sopel39 · 2018-09-14T14:14:30Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

                        aggregations.build(),
                        aggregation.getGroupingSets(),
                        ImmutableList.of(),
                        aggregation.getStep(),
                        aggregation.getHashSymbol(),
                        aggregation.getGroupIdSymbol()));
    }
+
+    private Expression getFilterPredicate(AggregationNode node, FunctionCall aggregate)


sopel39 · 2018-09-14T14:19:24Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

@@ -93,6 +97,9 @@ public Result apply(AggregationNode aggregation, Captures captures, Context cont
                newAssignments.put(symbol, filter);
                mask = Optional.of(symbol);


Add mask symbol to the list:

maskSymbols.add(symbol.toSymbolReference());

sopel39 · 2018-09-14T14:21:34Z

.../main/java/com/facebook/presto/sql/planner/iterative/rule/ImplementFilteredAggregations.java

@@ -105,15 +112,34 @@ public Result apply(AggregationNode aggregation, Captures captures, Context cont
        return Result.ofPlanNode(
                new AggregationNode(
                        context.getIdAllocator().getNextId(),
-                        new ProjectNode(
+                        new FilterNode(


add FilterNode above ProjectNode using maskSymbols:

Expression predicate = TRUE_LITERAL; if (!node.hasNonEmptyGroupingSet()) { predicate = combineDisjunctsWithDefault(maskSymbols.build(), TRUE_LITERAL)); } new FilterNode( ..., predicate, ...

This way we won't do redundant computations of expressions

This also needs to consider if any aggregate without a FILTER clause is present, made the change and fixed

sopel39 · 2018-09-17T15:51:14Z

I'm actually thinking that producing OR predicate (on different columns) might not be the best approach. Such predicate cannot be pushed to connectors and it can stuck high in the plan. It might also cover most of the data.

It doesn't make sense to have FilterNode just below AggregationNode as it doesn't bring benefit and might actually decrease performance. Keep in mind that global partial aggregations do not require dedicated data partitioning and therefore can be pushed low in the plan (even in source stages).

Therefore I think we should do an additional step when producing predicate. We should first try to extract tuple domain from the large OR expression (built using actual filter expressions) using DomainTranslator#fromPredicate and then convert produced TupleDomain back to predicate using com.facebook.presto.sql.planner.DomainTranslator#toPredicate only if such TupleDomain is not TupleDomain.all().
Such predicate would be much more pushable conjunction. I think its safer approach. What do you think?

CC: @martint

It would also be good to have examples of queries that execute much faster because of this PR.

@sopel39

due @sopel39 comments

kokosing · 2018-09-18T06:50:17Z

It doesn't make sense to have FilterNode just below AggregationNode as it doesn't bring benefit and might actually decrease performance.

It would be nice to know the overhead it introduces.

Keep in mind that global partial aggregations do not require dedicated data partitioning and therefore can be pushed low in the plan (even in source stages).

This PR only addresses global aggregation.

Therefore I think we should do an additional step when producing predicate. We should first try to extract tuple domain from the large OR expression (built using actual filter expressions) using DomainTranslator#fromPredicate and then convert produced TupleDomain back to predicate using com.facebook.presto.sql.planner.DomainTranslator#toPredicate only if such TupleDomain is not TupleDomain.all().

It does not guarantee anything as you can calculate domain on intermediate (not source) symbols. I think easier would to extract symbols from each disjunct and push filter node only when all disjuncts are using single same symbol, otherwise it will only produce tuple domain all.

sopel39 · 2018-09-18T08:49:29Z

After offline discussion we decided that we can go ahead with current implementation. The rationale is that even if such large OR might not be pushed down to connector it can still improve IO by filtering lazy pages even if partial aggregation is in source stage.

It would be good though to provide some query (could be based on TPCH) that would show performance improvement (should be fairly easy to produce either with or without partitions).

If we find this optimization to be problematic in the future we can always limit it's use to single filtered aggregation.

Thanks @atris ! Could you still apply my review comments (#11425 (review))?

Filtered aggregates do not make an explicit predicate of the filter clause which leads to optimizer not being able to push down predicate to source whenever possible. Note that the pushdown is possible only for cases when the aggregation is default and no grouping sets are present

sopel39

Could you provide query example that gets speedup?

facebook-github-bot added the CLA Signed label Sep 5, 2018

atris requested a review from kokosing September 5, 2018 18:08

atris force-pushed the filter_push_expressions branch from f01e5ee to 7765965 Compare September 5, 2018 18:11

kokosing reviewed Sep 5, 2018

View reviewed changes

atris force-pushed the filter_push_expressions branch from 7765965 to 9275d97 Compare September 7, 2018 10:50

kokosing reviewed Sep 10, 2018

View reviewed changes

atris force-pushed the filter_push_expressions branch from 9275d97 to 3ef0ddd Compare September 10, 2018 11:35

kokosing reviewed Sep 11, 2018

View reviewed changes

atris force-pushed the filter_push_expressions branch 2 times, most recently from 358fec6 to ea6c0e5 Compare September 11, 2018 17:45

kokosing reviewed Sep 12, 2018

View reviewed changes

atris force-pushed the filter_push_expressions branch from ea6c0e5 to ac0ce9b Compare September 12, 2018 10:18

kokosing reviewed Sep 13, 2018

View reviewed changes

atris force-pushed the filter_push_expressions branch from ac0ce9b to 8595d00 Compare September 13, 2018 08:30

kokosing approved these changes Sep 14, 2018

View reviewed changes

atris force-pushed the filter_push_expressions branch 2 times, most recently from 53a6503 to c7837c2 Compare September 14, 2018 07:48

kokosing previously approved these changes Sep 14, 2018

View reviewed changes

sopel39 suggested changes Sep 14, 2018

View reviewed changes

atris force-pushed the filter_push_expressions branch from c7837c2 to 06c38a1 Compare September 18, 2018 12:19

atris force-pushed the filter_push_expressions branch from 06c38a1 to bf5457d Compare September 18, 2018 15:11

sopel39 approved these changes Sep 19, 2018

View reviewed changes

sopel39 merged commit 70426ef into prestodb:master Sep 20, 2018

sopel39 mentioned this pull request Mar 26, 2019

[WIP] Fix incorrect FILTER clause evaluation order trinodb/trino#533

Closed

		@@ -20,6 +20,8 @@
		import com.facebook.presto.sql.planner.plan.AggregationNode;


		filterExpressions.add(filter);

		if (aggregation.hasNonEmptyGroupingSet()) {


		assertPlanContainsNoFilter("SELECT sum(totalprice) FILTER(WHERE TRUE) FROM orders");

		assertPlanContainsNoFilter("SELECT sum(x) FILTER(WHERE x > 0) FROM (VALUES 1, 1, 0, 2, 3, 3) t(x) GROUP BY x");

		@@ -93,6 +97,9 @@ public Result apply(AggregationNode aggregation, Captures captures, Context cont
		newAssignments.put(symbol, filter);
		mask = Optional.of(symbol);

Add disjunctive predicate for filtered aggregates #11425

Add disjunctive predicate for filtered aggregates #11425

Conversation

atris commented Sep 5, 2018

kokosing 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

kokosing commented Sep 6, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kokosing 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

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

kokosing 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

atris commented Sep 14, 2018

kokosing commented Sep 14, 2018

kokosing 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

sopel39 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

sopel39 commented Sep 17, 2018 • edited Loading

kokosing commented Sep 18, 2018

sopel39 commented Sep 18, 2018 • edited Loading

sopel39 left a comment

Choose a reason for hiding this comment

sopel39 commented Sep 17, 2018 •

edited

Loading

sopel39 commented Sep 18, 2018 •

edited

Loading