abdullah alamoudi has posted comments on this change.

Change subject: Add Asterix Extension Manager
......................................................................


Patch Set 14:

(16 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1017/14/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/extension/ExtensionFunctionIdentifier.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/algebra/extension/ExtensionFunctionIdentifier.java:

Line 60:     }
> Override hashCode()?
Done


Line 60:     }
> Good point!
Done


https://asterix-gerrit.ics.uci.edu/#/c/1017/14/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/OptimizableOperatorSubTree.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/am/OptimizableOperatorSubTree.java:

Line 112:             } ;
> remove space and semicolon
I swear I removed it last time but not sure how it came back.


Line 233:                     }
> Would be nice to pull out
Right now, all DataSources are AQLDataSource but I will add an additional 
instanceof check.


https://asterix-gerrit.ics.uci.edu/#/c/1017/14/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 545:             // add the meta function
> I still couldn't understand the following assignment before feed modificati
Mmmm, I think there is a relation that is:
the source meta is going to be the destination meta.

This is how it works since meta was introduced. What could go wrong here?


https://asterix-gerrit.ics.uci.edu/#/c/1017/14/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/util/ConstantExpressionUtils.java
File 
asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/util/ConstantExpressionUtils.java:

Line 33:  */
> Did you merge the latest master?  Till's change for that is already in the 
Sure will do that.


https://asterix-gerrit.ics.uci.edu/#/c/1017/14/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/servlet/RESTAPIServlet.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/servlet/RESTAPIServlet.java:

Line 66:         this.compilationProvider = compilationProvider;
> WS
Done


https://asterix-gerrit.ics.uci.edu/#/c/1017/14/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/ExtensionId.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/api/ExtensionId.java:

Line 46:             return false;
> remove the "else if(o==null) block" because the predicate "o instanceof Ext
Done


https://asterix-gerrit.ics.uci.edu/#/c/1017/14/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/AsterixProperties.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/AsterixProperties.java:

Line 23: public class AsterixProperties {
> What's the motivation of this class?  The name sounds bigger than its scope
Yes. renaming this to AsterixPropertiesUtls


Line 34:     public static final String PROPERTY_CLUSTER_ADDRESS = 
"cluster.address";
> It seems many of the following properties duplicate things in:
I just want a single place where these strings are sorted. I moved them here in 
order to avoid spelling mistakes, inconsistency, etc and have things organized 
for anyone who wants to know about the different configurations stuff.

Will get rid of as much duplication as possible.


https://asterix-gerrit.ics.uci.edu/#/c/1017/14/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/AsxExtension.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/AsxExtension.java:

Line 25: public class AsxExtension {
> What does AsxExtension stand for?
Changing it to AsterixExtension


https://asterix-gerrit.ics.uci.edu/#/c/1017/14/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/utils/ConfigUtils.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/utils/ConfigUtils.java:

Line 31: public class ConfigUtils {
Done.


https://asterix-gerrit.ics.uci.edu/#/c/1017/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/api/ICachableMetadataEntity.java
File 
asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/api/ICachableMetadataEntity.java:

Line 25: public interface ICachableMetadataEntity<T> extends Serializable {
> Why this renaming is necessary?
Forgot to undo this. Done.


https://asterix-gerrit.ics.uci.edu/#/c/1017/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/bootstrap/MetadataIndex.java
File 
asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/bootstrap/MetadataIndex.java:

Line 89:         assert typesNamesMatch;
> Don't "assert", throw an AssertionError. "asserts" can be switched off by t
Done


https://asterix-gerrit.ics.uci.edu/#/c/1017/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlDataSource.java
File 
asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlDataSource.java:

Line 232:             throws AlgebricksException;
> Could you file an issue that we need to revisit this API?
I think that I did already but will double check.

Nope. didn't do it but now I did :)


https://asterix-gerrit.ics.uci.edu/#/c/1017/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/utils/SplitsAndConstraintsUtil.java
File 
asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/utils/SplitsAndConstraintsUtil.java:

Line 47:             int nodeParitions =
> s/nodeParitions/nodePartitions/
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I280268495cc3aad00f898cba21f7299f7120ce5c
Gerrit-PatchSet: 14
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-Reviewer: Steven Jacobs <sjaco...@ucr.edu>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to