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/6e5a8f76
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/6e5a8f76
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/6e5a8f76

Branch: refs/heads/4.x-cdh5.11.2
Commit: 6e5a8f76e0171cb0a2eecdaf84267c7c62a54bad
Parents: 2c4ca69
Author: Karan Mehta <karanmeht...@gmail.com>
Authored: Sat Nov 4 03:13:53 2017 +0000
Committer: Pedro Boado <pbo...@apache.org>
Committed: Wed Jan 31 22:24:48 2018 +0000

----------------------------------------------------------------------
 .../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/6e5a8f76/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/6e5a8f76/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/6e5a8f76/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/6e5a8f76/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/6e5a8f76/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/6e5a8f76/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 0127eeb..403cbfe 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/6e5a8f76/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/6e5a8f76/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'";

Reply via email to