Change in asterixdb[master]: Support implicit variable name and column name.

["Till Westmann (Code Review)" <do...@asterixdb.incubator.apache.org>]

Till Westmann has posted comments on this change.

Change subject: Support implicit variable name and column name.
......................................................................


Patch Set 13:

(11 comments)

Mostly questions and very few comments.

https://asterix-gerrit.ics.uci.edu/#/c/950/13/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 224:         if (mdp.findDataset(mdp.getDefaultDataverseName(), name) != null) {
Why do we check the default dataverse before the fully qualified name?


Line 261:         if (argFuncExpr.getExpressionTag() != LogicalExpressionTag.FUNCTION_CALL) {
This check was already done 4 lines above.


https://asterix-gerrit.ics.uci.edu/#/c/950/13/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 67:                     "Inside limit clauses, it is disallowed to reference a variable having the same name as any variable bound in the same scope as the limit clause.");
Reduce line length?


Line 102:         FunctionSignature signature = new FunctionSignature(MetadataConstants.METADATA_DATAVERSE_NAME, "dataset", 1);
Here we have a "dataset" function in the metadata dataverse and one in the  "asterix" namespace. What is the relationship between them? (Same seems to be true for "resolve" functions).

Also, should we have constants for these FunctionSignatures?

Related: What's the difference between an

    org.apache.hyracks.algebricks.core.algebra.functions.FunctionIdentifier

and an

    org.apache.asterix.common.functions.FunctionSignature

?


https://asterix-gerrit.ics.uci.edu/#/c/950/13/asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/ISqlppVisitor.java
File asterixdb/asterix-lang-sqlpp/src/main/java/org/apache/asterix/lang/sqlpp/visitor/base/ISqlppVisitor.java:

Line 66:     R visit(IndependentSubquery independentSubquery, T arg) throws AsterixException;
Generic question: Should all these visitors throw AlgebricksExceptions? If so, could we file an issue for that?


https://asterix-gerrit.ics.uci.edu/#/c/950/13/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
File asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj:

Line 2322:     expr= Expression() ((<AS>)? name = Identifier())?
'expr ='?


Line 2327:     if(name==null){
formatting?


https://asterix-gerrit.ics.uci.edu/#/c/950/13/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/AsterixBuiltinFunctions.java
File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/functions/AsterixBuiltinFunctions.java:

Line 1049:         addFunction(RESOLVE, AnyTypeComputer.INSTANCE, false);
What do these functions do?


https://asterix-gerrit.ics.uci.edu/#/c/950/13/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/OpenRecordConstructorResultType.java
File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/typecomputer/impl/OpenRecordConstructorResultType.java:

Line 64:         Set<String> allPossibleAdditionalFieldNames = new HashSet<>();
What are these field names?


https://asterix-gerrit.ics.uci.edu/#/c/950/13/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ARecordType.java
File asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/ARecordType.java:

Line 85:             Set<String> allPossibleAdditionalFieldNames) {
Why would a record type know about filed names that are not part of the type? This is confusing ...


https://asterix-gerrit.ics.uci.edu/#/c/950/13/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/StructuralPropertiesVector.java
File hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/properties/StructuralPropertiesVector.java:

Line 92:             }
How about 

    IPartitioningProperty normalizedReqPart =
        reqdPart.normalize(equivalenceClasses, 
            mayExpandProperties ? fds : null);


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/950
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I409b9ba139c9f000a6b9b84d519d172d0069e4bb
Gerrit-PatchSet: 13
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Yingyi Bu <bu...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-HasComments: Yes