Ali Alsuliman has posted comments on this change. Change subject: [ASTERIXDB-2015][IDX] Introduce Secondary Primary Index ......................................................................
Patch Set 8: (8 comments) https://asterix-gerrit.ics.uci.edu/#/c/1916/12//COMMIT_MSG Commit Message: PS12, Line 20: enforeced > enforced Done https://asterix-gerrit.ics.uci.edu/#/c/1916/12/asterixdb/asterix-lang-aql/src/main/javacc/AQL.jj File asterixdb/asterix-lang-aql/src/main/javacc/AQL.jj: PS12, Line 604: indexName = Identifier() ifNotExists = IfNotExists() : | : ifNotExists = IfNotExists() (indexName = Identifier())? > Let's only have the index creation option that's consistent with the one ab Done PS12, Line 613: + > WS around the "+"? Done https://asterix-gerrit.ics.uci.edu/#/c/1916/12/asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj File asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj: PS12, Line 650: indexName = Identifier() ifNotExists = IfNotExists() : | : ifNotExists = IfNotExists() (indexName = Identifier())? > Let's only have the index creation option that's consistent with the one ab Done https://asterix-gerrit.ics.uci.edu/#/c/1916/14/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Index.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/entities/Index.java: Line 146: > Why have these checks here? Can we instead disallow secondary primary index Yes, we could put checks in "QueryTranslator.handleCreateIndexStatement" to only allow primary index on internal datasets and remove this method altogether. In this case, keyFieldNames.isEmpty() would always imply that the index is primary index. https://asterix-gerrit.ics.uci.edu/#/c/1916/14/hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/FixReplicateOperatorOutputsRule.java File hyracks-fullstack/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/FixReplicateOperatorOutputsRule.java: Line 123: > There's no guarantee that the "old parent" is still in the query plan. (i.e Yes, when replicate/split operators hold references to their outputs (i.e. parents in the logical plan), it makes them prone to errors and a source of problems. Besides, using them, as they are now, requires some work and caution. The "outputs" variable is an ArrayList while "outputMaterializationFlags" is a static array. That means, if the operators are not used/maintained correctly, the "outputs" could outgrow its counterpart "outputMaterializationFlags". So, what if, instead, we delete all outputs of replicate/split operators at first. Then, as we traverse the plan and encounter the "real" parents, we add them to the list "outputs"? There is one hidden assumption here, the "real" parents and "outputMaterializationFlags" are in sync in terms of position in the plan and how many parents there are. https://asterix-gerrit.ics.uci.edu/#/c/1916/12/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreePointSearchCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreePointSearchCursor.java: PS12, Line 66: Override > line break Done PS12, Line 190: BloomFilter > Please file an issue and submit a separate change for this. The rest of the Done -- To view, visit https://asterix-gerrit.ics.uci.edu/1916 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59725425ba7c5fe438507dc900f83eaab239d296 Gerrit-PatchSet: 8 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Carey <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
