Michael Blow has posted comments on this change. Change subject: [ASTERIXDB-2204][STO] Fix implementations and usages of IIndexCursor ......................................................................
Patch Set 20: (4 comments) https://asterix-gerrit.ics.uci.edu/#/c/2324/20/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/DestroyUtils.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/DestroyUtils.java: PS20, Line 42: public static Throwable destroy(IDestroyable destroyable, Throwable root) can we refactor callers of this to pass the throwable first, then just inline this method at line 36 above? it seems strange to have both destroy(IDestroyable destroyable, Throwable root) and destroy(Throwable root, IDestroyable destroyable) be valid signatures. https://asterix-gerrit.ics.uci.edu/#/c/2324/20/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeOpContext.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeOpContext.java: PS20, Line 189: null, can remove this now, right? PS20, Line 190: failure = DestroyUtils.destroy(failure, mutableBTreeOpCtxs); : failure = DestroyUtils.destroy(insertSearchCursor, failure); see comment in DestroyUtils, very confusing to support both parameter orders for DestroyUtils.destroy() PS20, Line 189: Throwable failure = DestroyUtils.destroy(null, mutableBTreeAccessors); : failure = DestroyUtils.destroy(failure, mutableBTreeOpCtxs); : failure = DestroyUtils.destroy(insertSearchCursor, failure); : failure = DestroyUtils.destroy(memCursor, failure); can't we just have one statement here: Throwable failure = DestroyUtils.destroy(mutableBTreeAccessors, mutableBTreeOpCtxs, insertSearchCursor, memCursor)? -- To view, visit https://asterix-gerrit.ics.uci.edu/2324 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I98a7a8b931eb24dbe11bf2bdc61b754ca28ebdf9 Gerrit-PatchSet: 20 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Luo Chen <cl...@uci.edu> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Murtadha Hubail <mhub...@apache.org> Gerrit-Reviewer: Taewoo Kim <taew...@uci.edu> Gerrit-Reviewer: Taewoo Kim <wangs...@gmail.com> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com> Gerrit-HasComments: Yes
