Till Westmann has posted comments on this change.

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


Patch Set 13:

(2 comments)

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 102:         FunctionSignature signature = new 
FunctionSignature(MetadataConstants.METADATA_DATAVERSE_NAME, "dataset", 1);
> FunctionSignature is only used by the parser, and FunctionIdentifier is use
Could you file an issue for the unification?


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;
> Right now, they all throw AlgebricksException -- AsterixException is a subc
Yes, but I've seen exception wrapping code in the visitors. It seems that we 
could get rid of some of that, if we just threw AlgebricksException here. Do 
you see a problem with changing the interface this way? If you think that it 
would be a good idea to change the interface, could you file an issue to do 
that? Otherwise, just tell me why it's not a good idea :)


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