charlesconnell commented on PR #6557:
URL: https://github.com/apache/hbase/pull/6557#issuecomment-4187127823
Thinking more about this, I believe there are correctness problems. I don't
think it is safe to use SKIP_NEXT_COL because you can't know what other cells
you might be skipping, and some might need to be included in the scan result.
Here are some test cases you add to `TestStoreScannerDeleteMarkerOptimization`
that pass on master, and fail on this branch, showing examples of this:
```
/**
* Type-specific Delete at T=2 kills only Put@T=2. Put@T=1 is uncovered and
* must be returned as the latest visible version.
*/
@Test
public void testDeleteWithOlderUncoveredPut() throws IOException {
kvs.add(create("r", CF, "q", 2, Type.Delete, ""));
kvs.add(create("r", CF, "q", 2, Type.Put, "v2"));
kvs.add(create("r", CF, "q", 1, Type.Put, "v1"));
Pair<List<ExtendedCell>, Long> result = scanAll();
// Put@1 is not covered by Delete@2 (type-specific) — it must be returned
assertEquals("uncovered Put@1 should be returned", 1,
result.getFirst().size());
assertEquals(kvs.get(2), result.getFirst().get(0));
}
/**
* Same as above but with multiple columns. Delete@2 on "q" should only
* affect Put@2 on "q". Put@1 on "q" should still be visible, along with
* the put on "q1".
*/
@Test
public void testDeleteWithOlderUncoveredPutMultipleColumns() throws
IOException {
kvs.add(create("r", CF, "q", 2, Type.Delete, ""));
kvs.add(create("r", CF, "q", 2, Type.Put, "v2"));
kvs.add(create("r", CF, "q", 1, Type.Put, "v1"));
kvs.add(create("r", CF, "q1", 1, Type.Put, "v1"));
Pair<List<ExtendedCell>, Long> result = scanAll();
// Both Put@1 on "q" and Put@1 on "q1" should be returned
assertEquals("should return 2 cells", 2, result.getFirst().size());
assertEquals(kvs.get(2), result.getFirst().get(0));
assertEquals(kvs.get(3), result.getFirst().get(1));
}
/**
* DeleteFamilyVersion at T=2 kills all columns at T=2. But Put@1 on
* the same column is uncovered and must be returned.
*/
@Test
public void testDeleteFamilyVersionWithOlderUncoveredPut() throws
IOException {
kvs.add(create("r", CF, "", 2, Type.DeleteFamilyVersion, ""));
kvs.add(create("r", CF, "q", 2, Type.Put, "v2"));
kvs.add(create("r", CF, "q", 1, Type.Put, "v1"));
Pair<List<ExtendedCell>, Long> result = scanAll();
// Put@1 is not covered by DeleteFamilyVersion@2 — it must be returned
assertEquals("uncovered Put@1 should be returned", 1,
result.getFirst().size());
assertEquals(kvs.get(2), result.getFirst().get(0));
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
