From 787f4c958372d137d85feadaa7a9e5f847d0ab8f Mon Sep 17 00:00:00 2001 From: haxiaolin Date: Thu, 10 Feb 2022 11:53:45 +0800 Subject: [PATCH 1/2] HBASE-26742 Comparator of NOT_EQUAL NULL is invalid for checkAndMutate --- .../hadoop/hbase/regionserver/HRegion.java | 8 ++-- .../hbase/client/TestCheckAndMutate.java | 47 ++++++++++++++++++- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 2d7f720eebc8..accb8d457a27 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -4937,11 +4937,11 @@ private CheckAndMutateResult checkAndMutateInternal(CheckAndMutate checkAndMutat boolean valueIsNull = comparator.getValue() == null || comparator.getValue().length == 0; if (result.isEmpty() && valueIsNull) { - matches = true; - } else if (result.size() > 0 && result.get(0).getValueLength() == 0 && valueIsNull) { - matches = true; + matches = op != CompareOperator.NOT_EQUAL; + } else if (result.size() > 0 && valueIsNull) { + matches = (result.get(0).getValueLength() == 0) == (op != CompareOperator.NOT_EQUAL); cellTs = result.get(0).getTimestamp(); - } else if (result.size() == 1 && !valueIsNull) { + } else if (result.size() == 1) { Cell kv = result.get(0); cellTs = kv.getTimestamp(); int compareResult = PrivateCellUtil.compareValue(kv, comparator); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCheckAndMutate.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCheckAndMutate.java index 1c58db33e540..d8680219a0e7 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCheckAndMutate.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCheckAndMutate.java @@ -24,7 +24,6 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; - import java.io.IOException; import java.util.Arrays; import java.util.Collections; @@ -1216,4 +1215,50 @@ public void testCheckAndRowMutationsBatch() throws Throwable { assertEquals("h", Bytes.toString(result.getValue(FAMILY, Bytes.toBytes("H")))); } } + + @Test + public void testCheckAndMutateForNull() throws Exception { + byte[] qualifier = Bytes.toBytes("Q"); + try (Table table = createTable()) { + byte [] row1 = Bytes.toBytes("testRow1"); + Put put = new Put(row1); + put.addColumn(FAMILY, qualifier, Bytes.toBytes("v0")); + table.put(put); + assertEquals("v0", Bytes.toString( + table.get(new Get(row1).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier))); + + CheckAndMutate checkAndMutate1 = CheckAndMutate.newBuilder(row1) + .ifMatches(FAMILY, qualifier, CompareOperator.NOT_EQUAL, new byte[] {}) + .build(new Put(row1).addColumn(FAMILY, qualifier, Bytes.toBytes("v1"))); + table.checkAndMutate(checkAndMutate1); + assertEquals("v1", Bytes.toString(table.get(new Get(row1).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier))); + + byte [] row2 = Bytes.toBytes("testRow2"); + put = new Put(row2); + put.addColumn(FAMILY, qualifier, new byte[] {}); + table.put(put); + assertEquals(0, table.get(new Get(row2).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier).length); + + CheckAndMutate checkAndMutate2 = CheckAndMutate.newBuilder(row2) + .ifMatches(FAMILY, qualifier, CompareOperator.EQUAL, new byte[] {}) + .build(new Put(row2).addColumn(FAMILY, qualifier, Bytes.toBytes("v2"))); + table.checkAndMutate(checkAndMutate2); + assertEquals("v2", Bytes.toString(table.get(new Get(row2).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier))); + + byte [] row3 = Bytes.toBytes("testRow3"); + put = new Put(row3).addColumn(FAMILY, qualifier, Bytes.toBytes("v0")); + assertNull(table.get(new Get(row3).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier)); + CheckAndMutate checkAndMutate3 = CheckAndMutate.newBuilder(row3) + .ifMatches(FAMILY, qualifier, CompareOperator.NOT_EQUAL, new byte[] {}) + .build(put); + table.checkAndMutate(checkAndMutate3); + assertNull(table.get(new Get(row3).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier)); + + CheckAndMutate checkAndMutate4 = CheckAndMutate.newBuilder(row3) + .ifMatches(FAMILY, qualifier, CompareOperator.EQUAL, new byte[] {}) + .build(put); + table.checkAndMutate(checkAndMutate4); + assertEquals("v0", Bytes.toString(table.get(new Get(row3).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier))); + } + } } From b2a0c9df7d6713e2f512f3f5ec03338263eae0af Mon Sep 17 00:00:00 2001 From: sunhelly Date: Tue, 15 Feb 2022 20:31:52 +0800 Subject: [PATCH 2/2] fix checkstyle issue --- .../hadoop/hbase/client/TestCheckAndMutate.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCheckAndMutate.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCheckAndMutate.java index d8680219a0e7..e4509affb766 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCheckAndMutate.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCheckAndMutate.java @@ -1225,25 +1225,28 @@ public void testCheckAndMutateForNull() throws Exception { put.addColumn(FAMILY, qualifier, Bytes.toBytes("v0")); table.put(put); assertEquals("v0", Bytes.toString( - table.get(new Get(row1).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier))); + table.get(new Get(row1).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier))); CheckAndMutate checkAndMutate1 = CheckAndMutate.newBuilder(row1) .ifMatches(FAMILY, qualifier, CompareOperator.NOT_EQUAL, new byte[] {}) .build(new Put(row1).addColumn(FAMILY, qualifier, Bytes.toBytes("v1"))); table.checkAndMutate(checkAndMutate1); - assertEquals("v1", Bytes.toString(table.get(new Get(row1).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier))); + assertEquals("v1", Bytes.toString( + table.get(new Get(row1).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier))); byte [] row2 = Bytes.toBytes("testRow2"); put = new Put(row2); put.addColumn(FAMILY, qualifier, new byte[] {}); table.put(put); - assertEquals(0, table.get(new Get(row2).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier).length); + assertEquals(0, + table.get(new Get(row2).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier).length); CheckAndMutate checkAndMutate2 = CheckAndMutate.newBuilder(row2) .ifMatches(FAMILY, qualifier, CompareOperator.EQUAL, new byte[] {}) .build(new Put(row2).addColumn(FAMILY, qualifier, Bytes.toBytes("v2"))); table.checkAndMutate(checkAndMutate2); - assertEquals("v2", Bytes.toString(table.get(new Get(row2).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier))); + assertEquals("v2", Bytes.toString( + table.get(new Get(row2).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier))); byte [] row3 = Bytes.toBytes("testRow3"); put = new Put(row3).addColumn(FAMILY, qualifier, Bytes.toBytes("v0")); @@ -1258,7 +1261,8 @@ public void testCheckAndMutateForNull() throws Exception { .ifMatches(FAMILY, qualifier, CompareOperator.EQUAL, new byte[] {}) .build(put); table.checkAndMutate(checkAndMutate4); - assertEquals("v0", Bytes.toString(table.get(new Get(row3).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier))); + assertEquals("v0", Bytes.toString( + table.get(new Get(row3).addColumn(FAMILY, qualifier)).getValue(FAMILY, qualifier))); } } }