You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by gj...@apache.org on 2019/02/12 23:45:38 UTC

[phoenix] branch 4.x-HBase-1.3 updated: PHOENIX-5132 - View indexes with different owners but of the same base table can be assigned same ViewIndexId

This is an automated email from the ASF dual-hosted git repository.

gjacoby pushed a commit to branch 4.x-HBase-1.3
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/4.x-HBase-1.3 by this push:
     new 6239472  PHOENIX-5132 - View indexes with different owners but of the same base table can be assigned same ViewIndexId
6239472 is described below

commit 6239472f96ee13453ed71208d0d5c6ef47765e24
Author: Geoffrey Jacoby <gj...@apache.org>
AuthorDate: Mon Feb 11 16:23:13 2019 -0800

    PHOENIX-5132 - View indexes with different owners but of the same base table can be assigned same ViewIndexId
---
 .../end2end/BaseTenantSpecificViewIndexIT.java     | 19 +++---
 .../phoenix/end2end/TenantSpecificViewIndexIT.java | 20 ++++---
 .../apache/phoenix/end2end/index/ViewIndexIT.java  | 67 +++++++++++++++++++++-
 .../java/org/apache/phoenix/util/MetaDataUtil.java | 20 +++++--
 4 files changed, 102 insertions(+), 24 deletions(-)

diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseTenantSpecificViewIndexIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseTenantSpecificViewIndexIT.java
index 26e2860..216e2d3 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseTenantSpecificViewIndexIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BaseTenantSpecificViewIndexIT.java
@@ -41,7 +41,7 @@ public class BaseTenantSpecificViewIndexIT extends SplitSystemCatalogIT {
     public static final String NON_STRING_TENANT_ID = "1234";
     
     protected Set<Pair<String, String>> tenantViewsToDelete = newHashSet();
-    
+
     protected void testUpdatableView(Integer saltBuckets) throws Exception {
         testUpdatableView(saltBuckets, false);
     }
@@ -53,7 +53,7 @@ public class BaseTenantSpecificViewIndexIT extends SplitSystemCatalogIT {
         Connection conn = createTenantConnection(TENANT1);
         try {
             createAndPopulateTenantView(conn, TENANT1, tableName, "", viewName);
-            createAndVerifyIndex(conn, viewName, tableName, saltBuckets, TENANT1, "", localIndex);
+            createAndVerifyIndex(conn, viewName, tableName, saltBuckets, TENANT1, "", localIndex,0L);
             verifyViewData(conn, viewName, "");
         } finally {
             try { conn.close();} catch (Exception ignored) {}
@@ -93,8 +93,8 @@ public class BaseTenantSpecificViewIndexIT extends SplitSystemCatalogIT {
             createAndPopulateTenantView(conn1, TENANT1, tableName, prefixForTenant1Data, viewName1);
             createAndPopulateTenantView(conn2, TENANT2, tableName, prefixForTenant2Data, viewName2);
             
-            createAndVerifyIndex(conn1, viewName1, tableName, saltBuckets, TENANT1, prefixForTenant1Data, localIndex);
-            createAndVerifyIndex(conn2, viewName2, tableName, saltBuckets, TENANT2, prefixForTenant2Data, localIndex);
+            createAndVerifyIndex(conn1, viewName1, tableName, saltBuckets, TENANT1, prefixForTenant1Data, localIndex,0L);
+            createAndVerifyIndex(conn2, viewName2, tableName, saltBuckets, TENANT2, prefixForTenant2Data, localIndex,1L);
             
             verifyViewData(conn1, viewName1, prefixForTenant1Data);
             verifyViewData(conn2, viewName2, prefixForTenant2Data);
@@ -128,7 +128,7 @@ public class BaseTenantSpecificViewIndexIT extends SplitSystemCatalogIT {
         return viewName;
     }
     
-    private void createAndVerifyIndex(Connection conn, String viewName, String tableName, Integer saltBuckets, String tenantId, String valuePrefix, boolean localIndex) throws SQLException {
+    private void createAndVerifyIndex(Connection conn, String viewName, String tableName, Integer saltBuckets, String tenantId, String valuePrefix, boolean localIndex, Long expectedIndexIdOffset) throws SQLException {
         String indexName = generateUniqueName();
         if(localIndex){
             conn.createStatement().execute("CREATE LOCAL INDEX " + indexName + " ON " + viewName + "(v2)");
@@ -140,17 +140,18 @@ public class BaseTenantSpecificViewIndexIT extends SplitSystemCatalogIT {
         ResultSet rs = conn.createStatement().executeQuery("EXPLAIN SELECT k1, k2, v2 FROM " + viewName + " WHERE v2='" + valuePrefix + "v2-1'");
         if(localIndex){
             assertEquals(saltBuckets == null ? 
-                    "CLIENT PARALLEL 1-WAY RANGE SCAN OVER " + tableName + " [1,'" + tenantId + "','" + valuePrefix + "v2-1']\n"
+                    "CLIENT PARALLEL 1-WAY RANGE SCAN OVER " + tableName + " [" + Long.toString(1L + expectedIndexIdOffset) + ",'" + tenantId + "','" + valuePrefix + "v2-1']\n"
                             + "    SERVER FILTER BY FIRST KEY ONLY\n"
                             + "CLIENT MERGE SORT" :
-                    "CLIENT PARALLEL 3-WAY RANGE SCAN OVER " + tableName + " [1,'" + tenantId + "','" + valuePrefix + "v2-1']\n"
+                    "CLIENT PARALLEL 3-WAY RANGE SCAN OVER " + tableName + " [" + Long.toString(1L + expectedIndexIdOffset) + ",'" + tenantId + "','" + valuePrefix + "v2-1']\n"
                             + "    SERVER FILTER BY FIRST KEY ONLY\n"
                             + "CLIENT MERGE SORT", QueryUtil.getExplainPlan(rs));
         } else {
             String expected = saltBuckets == null ? 
-                    "CLIENT PARALLEL 1-WAY RANGE SCAN OVER _IDX_" + tableName + " [-9223372036854775808,'" + tenantId + "','" + valuePrefix + "v2-1']\n"
+                    "CLIENT PARALLEL 1-WAY RANGE SCAN OVER _IDX_" + tableName + " [" + Long.toString(Long.MIN_VALUE + expectedIndexIdOffset) + ",'" + tenantId + "','" + valuePrefix + "v2-1']\n"
                             + "    SERVER FILTER BY FIRST KEY ONLY" :
-                    "CLIENT PARALLEL 3-WAY RANGE SCAN OVER _IDX_" + tableName + " [0,-9223372036854775808,'" + tenantId + "','" + valuePrefix + "v2-1'] - ["+(saltBuckets.intValue()-1)+",-9223372036854775808,'" + tenantId + "','" + valuePrefix + "v2-1']\n"
+                    "CLIENT PARALLEL 3-WAY RANGE SCAN OVER _IDX_" + tableName + " [0," + Long.toString(Long.MIN_VALUE + expectedIndexIdOffset) + ",'" + tenantId + "','" + valuePrefix +
+                        "v2-1'] - ["+(saltBuckets.intValue()-1)+"," + Long.toString(Long.MIN_VALUE + expectedIndexIdOffset) + ",'" + tenantId + "','" + valuePrefix + "v2-1']\n"
 
                   + "    SERVER FILTER BY FIRST KEY ONLY\n"
                   + "CLIENT MERGE SORT";
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 a317693..db2c1b0 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
@@ -124,14 +124,16 @@ public class TenantSpecificViewIndexIT extends BaseTenantSpecificViewIndexIT {
         createTableAndValidate(tableName, isNamespaceEnabled);
         String tenantId1 = TENANT1;
         String tenantId2 = TENANT2;
-        createViewAndIndexesWithTenantId(tableName, viewName1, localIndex, tenantId1, isNamespaceEnabled);
-        createViewAndIndexesWithTenantId(tableName, viewName2, localIndex, tenantId2, isNamespaceEnabled);
+        createViewAndIndexesWithTenantId(tableName, viewName1, localIndex, tenantId1, isNamespaceEnabled, 0L);
+        createViewAndIndexesWithTenantId(tableName, viewName2, localIndex, tenantId2, isNamespaceEnabled, 1L);
         
         String sequenceNameA = getViewIndexSequenceName(PNameFactory.newName(tableName), PNameFactory.newName(tenantId2), isNamespaceEnabled);
         String sequenceNameB = getViewIndexSequenceName(PNameFactory.newName(tableName), PNameFactory.newName(tenantId1), isNamespaceEnabled);
+        //IndexIds of the same physical base table should come from the same sequence even if the view indexes
+        //are owned by different tenants.
+        assertEquals(sequenceNameA, sequenceNameB);
         String sequenceSchemaName = getViewIndexSequenceSchemaName(PNameFactory.newName(tableName), isNamespaceEnabled);
-        verifySequenceValue(isNamespaceEnabled? tenantId2 : null, sequenceNameA, sequenceSchemaName, -9223372036854775807L);
-        verifySequenceValue(isNamespaceEnabled? tenantId1 : null, sequenceNameB, sequenceSchemaName, -9223372036854775807L);
+        verifySequenceValue(null, sequenceNameA, sequenceSchemaName, Long.MIN_VALUE + 2L);
 
         Properties props = new Properties();
         props.setProperty(PhoenixRuntime.TENANT_ID_ATTRIB, tenantId2);
@@ -144,12 +146,11 @@ public class TenantSpecificViewIndexIT extends BaseTenantSpecificViewIndexIT {
         }
         DriverManager.getConnection(getUrl()).createStatement().execute("DROP TABLE " + tableName + " CASCADE");
 
-        verifySequenceNotExists(isNamespaceEnabled? tenantId2 : null, sequenceNameA, sequenceSchemaName);
-        verifySequenceNotExists(isNamespaceEnabled? tenantId1 : null, sequenceNameB, sequenceSchemaName);
+        verifySequenceNotExists(null, sequenceNameA, sequenceSchemaName);
     }
 
     private void createViewAndIndexesWithTenantId(String tableName, String viewName, boolean localIndex, String tenantId,
-            boolean isNamespaceMapped) throws Exception {
+            boolean isNamespaceMapped, long indexIdOffset) throws Exception {
         Properties props = new Properties();
         String indexName = "I_"+ generateUniqueName();
         if (tenantId != null) {
@@ -200,14 +201,15 @@ public class TenantSpecificViewIndexIT extends BaseTenantSpecificViewIndexIT {
         rs = conn.createStatement().executeQuery("explain select pk2,col1 from " + viewName + " where col1='f'");
         if (localIndex) {
             assertEquals("CLIENT PARALLEL 1-WAY RANGE SCAN OVER "
-                    + SchemaUtil.getPhysicalTableName(Bytes.toBytes(tableName), isNamespaceMapped) + " [1,'"
+                    + SchemaUtil.getPhysicalTableName(Bytes.toBytes(tableName), isNamespaceMapped) +
+                    " [" + Long.toString(1L + indexIdOffset) + ",'"
                     + tenantId + "','f']\n" + "    SERVER FILTER BY FIRST KEY ONLY\n" + "CLIENT MERGE SORT",
                     QueryUtil.getExplainPlan(rs));
         } else {
             assertEquals("CLIENT PARALLEL 1-WAY RANGE SCAN OVER "
                     + Bytes.toString(MetaDataUtil.getViewIndexPhysicalName(
                         SchemaUtil.getPhysicalTableName(Bytes.toBytes(tableName), isNamespaceMapped).toBytes()))
-                    + " [-9223372036854775808,'" + tenantId + "','f']\n" + "    SERVER FILTER BY FIRST KEY ONLY",
+                    + " [" + Long.toString(Long.MIN_VALUE + indexIdOffset) + ",'" + tenantId + "','f']\n" + "    SERVER FILTER BY FIRST KEY ONLY",
                     QueryUtil.getExplainPlan(rs));
         }
 
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/ViewIndexIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/ViewIndexIT.java
index 5fd023d..4de7034 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/ViewIndexIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/ViewIndexIT.java
@@ -43,10 +43,10 @@ import org.apache.phoenix.end2end.IndexToolIT;
 import org.apache.phoenix.end2end.SplitSystemCatalogIT;
 import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
 import org.apache.phoenix.jdbc.PhoenixStatement;
-import org.apache.phoenix.mapreduce.index.IndexTool;
 import org.apache.phoenix.query.KeyRange;
 import org.apache.phoenix.query.QueryServices;
 import org.apache.phoenix.schema.PNameFactory;
+import org.apache.phoenix.schema.PTable;
 import org.apache.phoenix.schema.TableNotFoundException;
 import org.apache.phoenix.util.MetaDataUtil;
 import org.apache.phoenix.util.PhoenixRuntime;
@@ -54,6 +54,7 @@ import org.apache.phoenix.util.PropertiesUtil;
 import org.apache.phoenix.util.QueryUtil;
 import org.apache.phoenix.util.SchemaUtil;
 import org.apache.phoenix.util.TestUtil;
+import org.junit.Assert;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -95,12 +96,40 @@ public class ViewIndexIT extends SplitSystemCatalogIT {
         conn.createStatement().execute(ddl + ddlOptions);
         conn.close();
     }
+
+    private void createView(Connection conn, String schemaName, String viewName, String baseTableName) throws SQLException {
+        if (isNamespaceMapped) {
+            conn.createStatement().execute("CREATE SCHEMA IF NOT EXISTS " + schemaName);
+        }
+        String fullViewName = SchemaUtil.getTableName(schemaName, viewName);
+        String fullTableName = SchemaUtil.getTableName(schemaName, baseTableName);
+        conn.createStatement().execute("CREATE VIEW " + fullViewName + " AS SELECT * FROM " + fullTableName);
+        conn.commit();
+    }
+
+    private void createViewIndex(Connection conn, String schemaName, String indexName, String viewName,
+                                 String indexColumn) throws SQLException {
+        if (isNamespaceMapped) {
+            conn.createStatement().execute("CREATE SCHEMA IF NOT EXISTS " + schemaName);
+            conn.commit();
+        }
+        String fullViewName = SchemaUtil.getTableName(schemaName, viewName);
+        conn.createStatement().execute("CREATE INDEX " + indexName + " ON " + fullViewName + "(" + indexColumn + ")");
+        conn.commit();
+    }
     
     private Connection getConnection() throws SQLException{
         Properties props = new Properties();
         props.setProperty(QueryServices.IS_NAMESPACE_MAPPING_ENABLED, Boolean.toString(isNamespaceMapped));
         return DriverManager.getConnection(getUrl(),props);
     }
+
+    private Connection getTenantConnection(String tenant) throws SQLException {
+        Properties props = new Properties();
+        props.setProperty(PhoenixRuntime.TENANT_ID_ATTRIB, tenant);
+        props.setProperty(QueryServices.IS_NAMESPACE_MAPPING_ENABLED, Boolean.toString(isNamespaceMapped));
+        return DriverManager.getConnection(getUrl(),props);
+    }
     
     public ViewIndexIT(boolean isNamespaceMapped) {
         this.isNamespaceMapped = isNamespaceMapped;
@@ -517,4 +546,40 @@ public class ViewIndexIT extends SplitSystemCatalogIT {
             }
         }
     }
+
+    @Test
+    public void testGlobalAndTenantViewIndexesHaveDifferentIndexIds() throws Exception {
+        String tableName = "T_" + generateUniqueName();
+        String globalViewName = "V_" + generateUniqueName();
+        String tenantViewName = "TV_" + generateUniqueName();
+        String globalViewIndexName = "GV_" + generateUniqueName();
+        String tenantViewIndexName = "TV_" + generateUniqueName();
+        Connection globalConn = getConnection();
+        Connection tenantConn = getTenantConnection(TENANT1);
+        createBaseTable(SCHEMA1, tableName, true, 0, null);
+        createView(globalConn, SCHEMA1, globalViewName, tableName);
+        createViewIndex(globalConn, SCHEMA1, globalViewIndexName, globalViewName, "v1");
+        createView(tenantConn, SCHEMA1, tenantViewName, tableName);
+        createViewIndex(tenantConn, SCHEMA1, tenantViewIndexName, tenantViewName, "v2");
+
+        PTable globalViewIndexTable = PhoenixRuntime.getTable(globalConn, SCHEMA1 + "." + globalViewIndexName);
+        PTable tenantViewIndexTable = PhoenixRuntime.getTable(tenantConn, SCHEMA1 + "." + tenantViewIndexName);
+        Assert.assertNotNull(globalViewIndexTable);
+        Assert.assertNotNull(tenantViewIndexName);
+        Assert.assertNotEquals(globalViewIndexTable.getViewIndexId(), tenantViewIndexTable.getViewIndexId());
+        globalConn.createStatement().execute("UPSERT INTO " + SchemaUtil.getTableName(SCHEMA1, globalViewName) + " (T_ID, K1, K2) VALUES ('GLOBAL', 'k1', 100)");
+        tenantConn.createStatement().execute("UPSERT INTO " + SchemaUtil.getTableName(SCHEMA1, tenantViewName) + " (T_ID, K1, K2) VALUES ('TENANT', 'k1', 101)");
+
+        assertEquals(1, getRowCountOfView(globalConn, SCHEMA1, globalViewIndexName));
+        assertEquals(1, getRowCountOfView(tenantConn, SCHEMA1, tenantViewName));
+    }
+
+    private int getRowCountOfView(Connection conn, String schemaName, String viewName) throws SQLException {
+      int size = 0;
+      ResultSet rs = conn.createStatement().executeQuery("SELECT COUNT(*) FROM " + SchemaUtil.getTableName(schemaName, viewName));
+      while (rs.next()){
+        size++;
+      }
+      return size;
+    }
 }
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
index 81d741f..f648fe5 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
@@ -621,19 +621,29 @@ public class MetaDataUtil {
     }
 
     public static String getViewIndexSequenceName(PName physicalName, PName tenantId, boolean isNamespaceMapped) {
-        if (!isNamespaceMapped) { return VIEW_INDEX_SEQUENCE_NAME_PREFIX + (tenantId == null ? "" : tenantId); }
+        if (!isNamespaceMapped) { return VIEW_INDEX_SEQUENCE_NAME_PREFIX; }
         return SchemaUtil.getTableNameFromFullName(physicalName.toString()) + VIEW_INDEX_SEQUENCE_NAME_PREFIX;
     }
 
+    /**
+     *
+     * @param tenantId No longer used, but kept in signature for backwards compatibility
+     * @param physicalName Name of physical view index table
+     * @param nSaltBuckets Number of salt buckets
+     * @param isNamespaceMapped Is namespace mapping enabled
+     * @return SequenceKey for the ViewIndexId
+     */
     public static SequenceKey getViewIndexSequenceKey(String tenantId, PName physicalName, int nSaltBuckets,
             boolean isNamespaceMapped) {
-        // Create global sequence of the form: <prefixed base table name><tenant id>
-        // rather than tenant-specific sequence, as it makes it much easier
+        // Create global sequence of the form: <prefixed base table name>.
+        // We can't use a tenant-owned or escaped sequence because of collisions,
+        // with other view indexes that may be global or owned by other tenants that
+        // also use this same physical view index table. It's also much easier
         // to cleanup when the physical table is dropped, as we can delete
         // all global sequences leading with <prefix> + physical name.
         String schemaName = getViewIndexSequenceSchemaName(physicalName, isNamespaceMapped);
-        String tableName = getViewIndexSequenceName(physicalName, PNameFactory.newName(tenantId), isNamespaceMapped);
-        return new SequenceKey(isNamespaceMapped ? tenantId : null, schemaName, tableName, nSaltBuckets);
+        String tableName = getViewIndexSequenceName(physicalName, null, isNamespaceMapped);
+        return new SequenceKey(null, schemaName, tableName, nSaltBuckets);
     }
 
     public static PDataType getViewIndexIdDataType() {