You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ja...@apache.org on 2014/10/11 11:52:27 UTC

[1/3] git commit: PHOENIX-1338 Logic to group together parallel scans is incorrect

Repository: phoenix
Updated Branches:
  refs/heads/master 1a991c5c8 -> c64cef6d8


PHOENIX-1338 Logic to group together parallel scans is incorrect


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

Branch: refs/heads/master
Commit: 3fc43d966f7eee2478a413941d63ffe2596ff910
Parents: 1a991c5
Author: James Taylor <jt...@salesforce.com>
Authored: Fri Oct 10 20:44:42 2014 -0700
Committer: James Taylor <jt...@salesforce.com>
Committed: Sat Oct 11 02:56:17 2014 -0700

----------------------------------------------------------------------
 .../org/apache/phoenix/end2end/BaseViewIT.java  | 25 ++++++++++++----
 .../phoenix/iterate/ParallelIterators.java      | 31 +++++++++-----------
 2 files changed, 33 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/3fc43d96/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseViewIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseViewIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseViewIT.java
index 4255e3f..13e743f 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseViewIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseViewIT.java
@@ -17,6 +17,8 @@
  */
 package org.apache.phoenix.end2end;
 
+import static org.apache.phoenix.util.TestUtil.analyzeTable;
+import static org.apache.phoenix.util.TestUtil.getAllSplits;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -25,8 +27,10 @@ import java.math.BigDecimal;
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.ResultSet;
+import java.util.List;
 import java.util.Map;
 
+import org.apache.phoenix.query.KeyRange;
 import org.apache.phoenix.query.QueryServices;
 import org.apache.phoenix.util.QueryUtil;
 import org.apache.phoenix.util.ReadOnlyProps;
@@ -110,9 +114,10 @@ public abstract class BaseViewIT extends BaseOwnClusterHBaseManagedTimeIT {
         }
         conn.createStatement().execute("UPSERT INTO v(k2,S,k3) VALUES(120,'foo',50.0)");
 
-//        analyzeTable(conn, "v");        
-//        List<KeyRange> splits = getAllSplits(conn, "i1");
-//        assertEquals(4, splits.size());
+        analyzeTable(conn, "v");        
+        List<KeyRange> splits = getAllSplits(conn, "i1");
+        // More guideposts with salted, since it's already pre-split at salt buckets
+        assertEquals(saltBuckets == null ? 6 : 8, splits.size());
         
         String query = "SELECT k1, k2, k3, s FROM v WHERE k3 = 51.0";
         rs = conn.createStatement().executeQuery(query);
