Repository: phoenix
Updated Branches:
  refs/heads/3.0 4c8798d57 -> ed3e3f55f


PHOENIX-1337 Unpadded fixed length tenant ID causes erroneous results
(James Taylor via Ram)


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

Branch: refs/heads/3.0
Commit: ed3e3f55f1e9d91cec8a02882ae4c61ced9e49f0
Parents: 4c8798d
Author: Ramkrishna <ramkrishna.s.vasude...@intel.com>
Authored: Mon Oct 13 12:53:26 2014 +0530
Committer: Ramkrishna <ramkrishna.s.vasude...@intel.com>
Committed: Mon Oct 13 12:53:26 2014 +0530

----------------------------------------------------------------------
 .../end2end/TenantSpecificViewIndexIT.java      | 44 ++++++++++++++++++++
 .../apache/phoenix/compile/DeleteCompiler.java  | 15 +++++--
 .../phoenix/compile/ProjectionCompiler.java     |  2 +-
 .../apache/phoenix/compile/UpsertCompiler.java  | 10 +++--
 .../apache/phoenix/compile/WhereOptimizer.java  |  2 +
 .../java/org/apache/phoenix/util/ScanUtil.java  | 19 +++++++++
 6 files changed, 84 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/ed3e3f55/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificViewIndexIT.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificViewIndexIT.java
 
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificViewIndexIT.java
index 1f8eb55..e7cdc01 100644
--- 
a/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificViewIndexIT.java
+++ 
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificViewIndexIT.java
@@ -24,6 +24,7 @@ import static org.junit.Assert.fail;
 
 import java.sql.Connection;
 import java.sql.DriverManager;
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.util.Properties;
 
@@ -121,4 +122,47 @@ public class TenantSpecificViewIndexIT extends 
BaseTenantSpecificViewIndexIT {
         assertFalse(rs.next());
         
     }
+    
+    @Test
+    public void testQueryingUsingTenantSpecific() throws Exception {
+        String tenantId1 = "org1";
+        String tenantId2 = "org2";
+        String ddl = "CREATE TABLE T (tenantId char(15) NOT NULL, pk1 varchar 
NOT NULL, pk2 INTEGER NOT NULL, val1 VARCHAR CONSTRAINT pk primary key 
(tenantId,pk1,pk2)) MULTI_TENANT = true";
+        Connection conn = DriverManager.getConnection(getUrl());
+        conn.createStatement().execute(ddl);
+        String dml = "UPSERT INTO T (tenantId, pk1, pk2, val1) VALUES (?, ?, 
?, ?)";
+        PreparedStatement stmt = conn.prepareStatement(dml);
+        
+        String pk = "pk1b";
+        // insert two rows in table T. One for tenantId1 and other for 
tenantId2.
+        stmt.setString(1, tenantId1);
+        stmt.setString(2, pk);
+        stmt.setInt(3, 100);
+        stmt.setString(4, "value1");
+        stmt.executeUpdate();
+        
+        stmt.setString(1, tenantId2);
+        stmt.setString(2, pk);
+        stmt.setInt(3, 200);
+        stmt.setString(4, "value2");
+        stmt.executeUpdate();
+        conn.commit();
+        conn.close();
+        
+        // get a tenant specific url.
+        String tenantUrl = getUrl() + ';' + PhoenixRuntime.TENANT_ID_ATTRIB + 
'=' + tenantId1;
+        Connection tenantConn = DriverManager.getConnection(tenantUrl);
+        
+        // create a tenant specific view.
+        tenantConn.createStatement().execute("CREATE VIEW V AS select * from 
T");
+        String query = "SELECT val1 FROM V WHERE pk1 = ?";
+        
+        // using the tenant connection query the view.
+        PreparedStatement stmt2 = tenantConn.prepareStatement(query);
+        stmt2.setString(1, pk); // for tenantId1 the row inserted has pk1 = 
"pk1b"
+        ResultSet rs = stmt2.executeQuery();
+        assertTrue(rs.next());
+        assertEquals("value1", rs.getString(1));
+        assertFalse("No other rows should have been returned for the tenant", 
rs.next()); // should have just returned one record since for org1 we have only 
one row.
+    }
 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/ed3e3f55/phoenix-core/src/main/java/org/apache/phoenix/compile/DeleteCompiler.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/compile/DeleteCompiler.java 
b/phoenix-core/src/main/java/org/apache/phoenix/compile/DeleteCompiler.java
index 469bb30..2fd5535 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/DeleteCompiler.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/DeleteCompiler.java
@@ -64,6 +64,7 @@ import org.apache.phoenix.schema.MetaDataClient;
 import org.apache.phoenix.schema.MetaDataEntityNotFoundException;
 import org.apache.phoenix.schema.PColumn;
 import org.apache.phoenix.schema.PDataType;
+import org.apache.phoenix.schema.PName;
 import org.apache.phoenix.schema.PRow;
 import org.apache.phoenix.schema.PTable;
 import org.apache.phoenix.schema.PTableType;
