Fix to #6647 - Left Join (GroupJoin) always materializes elements resulting in unnecessary data pulling #7772

maumar · 2017-03-03T00:29:33Z

Problem was that for GroupJoin we would always force materialization on participating query sources.
We were doing this because we are not always able to correctly divide outer elements of the GroupJoin into correct groups.
However, if the GroupJoin clause is wrapped around SelectMany clause then groups don't matter because they are getting flattened by SelectMany anyway.

Fix is to recognize those scenarios and only force materialization when the correct grouping actually matters.
We can avoid materialization if the GroupJoin is followed by SelectMany clause (that references the grouping) and that the grouping itself is not present anywhere else in the query.
This addresses optional navigations, which is the 80% case. Manually created GroupJoins that are not modeling LeftOuterJoins still require additional materialization, but this can be addressed later as the priority is not nearly as high.

This change also addresses #7722 - Query : error during compilation for queries with navigation properties and First/Single/client method operators inside a subquery.

Problem here was that for some queries we don't know how to properly bind to a value buffer (when the result of binding is subquery, and not qsre).
Fix/mitigation is to recognize those scenarios and force materialization on the subqueries. This can be properly addressed in later commit (i.e. by improving the binding logic)

Also fixed several minor issues that were uncovered since we no longer do extensive materialization.

maumar · 2017-03-03T00:29:45Z

@anpete @smitpatel

maumar · 2017-03-03T00:31:06Z

@tuespetre fyi, feel free to chime in as you have significant knowledge in this area

maumar · 2017-03-03T00:35:27Z

src/Microsoft.EntityFrameworkCore/Query/QueryCompilationContext.cs

+                }
+            }
+
+            private Expression ExtractSourceExpression(Expression expression)


This code has been copied and modified from the EntityQueryModelVisitor.IterateCompositePropertyExpression Might be worthwhile to add a "debug"/"mock" binding to EQMV. Wasn't sure if its worth to add public surface to that class - this approach is sort of a hack, ideally we would fix the binding itself to work for those scenarios. That's why I opted for copying the code into a private method that can be removed without issue once a proper fix is implemented. Thoughts?

Not much need for this, once inside the BindMemberExpression/BindMethodCallExpression lambda, there can only have been a single property, so RemoveConvert() on node.Expression or node.Arguments[0] suffices

smitpatel · 2017-03-03T00:46:32Z

src/Microsoft.EntityFrameworkCore/Query/EntityQueryModelVisitor.cs

@@ -318,6 +318,8 @@ var entityEqualityRewritingExpressionVisitor
            _navigationRewritingExpressionVisitorFactory
                .Create(this).Rewrite(queryModel, parentQueryModel: null);

+            entityEqualityRewritingExpressionVisitor.Rewrite(queryModel);


Can we avoid rewrite twice?

I can try, but IIRC we were missing some cases if the equality rewrite happens only after nav rewrite

why does nav rewrite depends on equality rewrite?

Currently some scenarios involving navigation entity comparison is broken if entity equality doesn't run before nav rewrite. We could probably refactor that code by adding some intermediate step between nav rewrite and entity comparison, filed #7773 to track this

maumar · 2017-03-03T00:51:45Z

src/Microsoft.EntityFrameworkCore/Query/QueryCompilationContext.cs

+            }
+        }
+
+        private class RequiresMaterializationCompensatingVisitorState


I can split this into individual properties and pass them directly. In earlier iterations there was a boolean state that was shared between query model visitor and expression visitor so passing a reference was needed. Now it can be rewritten, but not sure if it's worth - this "nicely" encapsulates all the data/helpers that those visitors need. Thoughts?

No need to pass the state in -- the QueryModelVisitor and the ExpressionVisitor each can have a public HashSet<IQuerySource> QuerySources { get; } that they add to. In FindQuerySourcesRequiringMaterialization, the resulting sets from all three visitors (the original + two compensating) can be combined

Makes sense, simplifies this code a lot, now that both visitors don't share state

tuespetre · 2017-03-03T01:24:43Z

src/Microsoft.EntityFrameworkCore.Relational/Query/Expressions/TableExpressionBase.cs

