Mailing-List: contact commits-help@phoenix.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@phoenix.incubator.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: jamestaylor@apache.org
To: commits@phoenix.incubator.apache.org
Message-Id: <916f04a278874bdc8545e02c17c71e24@git.apache.org>
Subject: git commit: PHOENIX-63 Can't add null value to a secondary indexed
 table (SamarthJain)
Date: Tue, 29 Apr 2014 22:52:14 +0000 (UTC)

Repository: incubator-phoenix
Updated Branches:
  refs/heads/4.0 2cd5e8bee -> 0fe01e1b0


PHOENIX-63 Can't add null value to a secondary indexed table (SamarthJain)


Project: http://git-wip-us.apache.org/repos/asf/incubator-phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-phoenix/commit/0fe01e1b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-phoenix/tree/0fe01e1b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-phoenix/diff/0fe01e1b

Branch: refs/heads/4.0
Commit: 0fe01e1b0764bd6f612a4b68101409a8add823e2
Parents: 2cd5e8b
Author: James Taylor <jamestaylor@apache.org>
Authored: Tue Apr 29 15:52:29 2014 -0700
Committer: James Taylor <jamestaylor@apache.org>
Committed: Tue Apr 29 15:52:29 2014 -0700

----------------------------------------------------------------------
 .../phoenix/end2end/index/MutableIndexIT.java   | 105 +++++++++++++------
 .../apache/phoenix/index/IndexMaintainer.java   |  33 +++---
 2 files changed, 95 insertions(+), 43 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/0fe01e1b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/MutableIndexIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/MutableIndexIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/MutableIndexIT.java
index 50310bf..64bd940 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/MutableIndexIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/MutableIndexIT.java
@@ -29,6 +29,7 @@ import java.sql.Date;
 import java.sql.DriverManager;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
+import java.sql.Statement;
 import java.util.Map;
 import java.util.Properties;
 
@@ -42,6 +43,7 @@ import org.junit.Ignore;
 import org.junit.Test;
 
 import com.google.common.collect.Maps;
+import com.google.common.primitives.Doubles;
 
 public class MutableIndexIT extends BaseMutableIndexIT {
     @BeforeClass 
@@ -821,38 +823,79 @@ public class MutableIndexIT extends BaseMutableIndexIT {
             conn.close();
         }
     }
