This is an automated email from the ASF dual-hosted git repository. gjacoby pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/master by this push: new 207263f PHOENIX-5132 - View indexes with different owners but of the same base table can be assigned same ViewIndexId 207263f is described below commit 207263f7f0e45ebea02f2f12bec1f7293ad737c5 Author: Geoffrey Jacoby <gjac...@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 4b06d46..3c92a99 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 @@ -624,19 +624,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() {