+
+        protected virtual IQuerySource PreProcessQuerySource(IQuerySource querySource)
+        {
+            var newQuerySource = ExtractGroupJoinFromSelectManySubquery(querySource);


Could the same result be achieved by assigning the appropriate QuerySource to joinExpression in OptimizeJoinClause (assign it the joinClause, ~~and then if flattening a DefaultIfEmpty, assign it the additionalFromClause instead~~)?

@maumar these changes work but keep the 'hairiness' out of TableExpressionBase:

6a20802

It should be noted that the extra-hair-tastic bit (the else if block) is only required because we don't lift all the subqueries we could yet (i.e. lifting _Select using a ProjectionShaper) and thus the group joins aren't flattened as they could be.

@smitpatel @anpete any thoughts on this? - you guys know this area better than me ;) One thing worth pointing out is that we can't rely on ProjectionShaper optimization to simplify the code, since there can always be client method involved

Noting before I forget: The block itself was only needed to make two tests pass: GroupJoin_on_left_side_being_a_subquery and its twin.

tuespetre · 2017-03-03T04:33:00Z

src/Microsoft.EntityFrameworkCore/Query/QueryCompilationContext.cs

+                if (sourceExpression is SubQueryExpression subQueryExpression)
+                {
+                    var queryModel = subQueryExpression.QueryModel;
+                    if (queryModel.ResultOperators.ToList().OfType<ChoiceResultOperatorBase>().Any())


Shouldn't need to check the result operators -- if it's a subquery bound to a property, you can just promote the selector as a QSRE, using HandleUnderlyingQuerySources pointing at a MarkForMaterialization method that adds them to the collection

is selector guaranteed to be QSRE? Might be safer to just delegate to the requiresmaterialization visitor, but I agree that there is no need to check for result operators - (I think) there should always be one if we end up in this situation

If it's an entity property access, I would think so (or it's another subquery that returns an entity, which HandleUnderlyingQuerySources should handle) BUT now that you bring it up, what about a subquery that selects an entity via a constructor, like (from c in customers select new Customer() { SingleProp = c.SingleProp }).FirstOrDefault(),SingleProp? I'm not sure I've seen a test that covers that already.

EDIT: Thought occurred, and that case wouldn't matter -- because it would return a Customer object, not a value buffer.

tuespetre · 2017-03-03T04:38:52Z

@maumar I had to confirm all of my suspicions before making comments, here are the results in case it's of any help to you: fix7290...tuespetre:fix7290

maumar · 2017-03-03T08:03:55Z

new version up

tuespetre · 2017-03-03T14:05:56Z

src/Microsoft.EntityFrameworkCore/Query/QueryCompilationContext.cs

+            private void CompensateForUnboundPropertyAccess(Expression expression)
+            {
+                var sourceExpression = expression.RemoveConvert();
+                if (sourceExpression is SubQueryExpression subQueryExpression)


⛏ : if (expression.RemoveConvert() is SubQueryExpression subQueryExpression)

Hold up, new developments coming in

tuespetre · 2017-03-03T21:03:40Z

@maumar, with a couple small changes, RequiresClientSingleColumnResultOperator can be removed. I also tried to see if the UnboundPropertyAccessCompensatingVisitor could be removed entirely. Here is the result of both: 8416464

maumar · 2017-03-03T21:52:56Z

@tuespetre the change is dangerously growing in complexity ;) I thinking to keep the hack for now as its very simple and localized and will leave #7722 open. Once this change is in, you could submit a new PR with the proper fix and remove the hack.

tuespetre · 2017-03-03T21:58:18Z

I thought so too, especially the UnboundPropertyAccessCompensatingVisitor. Good call to leave that for afterwards tied to #7722.

The RequiresClientSingleColumnResultOperator change is not complex in itself, though. (2 changed files, other than test SQL output)

maumar · 2017-03-03T22:01:51Z

@tuespetre agreed, especially that it's related to this change - we introduced this flag because it was messing with out groupjoins. Will incorporate this into the PR

maumar · 2017-03-03T23:01:13Z

@tuespetre after doing some more digging, simply removing RequiresClientSingleColumnResultOperator regresses queries like this:

                var query = context.Gears.GroupJoin(
                    context.Tags,
                    g => new { k1 = g.Nickname, k2 = (int?)g.SquadId },
                    t => new { k1 = t.GearNickName, k2 = t.GearSquadId },
                    (g, t) => g.HasSoulPatch).Distinct();

