Dmitry Lychagin has posted comments on this change. Change subject: [ASTERIXDB-2015][IDX] Introduce Secondary Primary Index ......................................................................
Patch Set 14: (2 comments) 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: if (isPrimaryIndex || indexDataset.getDatasetType() == DatasetConfig.DatasetType.EXTERNAL || Why have these checks here? Can we instead disallow secondary primary index over external datasets when processing 'create index' statement (QueryTranslator.handleCreateIndexStatement)? If we do that this method could be simplified to "isSecondaryIndex() && keyFieldNames.isEmpty()", right? May be after this simplification this method could be removed at all. It's currently seems to be only used by IntroduceSecondaryIndexInsertDeleteRule.java to enable materialization, but that rule could just check keyFieldNames.isEmpty() because it already checks that it operates on secondary indexes, and empty keyFieldNames means that the sort won't be introduced hence we need to set the materialization flag instead. 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: if (parentsPathToReplicate.contains(replicateOperator.getOutputs().get(oldParentIndex))) { There's no guarantee that the "old parent" is still in the query plan. (i.e. what if it was already removed by some other rule?). So it seems that we cannot assume that we can always correctly fix replicate operator outputs. Long term we should explore whether we can change the replicate/split operators such that they do not hold references to their outputs at all. I'll file an issue for that. -- 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: 14 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Ali Alsuliman <ali.al.solai...@gmail.com> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Carey <dtab...@gmail.com> Gerrit-Reviewer: Taewoo Kim <wangs...@gmail.com> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com> Gerrit-HasComments: Yes