-
-    @Ignore("PHOENIX-63")
-    @Test
-    public void testNullValueIndexKey() throws Exception {
-        Properties props = new Properties(TEST_PROPERTIES);
-        Connection conn = DriverManager.getConnection(getUrl(), props);
-        conn.setAutoCommit(false);
-        try {
-            String ddl = "CREATE TABLE DEMO(R VARCHAR PRIMARY KEY, A DOUBLE, C VARCHAR)";
-            conn.createStatement().execute(ddl);
-            ddl = "CREATE INDEX IDX_DEMO ON DEMO (A) INCLUDE (C)";
-            conn.createStatement().execute(ddl);
     
-            PreparedStatement stmt = conn.prepareStatement("upsert into DEMO values(?, ?, ?)");
-            stmt.setString(1, "r1");
-            stmt.setString(2, null);
-            stmt.setString(3, "c1");
-            stmt.executeUpdate();
-            conn.commit();
-            
-            String query = "select * from DEMO"; 
-            ResultSet rs = conn.createStatement().executeQuery("EXPLAIN " + query);
-            assertEquals("CLIENT PARALLEL 1-WAY FULL SCAN OVER IDX_DEMO", QueryUtil.getExplainPlan(rs));
+    @Test
+    public void testUpsertingNullForIndexedColumns() throws Exception {
+    	Properties props = new Properties(TEST_PROPERTIES);
+    	Connection conn = DriverManager.getConnection(getUrl(), props);
+    	conn.setAutoCommit(false);
+    	try {
+    		Statement stmt = conn.createStatement();
+    		stmt.execute("CREATE TABLE DEMO(v1 VARCHAR PRIMARY KEY, v2 DOUBLE, v3 VARCHAR)");
+    		stmt.execute("CREATE INDEX DEMO_idx ON DEMO (v2) INCLUDE(v3)");
+    		
+    		//create a row with value null for indexed column v2
+    		stmt.executeUpdate("upsert into DEMO values('cc1', null, 'abc')");
+    		conn.commit();
             
-            rs = conn.createStatement().executeQuery(query);
-            assertTrue(rs.next());
-            assertEquals("r1", rs.getString(1));
-            assertNull(rs.getString(2));
-            assertEquals("c1", rs.getString(3));
-            assertFalse(rs.next());
-        } finally {
-            conn.close();
-        }
+    		//assert values in index table 
+    		ResultSet rs = stmt.executeQuery("select * from DEMO_IDX");
+    		assertTrue(rs.next());
+    		assertEquals(0, Doubles.compare(0, rs.getDouble(1)));
+    		assertEquals("cc1", rs.getString(2));
+    		assertEquals("abc", rs.getString(3));
+    		assertFalse(rs.next());
+    		
+    		//assert values in data table
+    		rs = stmt.executeQuery("select v1, v2, v3 from DEMO");
+    		assertTrue(rs.next());
+    		assertEquals("cc1", rs.getString(1));
+    		assertEquals(0, Doubles.compare(0, rs.getDouble(2)));
+    		assertEquals("abc", rs.getString(3));
+    		assertFalse(rs.next());
+    		
+    		//update the previously null value for indexed column v2 to a non-null value 1.23
+    		stmt.executeUpdate("upsert into DEMO values('cc1', 1.23, 'abc')");
+    		conn.commit();
+    		
+    		//assert values in index table 
+    		rs = stmt.executeQuery("select * from DEMO_IDX");
+    		assertTrue(rs.next());
+    		assertEquals(0, Doubles.compare(1.23, rs.getDouble(1)));
+    		assertEquals("cc1", rs.getString(2));
+    		assertEquals("abc", rs.getString(3));
+    		assertFalse(rs.next());
+    		
+    		//assert values in data table
+    		rs = stmt.executeQuery("select v1, v2, v3 from DEMO");
+    		assertTrue(rs.next());
+    		assertEquals("cc1", rs.getString(1));
+    		assertEquals(0, Doubles.compare(1.23, rs.getDouble(2)));
+    		assertEquals("abc", rs.getString(3));
+    		assertFalse(rs.next());
+    		
+    		//update the value for indexed column v2 back to null
+    		stmt.executeUpdate("upsert into DEMO values('cc1', null, 'abc')");
+    		conn.commit();
+    		
+    		//assert values in index table 
+    		rs = stmt.executeQuery("select * from DEMO_IDX");
+    		assertTrue(rs.next());
+    		assertEquals(0, Doubles.compare(0, rs.getDouble(1)));
+    		assertEquals("cc1", rs.getString(2));
+    		assertEquals("abc", rs.getString(3));
+    		assertFalse(rs.next());
+    		
+    		//assert values in data table
+    		rs = stmt.executeQuery("select v1, v2, v3 from DEMO");
+    		assertTrue(rs.next());
+    		assertEquals("cc1", rs.getString(1));
+    		assertEquals(0, Doubles.compare(0, rs.getDouble(2)));
+    		assertEquals("abc", rs.getString(3));
+    		assertFalse(rs.next());
+    	} finally {
+    		conn.close();
+    	}
     }
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/0fe01e1b/phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java b/phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
index c1d6c25..0c69df3 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java
@@ -34,6 +34,7 @@ import java.util.Set;
 import org.apache.hadoop.hbase.CellUtil;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.KeyValue.Type;
 import org.apache.hadoop.hbase.client.Delete;
 import org.apache.hadoop.hbase.client.Durability;
 import org.apache.hadoop.hbase.client.Put;
@@ -448,21 +449,29 @@ public class IndexMaintainer implements Writable, Iterable<ColumnReference> {
             newState.put(new ColumnReference(CellUtil.cloneFamily(kv), CellUtil.cloneQualifier(kv)), kv);
         }
         for (ColumnReference ref : indexedColumns) {
-            KeyValue newValue = newState.get(ref);
-            if (newValue != null) { // Indexed column was potentially changed
-                ImmutableBytesPtr oldValue = oldState.getLatestValue(ref);
-                // If there was no old value or the old value is different than the new value, the index row needs to be deleted
-                if (oldValue == null || 
-                        Bytes.compareTo(oldValue.get(), oldValue.getOffset(), oldValue.getLength(), 
-                          newValue.getValueArray(), newValue.getValueOffset(), newValue.getValueLength()) != 0){
-                    return true;
-                }
-            }
+        	KeyValue newValue = newState.get(ref);
+        	if (newValue != null) { // Indexed column has potentially changed
+        		ImmutableBytesPtr oldValue = oldState.getLatestValue(ref);
+        		boolean newValueSetAsNull = newValue.getTypeByte() == Type.DeleteColumn.getCode();
+        		//If the new column value has to be set as null and the older value is null too,
+        		//then just skip to the next indexed column.
+        		if (newValueSetAsNull && oldValue == null) {
+        			continue;
+        		}
+        		if ((oldValue == null && !newValueSetAsNull) || (oldValue != null && newValueSetAsNull)) {
+        			return true;
+        		}
+        		// If the old value is different than the new value, the index row needs to be deleted
+        		if (Bytes.compareTo(oldValue.get(), oldValue.getOffset(), oldValue.getLength(), 
+        				newValue.getValueArray(), newValue.getValueOffset(), newValue.getValueLength()) != 0) {
+        			return true;
+        		}
+        	}
         }
         return false;
     }
-
-    /**
+    
+   /**
      * Used for immutable indexes that only index PK column values. In that case, we can handle a data row deletion,
      * since we can build the corresponding index row key.
      */