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
