From 76a743e8f0314bdc5840419e1d59c43be84d66d3 Mon Sep 17 00:00:00 2001
From: Michael Stack <stack@apache.org>
Date: Mon, 13 Jun 2011 23:43:49 +0000
Subject: [PATCH] HBASE-3793 HBASE-3468 Broke checkAndPut with null value

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1135342 13f79535-47bb-0310-9956-ffa450edef68
---
 CHANGES.txt                                   |  1 +
 .../hadoop/hbase/regionserver/HRegion.java    |  7 ++--
 .../hbase/client/TestFromClientSide.java      | 42 +++++++++++++++++++
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 5debf525f40..e7f6dc5178a 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -124,6 +124,7 @@ Release 0.91.0 - Unreleased
                indicates to bypass default action (Ming Ma)
    HBASE-3963  Schedule all log-spliiting at startup all at once (mingjian)
    HBASE-3983  list command in shell seems broken
+   HBASE-3793  HBASE-3468 Broke checkAndPut with null value (Ming Ma)
 
   IMPROVEMENTS
    HBASE-3290  Max Compaction Size (Nicolas Spiegelberg via Stack)  
diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
index ff7f8638abf..cd65a4e8707 100644
--- a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
+++ b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
@@ -1774,11 +1774,12 @@ public class HRegion implements HeapSize { // , Writable{
       try {
         result = get(get, false);
 
+        boolean valueIsNull = comparator.getValue() == null ||
+          comparator.getValue().length == 0;
         boolean matches = false;
-        if (result.size() == 0 &&
-           (comparator.getValue() == null || comparator.getValue().length == 0)) {
+        if (result.size() == 0 && valueIsNull) {
           matches = true;
-        } else if (result.size() == 1) {
+        } else if (result.size() == 1 && !valueIsNull) {
           int compareResult = comparator.compareTo(result.get(0).getValue());
           switch (compareOp) {
           case LESS:
diff --git a/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java b/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
index c3b23fef973..a24cd312680 100644
--- a/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
+++ b/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
@@ -4151,5 +4151,47 @@ public class TestFromClientSide {
     }
     executorService.shutdownNow();
   }
+  
+  
+  @Test
+  public void testCheckAndPut() throws IOException {
+    final byte [] anotherrow = Bytes.toBytes("anotherrow");
+    final byte [] value2 = Bytes.toBytes("abcd");
+    
+    HTable table = TEST_UTIL.createTable(Bytes.toBytes("testCheckAndPut"),
+      new byte [][] {FAMILY});
+    Put put1 = new Put(ROW);
+    put1.add(FAMILY, QUALIFIER, VALUE);
+
+    // row doesn't exist, so using non-null value should be considered "not match".
+    boolean ok = table.checkAndPut(ROW, FAMILY, QUALIFIER, VALUE, put1);
+    assertEquals(ok, false);
+    
+    // row doesn't exist, so using "null" to check for existence should be considered "match".
+    ok = table.checkAndPut(ROW, FAMILY, QUALIFIER, null, put1);
+    assertEquals(ok, true);
+
+    // row now exists, so using "null" to check for existence should be considered "not match".
+    ok = table.checkAndPut(ROW, FAMILY, QUALIFIER, null, put1);
+    assertEquals(ok, false);
+    
+    Put put2 = new Put(ROW);
+    put2.add(FAMILY, QUALIFIER, value2);
+    
+    // row now exists, use the matching value to check
+    ok = table.checkAndPut(ROW, FAMILY, QUALIFIER, VALUE, put2);
+    assertEquals(ok, true);
+
+    Put put3 = new Put(anotherrow);
+    put3.add(FAMILY, QUALIFIER, VALUE);
+    
+    // try to do CheckAndPut on different rows       
+    try {
+        ok = table.checkAndPut(ROW, FAMILY, QUALIFIER, value2, put3);
+        fail("trying to check and modify different rows should have failed.");
+      } catch(Exception e) {}      
+    
+  }  
+  
 }