For those we still materialize extra stuff, so distinct needs to happen on the client still.

RequiresClientSingleColumnResultOperator can be dealt with when fixing #7497. I would imagine we will do something similar to the groupjoin compensating visitor - if the groupjoin is followed by appropriate SelectMany and is only present in the query model once, we allow result operators to be translated, otherwise force client eval.

tuespetre · 2017-03-03T23:05:30Z

Oh, ok. I suppose Distinct on a bool would mask some errors like that!

maumar · 2017-03-03T23:09:57Z

new version up (adding more tests)

maumar · 2017-03-10T00:09:59Z

new version up, rebased on the latest changes and fixed some more bugs found along the way

tuespetre · 2017-03-10T00:16:04Z

src/EFCore/Query/ExpressionVisitors/Internal/RecursiveQueryModelVisitor.cs

+    ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+    ///     directly from your code. This API may change or be removed in future releases.
+    /// </summary>
+    public class RecursiveQueryModelVisitor : ExpressionTransformingQueryModelVisitor<RecursiveQueryModelExpressionVisitor>


why hello there beautiful 💖

@maumar - Is it visitor to visit a visitor?

This reminds me, we can reuse this in NondeterministicResultCheckingVisitor

tuespetre · 2017-03-10T16:31:17Z

@maumar I have an idea that might eliminate the need for not only the PreProcessQuerySource stuff, but also the SnapshotQuerySourceMapping/PopulateQueryModelMapping etc. stuff.

tuespetre · 2017-03-10T17:44:58Z

src/EFCore/Query/ExpressionVisitors/Internal/RecursiveQueryModelVisitor.cs

+    ///     This API supports the Entity Framework Core infrastructure and is not intended to be used
+    ///     directly from your code. This API may change or be removed in future releases.
+    /// </summary>
+    public class RecursiveQueryModelVisitor : QueryModelVisitorBase


Yo @maumar would you mind splitting this into its own PR so it can be merged sooner? I've got multiple places in multiple branches where I'd like to use this.

@maumar - We talked about it yesterday only. 😄 I had implemented a visitor which would apply QueryModelVisitor recursively. Lets discuss about it.

@tuespetre - PR #7822 can utilize such recursive visitor & detect all subquery expressions but there is one test which is regressing on that due to how parsed querymodel is generated and order which optimizations are applied. If you want to tackle that issue in that way. I do not want you to block you on that so the current changes in that PR is also fine.

#7843 is merged so you can simply this. Can you also post reference where this is being used? (I suppose there is derived class for this else this 2 visitor does nothing other than calling each other. So much Love)

anpete · 2017-03-10T18:18:52Z

src/EFCore.Relational/Query/ExpressionVisitors/RelationalEntityQueryableExpressionVisitor.cs

@@ -129,6 +137,21 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
            return base.VisitMethodCall(node);
        }

+        protected override Expression VisitExtension(Expression node)


Base VisitExtension calls VisitChildren on the extension node. I know we have been inconsistent with this but I'd like to fix that 😄

we skip over NullConditionalExpression in ExpressionVisitorBase. However we do process NullConditional in DefaultQueryExpressionVisitor so it can be safely removed here

we skip over NullConditionalExpression in ExpressionVisitorBase

Yeah, this needs to change, too. We are breaking the pattern by doing that.

I will do this in a separate PR

smitpatel · 2017-03-11T00:28:20Z

src/EFCore/Query/ExpressionVisitors/Internal/NavigationRewritingExpressionVisitor.cs

-                propertyExpression,
-                constantNull);
+            return addNullCheck
+                ? new NullConditionalExpression(target, target, propertyExpression)


For this reason, I wonder what is difference between caller and nullableCaller in NullConditionalExpression.

its relevant for the nested NCEs - nullable caller can be another NCE, whereas caller and access operation will always be normal expressions (methods/member access)

smitpatel · 2017-03-11T00:37:23Z

src/EFCore/Query/QueryCompilationContext.cs