@@ -73,6 +74,7 @@ import org.apache.phoenix.schema.TableRef;
 import org.apache.phoenix.schema.tuple.Tuple;
 import org.apache.phoenix.util.IndexUtil;
 import org.apache.phoenix.util.MetaDataUtil;
+import org.apache.phoenix.util.ScanUtil;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
@@ -87,22 +89,27 @@ public class DeleteCompiler {
     }
     
     private static MutationState deleteRows(PhoenixStatement statement, 
TableRef tableRef, ResultIterator iterator, RowProjector projector) throws 
SQLException {
+        PTable table = tableRef.getTable();
         PhoenixConnection connection = statement.getConnection();
-        byte[] tenantId = connection.getTenantId() == null ? null : 
connection.getTenantId().getBytes();
+        PName tenantId = connection.getTenantId();
+        byte[] tenantIdBytes = null;
+        if (tenantId != null) {
+            tenantId = 
ScanUtil.padTenantIdIfNecessary(table.getRowKeySchema(), table.getBucketNum() 
!= null, tenantId);
+            tenantIdBytes = tenantId.getBytes();
+        }
         final boolean isAutoCommit = connection.getAutoCommit();
         ConnectionQueryServices services = connection.getQueryServices();
         final int maxSize = 
services.getProps().getInt(QueryServices.MAX_MUTATION_SIZE_ATTRIB,QueryServicesOptions.DEFAULT_MAX_MUTATION_SIZE);
         final int batchSize = Math.min(connection.getMutateBatchSize(), 
maxSize);
         Map<ImmutableBytesPtr,Map<PColumn,byte[]>> mutations = 
Maps.newHashMapWithExpectedSize(batchSize);
         try {
-            PTable table = tableRef.getTable();
             List<PColumn> pkColumns = table.getPKColumns();
-            boolean isMultiTenant = table.isMultiTenant() && tenantId != null;
+            boolean isMultiTenant = table.isMultiTenant() && tenantIdBytes != 
null;
             boolean isSharedViewIndex = table.getViewIndexId() != null;
             int offset = (table.getBucketNum() == null ? 0 : 1);
             byte[][] values = new byte[pkColumns.size()][];
             if (isMultiTenant) {
-                values[offset++] = tenantId;
+                values[offset++] = tenantIdBytes;
             }
             if (isSharedViewIndex) {
                 values[offset++] = 
MetaDataUtil.getViewIndexIdDataType().toBytes(table.getViewIndexId());

http://git-wip-us.apache.org/repos/asf/phoenix/blob/ed3e3f55/phoenix-core/src/main/java/org/apache/phoenix/compile/ProjectionCompiler.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/compile/ProjectionCompiler.java 
b/phoenix-core/src/main/java/org/apache/phoenix/compile/ProjectionCompiler.java
index e3869de..f23c3ec 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/compile/ProjectionCompiler.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/compile/ProjectionCompiler.java
@@ -168,7 +168,7 @@ public class ProjectionCompiler {
         PhoenixConnection conn = context.getConnection();
         PName tenantId = conn.getTenantId();
         String tableName = index.getParentName().getString();
-        PTable table = conn.getMetaDataCache().getTable(new 
PTableKey(conn.getTenantId(), tableName));
+        PTable table = conn.getMetaDataCache().getTable(new 
PTableKey(tenantId, tableName));
         int tableOffset = table.getBucketNum() == null ? 0 : 1;
         int minTablePKOffset = getMinPKOffset(table, tenantId);
         int minIndexPKOffset = getMinPKOffset(index, tenantId);

http://git-wip-us.apache.org/repos/asf/phoenix/blob/ed3e3f55/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java 
b/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
index 6652ce3..046e375 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
@@ -74,6 +74,7 @@ import 
org.apache.phoenix.schema.MetaDataEntityNotFoundException;
 import org.apache.phoenix.schema.PColumn;
 import org.apache.phoenix.schema.PColumnImpl;
 import org.apache.phoenix.schema.PDataType;
+import org.apache.phoenix.schema.PName;
 import org.apache.phoenix.schema.PTable;
 import org.apache.phoenix.schema.PTable.ViewType;
 import org.apache.phoenix.schema.PTableImpl;
@@ -86,6 +87,7 @@ import org.apache.phoenix.schema.tuple.Tuple;
 import org.apache.phoenix.util.ByteUtil;
 import org.apache.phoenix.util.IndexUtil;
 import org.apache.phoenix.util.MetaDataUtil;
+import org.apache.phoenix.util.ScanUtil;
 import org.apache.phoenix.util.SchemaUtil;
 
 import com.google.common.collect.Lists;
@@ -218,7 +220,7 @@ public class UpsertCompiler {
         List<PColumn> allColumnsToBe = Collections.emptyList();
         boolean isTenantSpecific = false;
         boolean isSharedViewIndex = false;
-        String tenantId = null;
+        String tenantIdStr = null;
         ColumnResolver resolver = null;
         int[] columnIndexesToBe;
         int nColumnsToSet = 0;
@@ -250,7 +252,7 @@ public class UpsertCompiler {
                 boolean isSalted = table.getBucketNum() != null;
                 isTenantSpecific = table.isMultiTenant() && 
connection.getTenantId() != null;
                 isSharedViewIndex = table.getViewIndexId() != null;
-                tenantId = isTenantSpecific ? 
connection.getTenantId().getString() : null;
+                tenantIdStr = isTenantSpecific ? 
connection.getTenantId().getString() : null;
                 int posOffset = isSalted ? 1 : 0;
                 // Setup array of column indexes parallel to values that are 
going to be set
                 allColumnsToBe = table.getColumns();
@@ -371,7 +373,7 @@ public class UpsertCompiler {
                     select = SubselectRewriter.flatten(select, connection);
                     ColumnResolver selectResolver = 
FromCompiler.getResolverForQuery(select, connection);
                     select = StatementNormalizer.normalize(select, 
selectResolver);
-                    select = prependTenantAndViewConstants(table, select, 
tenantId, addViewColumnsToBe);
+                    select = prependTenantAndViewConstants(table, select, 
tenantIdStr, addViewColumnsToBe);
                     SelectStatement transformedSelect = 
SubqueryRewriter.transform(select, selectResolver, connection);
                     if (transformedSelect != select) {
                         selectResolver = 
FromCompiler.getResolverForQuery(transformedSelect, connection);
@@ -693,6 +695,8 @@ public class UpsertCompiler {
         // initialze values with constant byte values first
         final byte[][] values = new byte[nValuesToSet][];
         if (isTenantSpecific) {
+            PName tenantId = connection.getTenantId();
+            tenantId = 
ScanUtil.padTenantIdIfNecessary(table.getRowKeySchema(), table.getBucketNum() 
!= null, tenantId);
             values[nodeIndex++] = connection.getTenantId().getBytes();
         }
         if (isSharedViewIndex) {

http://git-wip-us.apache.org/repos/asf/phoenix/blob/ed3e3f55/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java 
b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
index bb9e351..29ad6ee 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
@@ -64,6 +64,7 @@ import org.apache.phoenix.schema.SortOrder;
 import org.apache.phoenix.schema.tuple.Tuple;
 import org.apache.phoenix.util.ByteUtil;
 import org.apache.phoenix.util.MetaDataUtil;
+import org.apache.phoenix.util.ScanUtil;
 import org.apache.phoenix.util.SchemaUtil;
 import org.apache.phoenix.util.StringUtil;
 
@@ -180,6 +181,7 @@ public class WhereOptimizer {
         
         // Add tenant data isolation for tenant-specific tables
         if (isMultiTenant) {
+            tenantId = ScanUtil.padTenantIdIfNecessary(schema, isSalted, 
tenantId);
             byte[] tenantIdBytes = tenantId.getBytes();
             KeyRange tenantIdKeyRange = KeyRange.getKeyRange(tenantIdBytes);
             cnf.add(singletonList(tenantIdKeyRange));

http://git-wip-us.apache.org/repos/asf/phoenix/blob/ed3e3f55/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java 
b/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java
index c0da0bb..90fbba7 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java
@@ -41,7 +41,10 @@ import org.apache.phoenix.query.KeyRange;
 import org.apache.phoenix.query.KeyRange.Bound;
 import org.apache.phoenix.query.QueryConstants;
 import org.apache.phoenix.schema.PDataType;
+import org.apache.phoenix.schema.PName;
+import org.apache.phoenix.schema.PNameFactory;
 import org.apache.phoenix.schema.RowKeySchema;
+import org.apache.phoenix.schema.ValueSchema.Field;
 
 import com.google.common.collect.Lists;
 
@@ -519,4 +522,20 @@ public class ScanUtil {
         }
         return Bytes.compareTo(key, 0, nBytesToCheck, ZERO_BYTE_ARRAY, 0, 
nBytesToCheck) != 0;
     }
+    
+    public static PName padTenantIdIfNecessary(RowKeySchema schema, boolean 
isSalted, PName tenantId) {
+        int pkPos = isSalted ? 1 : 0;
+        String tenantIdStr = tenantId.getString();
+        Field field = schema.getField(pkPos);
+        PDataType dataType = field.getDataType();
+        boolean isFixedWidth = dataType.isFixedWidth();
+        Integer maxLength = isFixedWidth ? field.getByteSize() : null;
+        if (isFixedWidth && maxLength != null) {
+            if (tenantIdStr.length() < maxLength) {
+                tenantIdStr = (String)dataType.pad(tenantIdStr, maxLength);
+                return PNameFactory.newName(tenantIdStr);
+            }
+        }
+        return tenantId;
+    }
 }
\ No newline at end of file

Reply via email to