phoenix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jamestay...@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 GMT
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.
      */


Mime
View raw message