From 52ca440ad646127b6bfce82ea24c0892b4a005cb Mon Sep 17 00:00:00 2001 From: eomiks Date: Tue, 12 Apr 2022 21:26:36 +0900 Subject: [PATCH] HBASE-26901 delete with null columnQualifier occurs NullPointerException when NewVersionBehavior is on (#4295) Signed-off-by: Duo Zhang (cherry picked from commit 7ac9e0be27afe312116943842855fdb776dce958) --- .../querymatcher/NewVersionBehaviorTracker.java | 16 +++++++++++----- .../TestNewVersionBehaviorFromClientSide.java | 11 +++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/NewVersionBehaviorTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/NewVersionBehaviorTracker.java index 3c9a541d0b38..d62e2aa1f5b6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/NewVersionBehaviorTracker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/NewVersionBehaviorTracker.java @@ -165,9 +165,7 @@ protected DeleteVersionsNode getDeepCopy() { * Else return MAX_VALUE. */ protected long prepare(Cell cell) { - boolean matchCq = - PrivateCellUtil.matchingQualifier(cell, lastCqArray, lastCqOffset, lastCqLength); - if (!matchCq) { + if (isColumnQualifierChanged(cell)) { // The last cell is family-level delete and this is not, or the cq is changed, // we should construct delColMap as a deep copy of delFamMap. delColMap.clear(); @@ -175,8 +173,7 @@ protected long prepare(Cell cell) { delColMap.put(e.getKey(), e.getValue().getDeepCopy()); } countCurrentCol = 0; - } - if (matchCq && !PrivateCellUtil.isDelete(lastCqType) && lastCqType == cell.getTypeByte() + } else if (!PrivateCellUtil.isDelete(lastCqType) && lastCqType == cell.getTypeByte() && lastCqTs == cell.getTimestamp()) { // Put with duplicate timestamp, ignore. return lastCqMvcc; @@ -190,6 +187,15 @@ protected long prepare(Cell cell) { return Long.MAX_VALUE; } + private boolean isColumnQualifierChanged(Cell cell) { + if (delColMap.isEmpty() && lastCqArray == null && cell.getQualifierLength() == 0 + && (PrivateCellUtil.isDeleteColumns(cell) || PrivateCellUtil.isDeleteColumnVersion(cell))) { + // for null columnQualifier + return true; + } + return !PrivateCellUtil.matchingQualifier(cell, lastCqArray, lastCqOffset, lastCqLength); + } + // DeleteTracker @Override public void add(Cell cell) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestNewVersionBehaviorFromClientSide.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestNewVersionBehaviorFromClientSide.java index 897fc3c00d58..7fdc10d18eed 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestNewVersionBehaviorFromClientSide.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestNewVersionBehaviorFromClientSide.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import java.io.IOException; import org.apache.hadoop.hbase.HBaseClassTestRule; @@ -358,4 +359,14 @@ public void testRawScanAndMajorCompaction() throws IOException { } } + @Test + public void testNullColumnQualifier() throws IOException { + try (Table t = createTable()) { + Delete del = new Delete(ROW); + del.addColumn(FAMILY, null); + t.delete(del); + Result r = t.get(new Get(ROW)); //NPE + assertTrue(r.isEmpty()); + } + } }