Ian Maxon has posted comments on this change. Change subject: Hyracks Unit tests for LSM Filters ......................................................................
Patch Set 9: (7 comments) > (7 comments) > > One Q: is there a test for "filtered = true" case? All I saw was > "filtered = false". Yes, it is in LSMBTreeFilterMergeTestDriver. That is the impetus behind adding the parameter :) https://asterix-gerrit.ics.uci.edu/#/c/1704/9/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/StubIOOperationCallback.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/StubIOOperationCallback.java: PS9, Line 35: > Can you move this to the line 28 and make it as a java doc style? Done https://asterix-gerrit.ics.uci.edu/#/c/1704/9/hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/btree/OrderedIndexTestDriver.java File hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/btree/OrderedIndexTestDriver.java: Line 43: BTreeLeafFrameType leafType, boolean filtered) throws Exception; > Can we handle this? Or, it doesn't have to be handled? That would require changing quite a lot.The original signature had this. Sonarqube is just complaining because I changed that line for other reasons. https://asterix-gerrit.ics.uci.edu/#/c/1704/9/hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/btree/OrderedIndexTestUtils.java File hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/btree/OrderedIndexTestUtils.java: Line 302: if (filtered) { > Please put a simple comment about this? What does this do? Done https://asterix-gerrit.ics.uci.edu/#/c/1704/9/hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/common/TreeIndexTestUtils.java File hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/storage/am/common/TreeIndexTestUtils.java: Line 187: boolean filtered, Random rnd) throws Exception { > Here, too: Exception See other comment. Line 210: if (filtered) { > I have a feeling that the same code is repeated. Can we integrate them and Good point, refactored. https://asterix-gerrit.ics.uci.edu/#/c/1704/9/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/LSMBTreeFilterMergeTest.java File hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/LSMBTreeFilterMergeTest.java: Line 32: > Can we put java-doc style comments here? What does this test do? This is the purpose of the change. It does what LSMBTreeMergeTest does but, it does it with a filter and validates the filter state on each step of the test.The comments are in the Driver class which has the actual test. https://asterix-gerrit.ics.uci.edu/#/c/1704/9/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/LSMBTreeFilterMergeTestDriver.java File hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/LSMBTreeFilterMergeTestDriver.java: Line 46: > Also, can we put java-doc style comments here? Done -- To view, visit https://asterix-gerrit.ics.uci.edu/1704 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie0e79c0a25d5d6be727b9ebad841801024c00e94 Gerrit-PatchSet: 9 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Ian Maxon <ima...@apache.org> Gerrit-Reviewer: Ian Maxon <ima...@apache.org> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Jianfeng Jia <jianfeng....@gmail.com> Gerrit-Reviewer: Taewoo Kim <wangs...@gmail.com> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com> Gerrit-HasComments: Yes
