Yingyi Bu has posted comments on this change. Change subject: Clean up GROUP BY and WITH clause. ......................................................................
Patch Set 18: (16 comments) https://asterix-gerrit.ics.uci.edu/#/c/1050/18/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/base/RuleCollections.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/base/RuleCollections.java: Line 135: List<IAlgebraicRewriteRule> translationRule = new LinkedList<>(); > s/translationRule/translationRules/ Done https://asterix-gerrit.ics.uci.edu/#/c/1050/18/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ResolveVariableRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ResolveVariableRule.java: Line 57: * based on the available type and metatadata information. > s/metatadata/metadata/ Done https://asterix-gerrit.ics.uci.edu/#/c/1050/18/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/LangExpressionToPlanTranslator.java: Line 792: // A gloabal aggregation can still have a decoration variable list which are used for propagate > s/gloabal/global/ Done https://asterix-gerrit.ics.uci.edu/#/c/1050/18/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/VariableCheckAndRewriteVisitor.java File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/rewrites/visitor/VariableCheckAndRewriteVisitor.java: Line 109: // asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/subquery/exists > Should we capture this somewhere (e.g. in an issue)? Then we could track it Done https://asterix-gerrit.ics.uci.edu/#/c/1050/18/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppVariableUtil.java File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/util/SqlppVariableUtil.java: Line 89: if(includeWithVariables || !liveVar.getVar().namedValueAccess()) { > missing space Done Line 105: if(langExpr == null){ > missing space We should never do that. I just removed this line. https://asterix-gerrit.ics.uci.edu/#/c/1050/18/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteExpressionVisitor.java File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/SqlppSubstituteExpressionVisitor.java: Line 20: */ > Too much nesting in the license. Done Line 51: // get substituted first if some of their child expressions present as keys in the exprMap. > child expressions *are* present ? Done https://asterix-gerrit.ics.uci.edu/#/c/1050/18/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/FieldAccessByNameResultType.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/FieldAccessByNameResultType.java: Line 49: throw new AlgebricksException("The second argument of a field access should be an STRING, but it is found " > remove "found" ? Done https://asterix-gerrit.ics.uci.edu/#/c/1050/18/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/PropagateTypeComputer.java File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/PropagateTypeComputer.java: Line 20: */ > Too much nesting in the license. Done https://asterix-gerrit.ics.uci.edu/#/c/1050/18/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/collections/FirstElementAggregateDescriptor.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/collections/FirstElementAggregateDescriptor.java: Line 20: */ > Too much nesting in the license. Done https://asterix-gerrit.ics.uci.edu/#/c/1050/18/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/collections/FirstElementEvalFactory.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/collections/FirstElementEvalFactory.java: Line 20: */ > Too much nesting in the license. Done Line 62: first = true; > Only 1 space? Done Line 67: if(!first){ > space between "if" and "(" Done https://asterix-gerrit.ics.uci.edu/#/c/1050/18/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/collections/LocalFirstElementAggregateDescriptor.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/collections/LocalFirstElementAggregateDescriptor.java: Line 20: */ > Too much nesting in the license. Done https://asterix-gerrit.ics.uci.edu/#/c/1050/18/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractGroupByDecorVariablesRule.java File hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/ExtractGroupByDecorVariablesRule.java: Line 21: > Too much nesting in the license. Done -- To view, visit https://asterix-gerrit.ics.uci.edu/1050 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62fca7f937aa007d97ed87c75cef19f6aa3e5ade Gerrit-PatchSet: 18 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Yingyi Bu <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-HasComments: Yes
