asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Till Westmann (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Clean up GROUP BY and WITH clause.
Date Sun, 14 Aug 2016 08:24:15 GMT
Till Westmann has posted comments on this change.

Change subject: Clean up GROUP BY and WITH clause.
......................................................................


Patch Set 18:

(16 comments)

Only a few formalisms - otherwise things look good (as far as I can tell :) ).

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/


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/


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/


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 and also put it
into release notes (if we ever start doing those ...)


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


Line 105:         if(langExpr == null){
missing space

Also, when do we ever call this on a null expression ...


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.


Line 51:     // get substituted first if some of their child expressions present as keys in
the exprMap.
child expressions *are* present ?


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" ?


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.


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.


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.


Line 62:                 first  = true;
Only 1 space?


Line 67:                 if(!first){
space between "if" and "("


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.


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.


-- 
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 <buyingyi@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-HasComments: Yes

Mime
View raw message