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