@@ -123,14 +128,15 @@ public abstract class BaseViewIT extends BaseOwnClusterHBaseManagedTimeIT {
         assertEquals("bar", rs.getString(4));
         assertFalse(rs.next());
         rs = conn.createStatement().executeQuery("EXPLAIN " + query);
+        String queryPlan = QueryUtil.getExplainPlan(rs);
         if (localIndex) {
             assertEquals("CLIENT PARALLEL 3-WAY RANGE SCAN OVER _LOCAL_IDX_T [-32768,51]\nCLIENT MERGE SORT",
-                QueryUtil.getExplainPlan(rs));
+                queryPlan);
         } else {
             assertEquals(saltBuckets == null
                     ? "CLIENT PARALLEL 1-WAY RANGE SCAN OVER _IDX_T [" + Short.MIN_VALUE + ",51]"
                             : "CLIENT PARALLEL " + saltBuckets + "-WAY RANGE SCAN OVER _IDX_T [0," + Short.MIN_VALUE + ",51]\nCLIENT MERGE SORT",
-                            QueryUtil.getExplainPlan(rs));
+                            queryPlan);
         }
 
         if (localIndex) {
@@ -139,8 +145,15 @@ public abstract class BaseViewIT extends BaseOwnClusterHBaseManagedTimeIT {
             conn.createStatement().execute("CREATE INDEX i2 on v(s)");
         }
         
+        // new index hasn't been analyzed yet
+        splits = getAllSplits(conn, "i2");
+        assertEquals(saltBuckets == null ? 1 : 3, splits.size());
+        
+        // analyze table should analyze all view data
+//        analyzeTable(conn, "t");        
 //        splits = getAllSplits(conn, "i2");
-//        assertEquals(4, splits.size());
+//        assertEquals(saltBuckets == null ? 6 : 8, splits.size());
+
         
         query = "SELECT k1, k2, s FROM v WHERE s = 'foo'";
         rs = conn.createStatement().executeQuery(query);

http://git-wip-us.apache.org/repos/asf/phoenix/blob/3fc43d96/phoenix-core/src/main/java/org/apache/phoenix/iterate/ParallelIterators.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/iterate/ParallelIterators.java b/phoenix-core/src/main/java/org/apache/phoenix/iterate/ParallelIterators.java
index a55ae70..f003a39 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/iterate/ParallelIterators.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/iterate/ParallelIterators.java
@@ -180,7 +180,6 @@ public class ParallelIterators extends ExplainTable implements ResultIterators {
         
         this.iteratorFactory = iteratorFactory;
         this.scans = getParallelScans(context.getScan());
-        List<List<Scan>> scans = getParallelScans(context.getScan());
         List<KeyRange> splitRanges = Lists.newArrayListWithExpectedSize(scans.size() * ESTIMATED_GUIDEPOSTS_PER_REGION);
         for (List<Scan> scanList : scans) {
             for (Scan aScan : scanList) {
@@ -365,26 +364,24 @@ public class ParallelIterators extends ExplainTable implements ResultIterators {
             return scans;
         }
         PTable table = getTable();
-        if (!scans.isEmpty()) {
-            boolean startNewScanList = false;
-            if (!plan.isRowKeyOrdered()) {
+        boolean startNewScanList = false;
+        if (!plan.isRowKeyOrdered()) {
+            startNewScanList = true;
+        } else if (crossedRegionBoundary) {
+            if (table.getIndexType() == IndexType.LOCAL) {
                 startNewScanList = true;
-            } else if (crossedRegionBoundary) {
-                if (table.getIndexType() == IndexType.LOCAL) {
-                    startNewScanList = true;
-                } else if (table.getBucketNum() != null) {
-                    byte[] previousStartKey = scans.get(scans.size()-1).getStartRow();
-                    byte[] currentStartKey = scan.getStartRow();
-                    byte[] prefix = ScanUtil.getPrefix(previousStartKey, SaltingUtil.NUM_SALTING_BYTES);
-                    startNewScanList = ScanUtil.crossesPrefixBoundary(currentStartKey, prefix, SaltingUtil.NUM_SALTING_BYTES);
-                }
-            }
-            if (startNewScanList) {
-                parallelScans.add(scans);
-                scans = Lists.newArrayListWithExpectedSize(1);
+            } else if (table.getBucketNum() != null) {
+                startNewScanList = scans.isEmpty() ||
+                        ScanUtil.crossesPrefixBoundary(scan.getStartRow(),
+                                ScanUtil.getPrefix(scans.get(scans.size()-1).getStartRow(), SaltingUtil.NUM_SALTING_BYTES), 
+                                SaltingUtil.NUM_SALTING_BYTES);
             }
         }
         scans.add(scan);
+        if (startNewScanList) {
+            parallelScans.add(scans);
+            scans = Lists.newArrayListWithExpectedSize(1);
+        }
         return scans;
     }
     /**


[3/3] git commit: PHOENIX-1309 Ensure Phoenix table is created for Local index and view index tables to store guideposts against them

Posted by ja...@apache.org.
PHOENIX-1309 Ensure Phoenix table is created for Local index and view index tables to store guideposts against them


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

Branch: refs/heads/master
Commit: c64cef6d80eb7af6f26fdc82db9c9be1e14e8184
Parents: 43afafb
Author: James Taylor <jt...@salesforce.com>
Authored: Fri Oct 10 23:25:30 2014 -0700
Committer: James Taylor <jt...@salesforce.com>
Committed: Sat Oct 11 02:56:56 2014 -0700

----------------------------------------------------------------------
 .../org/apache/phoenix/end2end/BaseViewIT.java  |   6 +-
 .../end2end/TenantSpecificViewIndexIT.java      |   2 +-
 .../apache/phoenix/compile/PostDDLCompiler.java |  14 +-
 .../phoenix/compile/StatementContext.java       |   4 +
 .../apache/phoenix/schema/DelegateTable.java    | 228 +++++++++++++++++++
 .../apache/phoenix/schema/MetaDataClient.java   | 106 +++++----
 .../java/org/apache/phoenix/query/BaseTest.java |   2 +-
 7 files changed, 308 insertions(+), 54 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/c64cef6d/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseViewIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseViewIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseViewIT.java
index 13e743f..2fe7fd1 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseViewIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseViewIT.java
@@ -150,9 +150,9 @@ public abstract class BaseViewIT extends BaseOwnClusterHBaseManagedTimeIT {
         assertEquals(saltBuckets == null ? 1 : 3, splits.size());
         
         // analyze table should analyze all view data
-//        analyzeTable(conn, "t");        
-//        splits = getAllSplits(conn, "i2");
-//        assertEquals(saltBuckets == null ? 6 : 8, splits.size());
+        analyzeTable(conn, "t");        
+        splits = getAllSplits(conn, "i2");
+        assertEquals(saltBuckets == null ? 6 : 8, splits.size());
 
         
         query = "SELECT k1, k2, s FROM v WHERE s = 'foo'";

http://git-wip-us.apache.org/repos/asf/phoenix/blob/c64cef6d/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 4ccdef4..094222b 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
@@ -151,7 +151,7 @@ public class TenantSpecificViewIndexIT extends BaseTenantSpecificViewIndexIT {
     }
     
     @Test
-    public void testQueryingUsingTenantSpecific() throws Exception {
+    public void testNonPaddedTenantId() 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";

http://git-wip-us.apache.org/repos/asf/phoenix/blob/c64cef6d/phoenix-core/src/main/java/org/apache/phoenix/compile/PostDDLCompiler.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/PostDDLCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/PostDDLCompiler.java
index 294942f..033995e 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/PostDDLCompiler.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/PostDDLCompiler.java
@@ -68,8 +68,13 @@ public class PostDDLCompiler {
     private final StatementContext context; // bogus context
 
     public PostDDLCompiler(PhoenixConnection connection) {
+        this(connection, new Scan());
+    }
+
+    public PostDDLCompiler(PhoenixConnection connection, Scan scan) {
         this.connection = connection;
-        this.context = new StatementContext(new PhoenixStatement(connection));
+        this.context = new StatementContext(new PhoenixStatement(connection), scan);
+        scan.setAttribute(BaseScannerRegionObserver.UNGROUPED_AGG, QueryConstants.TRUE);
     }
 
     public MutationPlan compile(final List<TableRef> tableRefs, final byte[] emptyCF, final byte[] projectCF, final List<PColumn> deleteList,
@@ -101,19 +106,16 @@ public class PostDDLCompiler {
                 try {
                     connection.setAutoCommit(true);
                     SQLException sqlE = null;
-                    if (deleteList == null && emptyCF == null) {
-                        return new MutationState(0, connection);
-                    }
                     /*
                      * Handles:
                      * 1) deletion of all rows for a DROP TABLE and subsequently deletion of all rows for a DROP INDEX;
                      * 2) deletion of all column values for a ALTER TABLE DROP COLUMN
                      * 3) updating the necessary rows to have an empty KV
+                     * 4) updating table stats
                      */
                     long totalMutationCount = 0;
                     for (final TableRef tableRef : tableRefs) {
-                        Scan scan = new Scan();
-                        scan.setAttribute(BaseScannerRegionObserver.UNGROUPED_AGG, QueryConstants.TRUE);
+                        Scan scan = ScanUtil.newScan(context.getScan());
                         SelectStatement select = SelectStatement.COUNT_ONE;
                         // We need to use this tableRef
                         ColumnResolver resolver = new ColumnResolver() {

http://git-wip-us.apache.org/repos/asf/phoenix/blob/c64cef6d/phoenix-core/src/main/java/org/apache/phoenix/compile/StatementContext.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/StatementContext.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/StatementContext.java
index 242fc45..b0ba6f0 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/StatementContext.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/StatementContext.java
@@ -79,6 +79,10 @@ public class StatementContext {
     private Map<SelectStatement, Object> subqueryResults;
     
     public StatementContext(PhoenixStatement statement) {
+        this(statement, new Scan());
+    }
+    
+    public StatementContext(PhoenixStatement statement, Scan scan) {
         this(statement, FromCompiler.EMPTY_TABLE_RESOLVER, new Scan(), new SequenceManager(statement));
     }
     

http://git-wip-us.apache.org/repos/asf/phoenix/blob/c64cef6d/phoenix-core/src/main/java/org/apache/phoenix/schema/DelegateTable.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/DelegateTable.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/DelegateTable.java
new file mode 100644
index 0000000..f4bf1cc
--- /dev/null
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/DelegateTable.java
@@ -0,0 +1,228 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.schema;
+
+import java.util.List;
+
+import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
+import org.apache.phoenix.hbase.index.util.KeyValueBuilder;
+import org.apache.phoenix.index.IndexMaintainer;
+import org.apache.phoenix.schema.stats.PTableStats;
+
+public class DelegateTable implements PTable {
+    @Override
+    public long getTimeStamp() {
+        return delegate.getTimeStamp();
+    }
+
+    @Override
+    public long getSequenceNumber() {
+        return delegate.getSequenceNumber();
+    }
+
+    @Override
+    public PName getName() {
+        return delegate.getName();
+    }
+
+    @Override
+    public PName getSchemaName() {
+        return delegate.getSchemaName();
+    }
+
+    @Override
+    public PName getTableName() {
+        return delegate.getTableName();
+    }
+
+    @Override
+    public PName getTenantId() {
+        return delegate.getTenantId();
+    }
+
+    @Override
+    public PTableType getType() {
+        return delegate.getType();
+    }
+
+    @Override
+    public PName getPKName() {
+        return delegate.getPKName();
+    }
+
+    @Override
+    public List<PColumn> getPKColumns() {
+        return delegate.getPKColumns();
+    }
+
+    @Override
+    public List<PColumn> getColumns() {
+        return delegate.getColumns();
+    }
+
+    @Override
+    public List<PColumnFamily> getColumnFamilies() {
+        return delegate.getColumnFamilies();
+    }
+
+    @Override
+    public PColumnFamily getColumnFamily(byte[] family) throws ColumnFamilyNotFoundException {
+        return delegate.getColumnFamily(family);
+    }
+
+    @Override
+    public PColumnFamily getColumnFamily(String family) throws ColumnFamilyNotFoundException {
+        return delegate.getColumnFamily(family);
+    }
+
+    @Override
+    public PColumn getColumn(String name) throws ColumnNotFoundException, AmbiguousColumnException {
+        return delegate.getColumn(name);
+    }
+
+    @Override
+    public PColumn getPKColumn(String name) throws ColumnNotFoundException {
+        return delegate.getPKColumn(name);
+    }
+
+    @Override
+    public PRow newRow(KeyValueBuilder builder, long ts, ImmutableBytesWritable key, byte[]... values) {
+        return delegate.newRow(builder, ts, key, values);
+    }
+
+    @Override
+    public PRow newRow(KeyValueBuilder builder, ImmutableBytesWritable key, byte[]... values) {
+        return delegate.newRow(builder, key, values);
+    }
+
+    @Override
+    public int newKey(ImmutableBytesWritable key, byte[][] values) {
+        return delegate.newKey(key, values);
+    }
+
+    @Override
+    public RowKeySchema getRowKeySchema() {
+        return delegate.getRowKeySchema();
+    }
+
+    @Override
+    public Integer getBucketNum() {
+        return delegate.getBucketNum();
+    }
+
+    @Override
+    public List<PTable> getIndexes() {
+        return delegate.getIndexes();
+    }
+
+    @Override
+    public PIndexState getIndexState() {
+        return delegate.getIndexState();
+    }
+
+    @Override
+    public PName getParentName() {
+        return delegate.getParentName();
+    }
+
+    @Override
+    public PName getParentTableName() {
+        return delegate.getParentTableName();
+    }
+
+    @Override
+    public List<PName> getPhysicalNames() {
+        return delegate.getPhysicalNames();
+    }
+
+    @Override
+    public PName getPhysicalName() {
+        return delegate.getPhysicalName();
+    }
+
+    @Override
+    public boolean isImmutableRows() {
+        return delegate.isImmutableRows();
+    }
+
+    @Override
+    public void getIndexMaintainers(ImmutableBytesWritable ptr) {
+        delegate.getIndexMaintainers(ptr);
+    }
+
+    @Override
+    public IndexMaintainer getIndexMaintainer(PTable dataTable) {
+        return delegate.getIndexMaintainer(dataTable);
+    }
+
+    @Override
+    public PName getDefaultFamilyName() {
+        return delegate.getDefaultFamilyName();
+    }
+
+    @Override
+    public boolean isWALDisabled() {
+        return delegate.isWALDisabled();
+    }
+
+    @Override
+    public boolean isMultiTenant() {
+        return delegate.isMultiTenant();
+    }
+
+    @Override
+    public ViewType getViewType() {
+        return delegate.getViewType();
+    }
+
+    @Override
+    public String getViewStatement() {
+        return delegate.getViewStatement();
+    }
+
+    @Override
+    public Short getViewIndexId() {
+        return delegate.getViewIndexId();
+    }
+
+    @Override
+    public PTableKey getKey() {
+        return delegate.getKey();
+    }
+
+    @Override
+    public int getEstimatedSize() {
+        return delegate.getEstimatedSize();
+    }
+
+    @Override
+    public IndexType getIndexType() {
+        return delegate.getIndexType();
+    }
+
+    @Override
+    public PTableStats getTableStats() {
+        return delegate.getTableStats();
+    }
+
+    private final PTable delegate;
+    
+    public DelegateTable(PTable delegate) {
+        this.delegate = delegate;
+    }
+}

http://git-wip-us.apache.org/repos/asf/phoenix/blob/c64cef6d/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
index 9b949b7..8e63cf3 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
@@ -490,33 +490,58 @@ public class MetaDataClient {
     public MutationState updateStatistics(UpdateStatisticsStatement updateStatisticsStmt)
             throws SQLException {
         // Check before updating the stats if we have reached the configured time to reupdate the stats once again
-        ReadOnlyProps props = connection.getQueryServices().getProps();
-        final long msMinBetweenUpdates = props
-                .getLong(QueryServices.MIN_STATS_UPDATE_FREQ_MS_ATTRIB,
-                        props.getLong(QueryServices.STATS_UPDATE_FREQ_MS_ATTRIB, 
-                                QueryServicesOptions.DEFAULT_STATS_UPDATE_FREQ_MS) / 2);
         ColumnResolver resolver = FromCompiler.getResolver(updateStatisticsStmt, connection);
         PTable table = resolver.getTables().get(0).getTable();
-        List<PTable> indexes = table.getIndexes();
-        List<PTable> tables = Lists.newArrayListWithExpectedSize(1 + indexes.size());
+        long rowCount = 0;
         if (updateStatisticsStmt.updateColumns()) {
-            tables.add(table);
+            rowCount += updateStatisticsInternal(table.getPhysicalName(), table);
         }
         if (updateStatisticsStmt.updateIndex()) {
-            tables.addAll(indexes);
-        }
-        for(PTable pTable : tables) {
-            updateStatisticsInternal(msMinBetweenUpdates, pTable);
+            // TODO: If our table is a VIEW with multiple indexes or a TABLE with local indexes,
+            // we may be doing more work that we have to here. We should union the scan ranges
+            // across all indexes in that case so that we don't re-calculate the same stats
+            // multiple times.
+            for (PTable index : table.getIndexes()) {
+                rowCount += updateStatisticsInternal(index.getPhysicalName(), index);
+            }
+            // If analyzing the indexes of a multi-tenant table or a table with view indexes
+            // then analyze all of those indexes too.
+            boolean isLocalIndex = false;
+            boolean isViewIndex = false;
+            if (table.getType() != PTableType.VIEW &&
+               (table.isMultiTenant())
+               || (isViewIndex = MetaDataUtil.hasViewIndexTable(connection, table.getName()))
+               || (isLocalIndex = MetaDataUtil.hasLocalIndexTable(connection, table.getName()))) {
+                
+                String viewIndexTableName = isLocalIndex ? MetaDataUtil.getLocalIndexTableName(table.getTableName().getString()) : MetaDataUtil.getViewIndexTableName(table.getTableName().getString());
+                String viewIndexSchemaName = isLocalIndex ? MetaDataUtil.getLocalIndexSchemaName(table.getSchemaName().getString()) : MetaDataUtil.getViewIndexSchemaName(table.getSchemaName().getString());
+                final PName viewIndexPhysicalName = PNameFactory.newName(SchemaUtil.getTableName(viewIndexSchemaName, viewIndexTableName));
+                PTable indexLogicalTable = new DelegateTable(table) {
+                    @Override
+                    public PName getPhysicalName() {
+                        return viewIndexPhysicalName;
+                    }
+                    @Override
+                    public PTableStats getTableStats() {
+                        return PTableStats.EMPTY_STATS;
+                    }
+                };
+                rowCount += updateStatisticsInternal(viewIndexPhysicalName, indexLogicalTable);
+            }
         }
-        return new MutationState(1, connection);
+        return new MutationState((int)rowCount, connection);
     }
 
-    private MutationState updateStatisticsInternal(long msMinBetweenUpdates, PTable table) throws SQLException {
-        PName physicalName = table.getPhysicalName();
+    private long updateStatisticsInternal(PName physicalName, PTable logicalTable) throws SQLException {
+        ReadOnlyProps props = connection.getQueryServices().getProps();
+        final long msMinBetweenUpdates = props
+                .getLong(QueryServices.MIN_STATS_UPDATE_FREQ_MS_ATTRIB,
+                        props.getLong(QueryServices.STATS_UPDATE_FREQ_MS_ATTRIB, 
+                                QueryServicesOptions.DEFAULT_STATS_UPDATE_FREQ_MS) / 2);
         byte[] tenantIdBytes = ByteUtil.EMPTY_BYTE_ARRAY;
         Long scn = connection.getSCN();
         // Always invalidate the cache
-        long clientTS = connection.getSCN() == null ? HConstants.LATEST_TIMESTAMP : scn;
+        long clientTimeStamp = connection.getSCN() == null ? HConstants.LATEST_TIMESTAMP : scn;
         String query = "SELECT CURRENT_DATE() - " + LAST_STATS_UPDATE_TIME + " FROM " + PhoenixDatabaseMetaData.SYSTEM_STATS_NAME
                 + " WHERE " + PHYSICAL_NAME + "='" + physicalName.getString() + "' AND " + COLUMN_FAMILY
                 + " IS NULL AND " + REGION_NAME + " IS NULL AND " + LAST_STATS_UPDATE_TIME + " IS NOT NULL";
@@ -525,32 +550,29 @@ public class MetaDataClient {
         if (rs.next()) {
             msSinceLastUpdate = rs.getLong(1);
         }
-        if (msSinceLastUpdate >= msMinBetweenUpdates) {
-            // Here create the select query.
-            String countQuery = "SELECT /*+ NO_CACHE NO_INDEX */ count(*) FROM " + table.getName().getString();
-            PhoenixStatement statement = (PhoenixStatement) connection.createStatement();
-            QueryPlan plan = statement.compileQuery(countQuery);
-            Scan scan = plan.getContext().getScan();
-            // Add all CF in the table
-            scan.getFamilyMap().clear();
-            for (PColumnFamily family : table.getColumnFamilies()) {
-                scan.addFamily(family.getName().getBytes());
-            }
-            scan.setAttribute(BaseScannerRegionObserver.ANALYZE_TABLE, PDataType.TRUE_BYTES);
-            Cell kv = plan.iterator().next().getValue(0);
-            ImmutableBytesWritable tempPtr = plan.getContext().getTempPtr();
-            tempPtr.set(kv.getValueArray(), kv.getValueOffset(), kv.getValueLength());
-            // A single Cell will be returned with the count(*) - we decode that here
-            long rowCount = PDataType.LONG.getCodec().decodeLong(tempPtr, SortOrder.getDefault());
-            // We need to update the stats table so that client will pull the new one with
-            // the updated stats.
-            connection.getQueryServices().incrementTableTimeStamp(tenantIdBytes,
-                    Bytes.toBytes(SchemaUtil.getSchemaNameFromFullName(physicalName.getString())),
-                    Bytes.toBytes(SchemaUtil.getTableNameFromFullName(physicalName.getString())), clientTS);
-            return new  MutationState(0, connection, rowCount);
-        } else {
-            return new MutationState(0, connection);
+        if (msSinceLastUpdate < msMinBetweenUpdates) {
+            return 0;
         }
+        
+        /*
+         * Execute a COUNT(*) through PostDDLCompiler as we need to use the logicalTable passed through,
+         * since it may not represent a "real" table in the case of the view indexes of a base table.
+         */
+        PostDDLCompiler compiler = new PostDDLCompiler(connection);
+        TableRef tableRef = new TableRef(null, logicalTable, clientTimeStamp, false);
+        MutationPlan plan = compiler.compile(Collections.singletonList(tableRef), null, null, null, clientTimeStamp);
+        Scan scan = plan.getContext().getScan();
+        scan.setCacheBlocks(false);
+        scan.setAttribute(BaseScannerRegionObserver.ANALYZE_TABLE, PDataType.TRUE_BYTES);
+        MutationState mutationState = plan.execute();
+        long rowCount = mutationState.getUpdateCount();
+
+        // We need to update the stats table so that client will pull the new one with
+        // the updated stats.
+        connection.getQueryServices().incrementTableTimeStamp(tenantIdBytes,
+                Bytes.toBytes(SchemaUtil.getSchemaNameFromFullName(physicalName.getString())),
+                Bytes.toBytes(SchemaUtil.getTableNameFromFullName(physicalName.getString())), clientTimeStamp);
+        return rowCount;
     }
 
     private MutationState buildIndexAtTimeStamp(PTable index, NamedTableNode dataTableNode) throws SQLException {
@@ -970,7 +992,6 @@ public class MetaDataClient {
             String parentTableName = null;
             PName tenantId = connection.getTenantId();
             String tenantIdStr = tenantId == null ? null : connection.getTenantId().getString();
-            boolean isParentImmutableRows = false;
             boolean multiTenant = false;
             Integer saltBucketNum = null;
             String defaultFamilyName = null;
@@ -998,7 +1019,6 @@ public class MetaDataClient {
                 }
                 
                 multiTenant = parent.isMultiTenant();
-                isParentImmutableRows = parent.isImmutableRows();
                 parentTableName = parent.getTableName().getString();
                 // Pass through data table sequence number so we can check it hasn't changed
                 PreparedStatement incrementStatement = connection.prepareStatement(INCREMENT_SEQ_NUM);

http://git-wip-us.apache.org/repos/asf/phoenix/blob/c64cef6d/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java b/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
index cdda1f1..b1f90a3 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
@@ -553,7 +553,7 @@ public abstract class BaseTest {
                 @Override
                 public void run() {
                     try {
-                        utility.shutdownMiniCluster();
+                        if (utility != null) utility.shutdownMiniCluster();
                     } catch (Exception e) {
                         logger.warn("Exception caught when shutting down mini cluster", e);
                     }


[2/3] git commit: PHOENIX-1337 Unpadded fixed length tenant ID causes erroneous results

Posted by ja...@apache.org.
PHOENIX-1337 Unpadded fixed length tenant ID causes erroneous results


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

Branch: refs/heads/master
Commit: 43afafbe1be31f5781120e90b6cb2e46280de276
Parents: 3fc43d9
Author: James Taylor <jt...@salesforce.com>
Authored: Fri Oct 10 21:10:12 2014 -0700
Committer: James Taylor <jt...@salesforce.com>
Committed: Sat Oct 11 02:56:40 2014 -0700

----------------------------------------------------------------------
 .../end2end/TenantSpecificViewIndexIT.java      | 44 ++++++++++++++++++++
 .../apache/phoenix/compile/DeleteCompiler.java  | 17 +++++---
 .../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, 85 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/43afafbe/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 20104a4..4ccdef4 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;
 
@@ -148,4 +149,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/43afafbe/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 868c4cd..9a313b7 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,16 +64,18 @@ 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.PTable.IndexType;
 import org.apache.phoenix.schema.PTableType;
 import org.apache.phoenix.schema.ReadOnlyTableException;
 import org.apache.phoenix.schema.SortOrder;
 import org.apache.phoenix.schema.TableRef;
-import org.apache.phoenix.schema.PTable.IndexType;
 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;
@@ -88,22 +90,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/43afafbe/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 b7277b3..0647e2e 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/43afafbe/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 3381aa8..d7129bf 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/43afafbe/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 64a49c8..5803bd2 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/43afafbe/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 e321c9c..4f7fcff 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
@@ -46,7 +46,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;
 
@@ -623,4 +626,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 = field.getMaxLength();
+        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