Repository: phoenix Updated Branches: refs/heads/4.x-HBase-1.2 0b1f22749 -> f6f7b4678
PHOENIX-4424 Allow users to create "DEFAULT" and "HBASE" Schema (Uppercase Schema Names) Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/f6f7b467 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/f6f7b467 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/f6f7b467 Branch: refs/heads/4.x-HBase-1.2 Commit: f6f7b467883e6089ab564b94d32042340186472d Parents: a3db335 Author: Karan Mehta <karanmeht...@gmail.com> Authored: Sat Nov 4 03:13:53 2017 +0000 Committer: James Taylor <jtay...@salesforce.com> Committed: Sat Dec 16 16:42:54 2017 -0800 ---------------------------------------------------------------------- .../phoenix/end2end/ChangePermissionsIT.java | 5 +- .../apache/phoenix/end2end/CreateSchemaIT.java | 64 ++++++++++++++------ phoenix-core/src/main/antlr3/PhoenixSQL.g | 2 +- .../phoenix/parse/CreateSchemaStatement.java | 2 +- .../apache/phoenix/query/QueryConstants.java | 1 - .../apache/phoenix/schema/MetaDataClient.java | 8 ++- .../org/apache/phoenix/util/SchemaUtil.java | 5 +- .../apache/phoenix/parse/QueryParserTest.java | 13 ++++ 8 files changed, 73 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/f6f7b467/phoenix-core/src/it/java/org/apache/phoenix/end2end/ChangePermissionsIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ChangePermissionsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ChangePermissionsIT.java index c023440..2bf7fe1 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ChangePermissionsIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ChangePermissionsIT.java @@ -23,6 +23,7 @@ import org.apache.hadoop.hbase.security.User; import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData; import org.apache.phoenix.query.QueryConstants; import org.apache.phoenix.schema.TableNotFoundException; +import org.apache.phoenix.util.SchemaUtil; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -144,7 +145,7 @@ public class ChangePermissionsIT extends BasePermissionsIT { verifyAllowed(createSchema(SCHEMA_NAME), superUser1); verifyAllowed(grantPermissions("C", regularUser1, SCHEMA_NAME, true), superUser1); } else { - verifyAllowed(grantPermissions("C", regularUser1, "\"" + QueryConstants.HBASE_DEFAULT_SCHEMA_NAME + "\"", true), superUser1); + verifyAllowed(grantPermissions("C", regularUser1, "\"" + SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE + "\"", true), superUser1); } // Create new table. Create indexes, views and view indexes on top of it. Verify the contents by querying it @@ -235,7 +236,7 @@ public class ChangePermissionsIT extends BasePermissionsIT { verifyAllowed(createSchema(SCHEMA_NAME), superUser1); verifyAllowed(grantPermissions("C", regularUser1, SCHEMA_NAME, true), superUser1); } else { - verifyAllowed(grantPermissions("C", regularUser1, "\"" + QueryConstants.HBASE_DEFAULT_SCHEMA_NAME + "\"", true), superUser1); + verifyAllowed(grantPermissions("C", regularUser1, "\"" + SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE + "\"", true), superUser1); } // Create MultiTenant Table (View Index Table should be automatically created) http://git-wip-us.apache.org/repos/asf/phoenix/blob/f6f7b467/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateSchemaIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateSchemaIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateSchemaIT.java index fe09dcd..8002dc1 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateSchemaIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateSchemaIT.java @@ -43,31 +43,61 @@ public class CreateSchemaIT extends ParallelStatsDisabledIT { Properties props = PropertiesUtil.deepCopy(TestUtil.TEST_PROPERTIES); props.setProperty(QueryServices.IS_NAMESPACE_MAPPING_ENABLED, Boolean.toString(true)); String schemaName = generateUniqueName(); - String ddl = "CREATE SCHEMA " + schemaName; + String schemaName1 = schemaName.toLowerCase(); + String schemaName2 = schemaName.toLowerCase(); + // Create unique name schema and verify that it exists + // ddl1 should create lowercase schemaName since it is passed in with double-quotes + // ddl2 should create uppercase schemaName since Phoenix upper-cases identifiers without quotes + // Both the statements should succeed + String ddl1 = "CREATE SCHEMA \"" + schemaName1 + "\""; + String ddl2 = "CREATE SCHEMA " + schemaName2; try (Connection conn = DriverManager.getConnection(getUrl(), props); HBaseAdmin admin = conn.unwrap(PhoenixConnection.class).getQueryServices().getAdmin();) { - conn.createStatement().execute(ddl); - assertNotNull(admin.getNamespaceDescriptor(schemaName)); + conn.createStatement().execute(ddl1); + assertNotNull(admin.getNamespaceDescriptor(schemaName1)); + conn.createStatement().execute(ddl2); + assertNotNull(admin.getNamespaceDescriptor(schemaName2.toUpperCase())); } + // Try creating it again and verify that it throws SchemaAlreadyExistsException try (Connection conn = DriverManager.getConnection(getUrl(), props)) { - conn.createStatement().execute(ddl); + conn.createStatement().execute(ddl1); fail(); } catch (SchemaAlreadyExistsException e) { // expected } - Connection conn = DriverManager.getConnection(getUrl(), props); - try { - conn.createStatement().execute("CREATE SCHEMA " + SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE); - fail(); - } catch (SQLException e) { - assertEquals(SQLExceptionCode.SCHEMA_NOT_ALLOWED.getErrorCode(), e.getErrorCode()); - } - try { - conn.createStatement().execute("CREATE SCHEMA " + SchemaUtil.HBASE_NAMESPACE); - fail(); - } catch (SQLException e) { - assertEquals(SQLExceptionCode.SCHEMA_NOT_ALLOWED.getErrorCode(), e.getErrorCode()); + + // See PHOENIX-4424 + // Create schema DEFAULT and HBASE (Should allow since they are upper-cased) and verify that it exists + // Create schema default and hbase and it should fail + try (Connection conn = DriverManager.getConnection(getUrl(), props); + HBaseAdmin admin = conn.unwrap(PhoenixConnection.class).getQueryServices().getAdmin();) { + + // default is a SQL keyword, hence it should always be passed in double-quotes + try { + conn.createStatement().execute("CREATE SCHEMA \"" + + SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE + "\""); + fail(); + } catch (SQLException e) { + assertEquals(SQLExceptionCode.SCHEMA_NOT_ALLOWED.getErrorCode(), e.getErrorCode()); + } + + try { + conn.createStatement().execute("CREATE SCHEMA \"" + + SchemaUtil.HBASE_NAMESPACE + "\""); + fail(); + } catch (SQLException e) { + assertEquals(SQLExceptionCode.SCHEMA_NOT_ALLOWED.getErrorCode(), e.getErrorCode()); + } + + // default is a SQL keyword, hence it should always be passed in double-quotes + conn.createStatement().execute("CREATE SCHEMA \"" + + SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE.toUpperCase() + "\""); + conn.createStatement().execute("CREATE SCHEMA \"" + + SchemaUtil.HBASE_NAMESPACE.toUpperCase() + "\""); + + assertNotNull(admin.getNamespaceDescriptor(SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE.toUpperCase())); + assertNotNull(admin.getNamespaceDescriptor(SchemaUtil.HBASE_NAMESPACE.toUpperCase())); + } - conn.close(); } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/f6f7b467/phoenix-core/src/main/antlr3/PhoenixSQL.g ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/antlr3/PhoenixSQL.g b/phoenix-core/src/main/antlr3/PhoenixSQL.g index ccf654b..87153cd 100644 --- a/phoenix-core/src/main/antlr3/PhoenixSQL.g +++ b/phoenix-core/src/main/antlr3/PhoenixSQL.g @@ -459,7 +459,7 @@ create_table_node returns [CreateTableStatement ret] // Parse a create schema statement. create_schema_node returns [CreateSchemaStatement ret] - : CREATE SCHEMA (IF NOT ex=EXISTS)? (DEFAULT | s=identifier) + : CREATE SCHEMA (IF NOT ex=EXISTS)? s=identifier {ret = factory.createSchema(s, ex!=null); } ; http://git-wip-us.apache.org/repos/asf/phoenix/blob/f6f7b467/phoenix-core/src/main/java/org/apache/phoenix/parse/CreateSchemaStatement.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/parse/CreateSchemaStatement.java b/phoenix-core/src/main/java/org/apache/phoenix/parse/CreateSchemaStatement.java index 7c255cb..f5ab3f6 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/parse/CreateSchemaStatement.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/parse/CreateSchemaStatement.java @@ -24,7 +24,7 @@ public class CreateSchemaStatement extends MutableStatement { private final boolean ifNotExists; public CreateSchemaStatement(String schemaName,boolean ifNotExists) { - this.schemaName = null == schemaName ? SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE : schemaName; + this.schemaName = schemaName; this.ifNotExists = ifNotExists; } http://git-wip-us.apache.org/repos/asf/phoenix/blob/f6f7b467/phoenix-core/src/main/java/org/apache/phoenix/query/QueryConstants.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/QueryConstants.java b/phoenix-core/src/main/java/org/apache/phoenix/query/QueryConstants.java index 851ba9a..7607388 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/query/QueryConstants.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/query/QueryConstants.java @@ -149,7 +149,6 @@ public interface QueryConstants { public enum JoinType {INNER, LEFT_OUTER} public final static String SYSTEM_SCHEMA_NAME = "SYSTEM"; public final static byte[] SYSTEM_SCHEMA_NAME_BYTES = Bytes.toBytes(SYSTEM_SCHEMA_NAME); - public final static String HBASE_DEFAULT_SCHEMA_NAME = "default"; public final static String PHOENIX_METADATA = "table"; public final static String OFFSET_ROW_KEY = "_OFFSET_"; public final static byte[] OFFSET_ROW_KEY_BYTES = Bytes.toBytes(OFFSET_ROW_KEY); http://git-wip-us.apache.org/repos/asf/phoenix/blob/f6f7b467/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 fc2e288..5ec5ac3 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 @@ -3980,8 +3980,10 @@ public class MetaDataClient { SQLExceptionCode.CREATE_SCHEMA_NOT_ALLOWED).setSchemaName(create.getSchemaName()) .build().buildException(); } boolean isIfNotExists = create.isIfNotExists(); - validateSchema(create.getSchemaName()); PSchema schema = new PSchema(create.getSchemaName()); + // Use SchemaName from PSchema object to get the normalized SchemaName + // See PHOENIX-4424 for details + validateSchema(schema.getSchemaName()); connection.setAutoCommit(false); List<Mutation> schemaMutations; @@ -4016,7 +4018,7 @@ public class MetaDataClient { private void validateSchema(String schemaName) throws SQLException { if (SchemaUtil.NOT_ALLOWED_SCHEMA_LIST.contains( - schemaName.toUpperCase())) { throw new SQLExceptionInfo.Builder(SQLExceptionCode.SCHEMA_NOT_ALLOWED) + schemaName)) { throw new SQLExceptionInfo.Builder(SQLExceptionCode.SCHEMA_NOT_ALLOWED) .setSchemaName(schemaName).build().buildException(); } } @@ -4082,7 +4084,7 @@ public class MetaDataClient { if (changePermsStatement.getSchemaName() != null) { // SYSTEM.CATALOG doesn't have any entry for "default" HBase namespace, hence we will bypass the check - if(!changePermsStatement.getSchemaName().equals(QueryConstants.HBASE_DEFAULT_SCHEMA_NAME)) { + if(!changePermsStatement.getSchemaName().equals(SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE)) { FromCompiler.getResolverForSchema(changePermsStatement.getSchemaName(), connection); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/f6f7b467/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java index 5b5c3a5..42c2dcb 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java @@ -133,8 +133,9 @@ public class SchemaUtil { }; public static final RowKeySchema VAR_BINARY_SCHEMA = new RowKeySchemaBuilder(1).addField(VAR_BINARY_DATUM, false, SortOrder.getDefault()).build(); - public static final String SCHEMA_FOR_DEFAULT_NAMESPACE = "DEFAULT"; - public static final String HBASE_NAMESPACE = "HBASE"; + // See PHOENIX-4424 + public static final String SCHEMA_FOR_DEFAULT_NAMESPACE = "default"; + public static final String HBASE_NAMESPACE = "hbase"; public static final List<String> NOT_ALLOWED_SCHEMA_LIST = Arrays.asList(SCHEMA_FOR_DEFAULT_NAMESPACE, HBASE_NAMESPACE); http://git-wip-us.apache.org/repos/asf/phoenix/blob/f6f7b467/phoenix-core/src/test/java/org/apache/phoenix/parse/QueryParserTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/parse/QueryParserTest.java b/phoenix-core/src/test/java/org/apache/phoenix/parse/QueryParserTest.java index 25f59c0..24653c6 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/parse/QueryParserTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/parse/QueryParserTest.java @@ -69,6 +69,19 @@ public class QueryParserTest { } @Test + public void testCreateSchema() throws Exception { + + String sql0 = "create schema \"schema1\""; + parseQuery(sql0); + String sql1 = "create schema schema1"; + parseQuery(sql1); + String sql2 = "create schema \"default\""; + parseQuery(sql2); + String sql3 = "create schema \"DEFAULT\""; + parseQuery(sql3); + } + + @Test public void testParseGrantQuery() throws Exception { String sql0 = "GRANT 'RX' ON SYSTEM.\"SEQUENCE\" TO 'user'";