Yingyi Bu has posted comments on this change.

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


Patch Set 13:

(11 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?
Done


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


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


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 
FunctionSignature is only used by the parser, and FunctionIdentifier is used in 
the compiler.  

The translator translates a function signature to function identifier.  During 
the translation, it modifies/changes name spaces if necessary. Because in the 
parsing time, the function is not aware whether a function is a user defined 
function or a builtin function.

Fundamentally, they don't have much difference and should be unified.


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 
Right now, they all throw AlgebricksException -- AsterixException is a subclass 
of AlgebricksException.


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 ='?
Done


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


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?
Done. Removed, no longer needed.


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


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 typ
Done


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 
Done


-- 
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 <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-HasComments: Yes

Reply via email to