+            private void MaterializeOptionalNavigationSource(QuerySourceReferenceExpression sourceQsre)
+            {
+                if (sourceQsre != null
+                    && sourceQsre.ReferencedQuerySource is AdditionalFromClause additionalFromClause


DRY this conditions with the GroupJoin visitor. You can put it in QuerySourceExtensions if it is used outside of this file. Or private function in this file. This thing basically takes AdditionalFromClause and find the pattern.

PreProcessQuerySource uses this. Make extension method on AdditionalFromClause. Can also used in GroupJoin visitor if you override VisitAdditionalFromClause method.
Name this pattern something good in extension method like IsMaumerJoin

Wrt GroupJoinMaterializationCompensatingVisitor I need to process GroupJoin clauses, not AdditionalFrom clauses because there can be a groupjoin without select many, and we would not catch those cases (i.e. wouldn't mark them for materialization)

smitpatel · 2017-03-11T00:38:31Z

test/EFCore.Sqlite.FunctionalTests/ComplexNavigationsQuerySqliteTest.cs

+        public override void Optional_navigation_inside_nested_method_call_translated_to_join_keeps_original_nullability_also_for_arguments()
+        {
+            base.Optional_navigation_inside_nested_method_call_translated_to_join_keeps_original_nullability_also_for_arguments();
+        }


remove these.

smitpatel · 2017-03-11T01:34:06Z

src/EFCore.Relational/Query/ExpressionVisitors/RelationalEntityQueryableExpressionVisitor.cs

@@ -71,8 +74,13 @@ protected override Expression VisitSubQuery(SubQueryExpression expression)

            var queryModelVisitor = (RelationalQueryModelVisitor)CreateQueryModelVisitor();

+            var queryModelMapping = new Dictionary<QueryModel, QueryModel>();
+            expression.QueryModel.PopulateQueryModelMapping(queryModelMapping);


Why did we not require this before and what changes brought this requirement now?

It was an existing bug, we just didn't catch it because for the relevant tests we have, LOJ and IJ were returning the same results.

smitpatel · 2017-03-11T02:09:54Z

src/EFCore/Query/QueryCompilationContext.cs

                requiresMaterializationExpressionVisitor);

-            groupJoinMaterializationExpressionVisitor.QueryModelVisitor = groupJoinMaterializationQueryModelVistor;
-            groupJoinMaterializationQueryModelVistor.VisitQueryModel(queryModel);
+            var unboundPropertyAccessCompensatingVisitor = new UnboundPropertyAccessCompensatingVisitor(


smitpatel

Looks good.

maumar · 2017-03-11T04:44:01Z

new version up

smitpatel · 2017-03-11T09:21:55Z

src/EFCore.Relational/Query/Internal/RelationalResultOperatorHandler.cs

@@ -666,7 +653,7 @@ private static Expression HandleSum(HandlerContext handlerContext)

                if (!(expression.RemoveConvert() is SelectExpression))
                {
-                    var sumExpression = new SqlFunctionExpression("SUM", expression.Type, new [] { expression });
+                    var sumExpression = new SqlFunctionExpression("SUM", handlerContext.QueryModel.SelectClause.Selector.Type, new [] { expression });


test failure?

yes, problematic scenario is entities.Select(e => (int?)e.Optional.Id)).Sum() which we now fully translate to sql. When converting single result element to a collection, the expected element TResult is int? but looking at selectExpression, we think its int (because we nuke all converts when translating). Fix is to look at query model instead which has appropriate typing.

Don't forget Min/Max! 🌵

smitpatel

LGTM.
Wait for @anpete to final sign off on this,

tuespetre · 2017-03-12T03:55:38Z

src/EFCore.Relational/Query/Expressions/SelectExpression.cs

-            return _tables.Any(te => te.QuerySource == querySource || te.HandlesQuerySource(querySource));
+            var processedQuerySource = PreProcessQuerySource(querySource);
+
+            return _tables.Any(te => te.QuerySource == processedQuerySource || te.HandlesQuerySource(processedQuerySource));


Can be simplified (only needs to check if HandlesQuerySource)

Also wouldn't hurt to fallback via OrElse to base.HandlesQuerySource

I tried that initially, but some queries deteriorate - CROSS JOIN gets converted to CROSS APPLY, where no correlation is present. Will leave as is.

tuespetre · 2017-03-12T04:00:08Z

src/EFCore/Query/Internal/QuerySourceExtensions.cs

+                && subQueryExpression.QueryModel.ResultOperators.Count == 1
+                && subQueryExpression.QueryModel.ResultOperators[0] is DefaultIfEmptyResultOperator
+                && subQueryExpression.QueryModel.MainFromClause.FromExpression is QuerySourceReferenceExpression subqueryQsre
+                && subqueryQsre.ReferencedQuerySource is GroupJoinClause groupJoinClause)


Should probably explicitly check if the GroupJoinClause is the immediately preceding body clause in the query model

this actually requires query model, which we don't have in some contexts. Currently we are doing this check in the caller code, will keep as is.

maumar · 2017-03-14T00:43:50Z

@anpete new version up

anpete · 2017-03-14T22:57:49Z

src/EFCore.Relational/Query/Internal/RelationalResultOperatorHandler.cs

-                    var inputType = expression.Type;
-                    var outputType = expression.Type;
+                    var inputType = handlerContext.QueryModel.SelectClause.Selector.Type;
+                    var outputType = handlerContext.QueryModel.SelectClause.Selector.Type;


outputType = inputType?

anpete · 2017-03-14T23:02:33Z

src/EFCore/Query/QueryCompilationContext.cs

-            var groupJoinMaterializationQueryModelVistor = new RequiresMaterializationForGroupJoinQueryModelVisitor(
-                groupJoinMaterializationExpressionVisitor,
-                _querySourcesRequiringMaterialization,
+            var groupJoinCompensatingVisitor = new GroupJoinMaterializationCompensatingVisitor(


Anyway to unify all of this inside RequiresMaterializationExpressionVisitor?

I implemented them intentionally outside - RequiresMaterializationExpressionVisitor should just find query sources that are reachable from the final projection and/or ones that need to be materialized due to client evals. Those visitors here are just hacks/workarounds for existing issues and once those issues are resolved, they can be removed - without tinkering with the RMEV, which is itself quite complicated.

Tracking issues to remove hacks?

#7787 is already tracking OptionalCollectionNavigationCompensatingVisitor, will file a new issue to track GroupJoinMaterializationCompensatingVisitor once this is checked in (and 7290 gets closed) - to track the remaining work on groupjoin materialization

…ulting in unnecessary data pulling Problem was that for GroupJoin we would always force materialization on participating query sources. We were doing this because we are not always able to correctly divide outer elements of the GroupJoin into correct groups. However, if the GroupJoin clause is wrapped around SelectMany clause the groups don't matter because they are getting flattened by SelectMany anyway. Fix is to recognize those scenarios and only force materialization when the correct grouping actually matters. We can avoid materialization if the GroupJoin is followed by SelectMany clause (that references the grouping) and that the grouping itself is not present anywhere else in the query. This addresses optional navigations, which is the 80% case. Manually created GroupJoins that are not modeling LeftOuterJoins still require additional materialization, but this can be addressed later as the priority is not nearly as high. Other bugs fixed alongside the main change: #7722 - Query : error during compilation for queries with navigation properties and First/Single/client method operators inside a subquery. Problem here was that for some queries we don't know how to properly bind to a value buffer (when the result of binding is subquery, and not qsre). Fix/mitigation is to recognize those scenarios and force materialization on the subqueries. This can be properly addressed in later commit (i.e. by improving the binding logic) #7497 - Error calling Count() after GroupJoin() (and removed the temporary fix to #7348 and implemented a proper one) This turned out to be a chicken-and-egg problem - we had discrepancy between query sources that we marked for materialization in case of GroupJoin + result operator, and the actual client side operation that had to be performed. We had to fix the group join materialization to allow for translation of those result operators, but without fixing this issue also we would get invalid queries in some cases. Proper fix is to force client result operator only if the GroupJoin cannot be flattened. We also now do this for all result operators, not just a subset because it was possible to generate invalid queries with operaors like Count(). This is now consistent with the behavior of RequiresMaterializationExpressionVisitor. Also fixed several minor issues that were encountered once no longer do extensive materialization.

dnfclas added the cla-already-signed label Mar 3, 2017

maumar added the needs-review label Mar 3, 2017

maumar commented Mar 3, 2017

View reviewed changes

maumar force-pushed the fix7290 branch from 021ce05 to 8222f73 Compare March 3, 2017 00:44

smitpatel reviewed Mar 3, 2017

View reviewed changes

maumar commented Mar 3, 2017

View reviewed changes

maumar force-pushed the fix7290 branch from 8222f73 to 4412468 Compare March 3, 2017 00:54

tuespetre reviewed Mar 3, 2017

View reviewed changes

maumar force-pushed the fix7290 branch from 4412468 to cfaface Compare March 3, 2017 08:03

tuespetre reviewed Mar 3, 2017

View reviewed changes

maumar force-pushed the fix7290 branch from cfaface to c458b2f Compare March 3, 2017 20:08

smitpatel requested a review from anpete March 3, 2017 20:28

maumar force-pushed the fix7290 branch from c458b2f to 390c8d3 Compare March 3, 2017 20:58

maumar force-pushed the fix7290 branch from 390c8d3 to 42d6f97 Compare March 3, 2017 23:09

maumar force-pushed the fix7290 branch from 42d6f97 to ca79aa4 Compare March 4, 2017 02:38

tuespetre mentioned this pull request Mar 6, 2017

Query: new 'VisitUnderlyingQuerySources' extension method #7794

Closed

maumar force-pushed the fix7290 branch from ca79aa4 to 6a8c9ed Compare March 10, 2017 00:08

tuespetre reviewed Mar 10, 2017

View reviewed changes

maumar force-pushed the fix7290 branch 4 times, most recently from 61f2b80 to f20423b Compare March 10, 2017 04:20

tuespetre reviewed Mar 10, 2017

View reviewed changes

anpete reviewed Mar 10, 2017

View reviewed changes

tuespetre mentioned this pull request Mar 10, 2017

Query: Relational: Introduce a new LeftOuterJoinClause class #7839

Closed

bricelam removed the needs-review label Mar 11, 2017

smitpatel reviewed Mar 11, 2017

View reviewed changes

smitpatel suggested changes Mar 11, 2017

View reviewed changes

maumar force-pushed the fix7290 branch from f20423b to 26d6fee Compare March 11, 2017 04:43

smitpatel reviewed Mar 11, 2017

View reviewed changes

smitpatel approved these changes Mar 11, 2017

View reviewed changes

tuespetre reviewed Mar 12, 2017

View reviewed changes

maumar force-pushed the fix7290 branch from 26d6fee to 64f97fb Compare March 14, 2017 00:23

anpete reviewed Mar 14, 2017

View reviewed changes

anpete approved these changes Mar 14, 2017

View reviewed changes

maumar force-pushed the fix7290 branch from 64f97fb to 03a990c Compare March 15, 2017 03:13

maumar merged commit 03a990c into dev Mar 15, 2017

smitpatel deleted the fix7290 branch March 15, 2017 03:17

Fix to #6647 - Left Join (GroupJoin) always materializes elements resulting in unnecessary data pulling #7772

Fix to #6647 - Left Join (GroupJoin) always materializes elements resulting in unnecessary data pulling #7772

Conversation

maumar commented Mar 3, 2017 • edited Loading

maumar commented Mar 3, 2017

maumar commented Mar 3, 2017

maumar Mar 3, 2017 • 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

maumar Mar 3, 2017 • 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

tuespetre Mar 3, 2017 • edited Loading

Choose a reason for hiding this comment

tuespetre Mar 3, 2017 • edited Loading

Choose a reason for hiding this comment

maumar Mar 3, 2017 • 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

tuespetre Mar 3, 2017 • edited Loading

Choose a reason for hiding this comment

tuespetre commented Mar 3, 2017

maumar commented Mar 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tuespetre commented Mar 3, 2017

maumar commented Mar 3, 2017

tuespetre commented Mar 3, 2017

maumar commented Mar 3, 2017

maumar commented Mar 3, 2017

tuespetre commented Mar 3, 2017

maumar commented Mar 3, 2017

maumar commented Mar 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tuespetre commented Mar 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

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

maumar Mar 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smitpatel left a comment

Choose a reason for hiding this comment

maumar commented Mar 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smitpatel 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

maumar commented Mar 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maumar commented Mar 3, 2017 •

edited

Loading

maumar Mar 3, 2017 •

edited

Loading

maumar Mar 3, 2017 •

edited

Loading

tuespetre Mar 3, 2017 •

edited

Loading

tuespetre Mar 3, 2017 •

edited

Loading

maumar Mar 3, 2017 •

edited

Loading

tuespetre Mar 3, 2017 •

edited

Loading

maumar Mar 11, 2017 •

edited

Loading