You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by vj...@apache.org on 2021/07/10 16:54:58 UTC
[phoenix] branch 5.1 updated: PHOENIX-6509 PHOENIX-4424 Allow users
to create DEFAULT and HBASE Schema (Uppercase Schema Names) (#1263)
This is an automated email from the ASF dual-hosted git repository.
vjasani pushed a commit to branch 5.1
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/5.1 by this push:
new 2997463 PHOENIX-6509 PHOENIX-4424 Allow users to create DEFAULT and HBASE Schema (Uppercase Schema Names) (#1263)
2997463 is described below
commit 2997463ee5eb430bc5dfecde337b9ff85bcf6230
Author: Rushabh Shah <sh...@gmail.com>
AuthorDate: Sat Jul 10 12:52:35 2021 -0400
PHOENIX-6509 PHOENIX-4424 Allow users to create DEFAULT and HBASE Schema (Uppercase Schema Names) (#1263)
Signed-off-by: Viraj Jasani <vj...@apache.org>
---
.../apache/phoenix/end2end/BasePermissionsIT.java | 8 +--
.../org/apache/phoenix/end2end/CreateSchemaIT.java | 65 ++++++++++++++++------
phoenix-core/src/main/antlr3/PhoenixSQL.g | 2 +-
.../phoenix/parse/CreateSchemaStatement.java | 2 +-
.../org/apache/phoenix/query/QueryConstants.java | 1 -
.../org/apache/phoenix/schema/MetaDataClient.java | 10 ++--
.../java/org/apache/phoenix/util/SchemaUtil.java | 5 +-
.../org/apache/phoenix/parse/QueryParserTest.java | 12 ++++
8 files changed, 74 insertions(+), 31 deletions(-)
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
index 6e71dcc..7ee5aea 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
@@ -1062,7 +1062,7 @@ public abstract class BasePermissionsIT extends BaseTest {
verifyAllowed(createSchema(schemaName), superUser1);
verifyAllowed(grantPermissions("C", regularUser1, schemaName, true), superUser1);
} else {
- verifyAllowed(grantPermissions("C", regularUser1, surroundWithDoubleQuotes(QueryConstants.HBASE_DEFAULT_SCHEMA_NAME), true), superUser1);
+ verifyAllowed(grantPermissions("C", regularUser1, surroundWithDoubleQuotes(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
@@ -1123,7 +1123,7 @@ public abstract class BasePermissionsIT extends BaseTest {
verifyAllowed(createSchema(schemaName), superUser1);
verifyAllowed(grantPermissions("C", regularUser1, schemaName, true), superUser1);
} else {
- verifyAllowed(grantPermissions("C", regularUser1, surroundWithDoubleQuotes(QueryConstants.HBASE_DEFAULT_SCHEMA_NAME), true), superUser1);
+ verifyAllowed(grantPermissions("C", regularUser1, surroundWithDoubleQuotes(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
@@ -1207,7 +1207,7 @@ public abstract class BasePermissionsIT extends BaseTest {
verifyAllowed(createSchema(schemaName), superUser1);
verifyAllowed(grantPermissions("C", regularUser1, schemaName, true), superUser1);
} else {
- verifyAllowed(grantPermissions("C", regularUser1, surroundWithDoubleQuotes(QueryConstants.HBASE_DEFAULT_SCHEMA_NAME), true), superUser1);
+ verifyAllowed(grantPermissions("C", regularUser1, surroundWithDoubleQuotes(SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE), true), superUser1);
}
// Create MultiTenant Table (View Index Table should be automatically created)
@@ -1253,7 +1253,7 @@ public abstract class BasePermissionsIT extends BaseTest {
verifyAllowed(grantPermissions("RX", regularUser1, schemaName, true), superUser1);
} else {
verifyAllowed(createTable(fullTableName), superUser1);
- verifyAllowed(grantPermissions("RX", regularUser1, surroundWithDoubleQuotes(QueryConstants.HBASE_DEFAULT_SCHEMA_NAME), true), superUser1);
+ verifyAllowed(grantPermissions("RX", regularUser1, surroundWithDoubleQuotes(SchemaUtil.SCHEMA_FOR_DEFAULT_NAMESPACE), true), superUser1);
}
verifyAllowed(createView(view1TableName, fullTableName), regularUser1);
}
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 a05d702..ffb82dd 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
@@ -27,6 +27,7 @@ import java.sql.SQLException;
import java.util.Properties;
import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.HBaseAdmin;
import org.apache.phoenix.exception.SQLExceptionCode;
import org.apache.phoenix.jdbc.PhoenixConnection;
import org.apache.phoenix.query.QueryServices;
@@ -43,31 +44,59 @@ 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);
Admin 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);
+ Admin 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();
}
-}
+}
\ No newline at end of file
diff --git a/phoenix-core/src/main/antlr3/PhoenixSQL.g b/phoenix-core/src/main/antlr3/PhoenixSQL.g
index 2c23114..7cdce23 100644
--- a/phoenix-core/src/main/antlr3/PhoenixSQL.g
+++ b/phoenix-core/src/main/antlr3/PhoenixSQL.g
@@ -462,7 +462,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); }
;
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;
}
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 f2638cc..32fda92 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
@@ -182,7 +182,6 @@ public interface QueryConstants {
enum JoinType {INNER, LEFT_OUTER}
String SYSTEM_SCHEMA_NAME = "SYSTEM";
byte[] SYSTEM_SCHEMA_NAME_BYTES = Bytes.toBytes(SYSTEM_SCHEMA_NAME);
- String HBASE_DEFAULT_SCHEMA_NAME = "default";
String OFFSET_ROW_KEY = "_OFFSET_";
byte[] OFFSET_ROW_KEY_BYTES = Bytes.toBytes(OFFSET_ROW_KEY);
ImmutableBytesPtr OFFSET_ROW_KEY_PTR = new ImmutableBytesPtr(OFFSET_ROW_KEY_BYTES);
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 9f22ed4..27d5d5f 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
@@ -4947,8 +4947,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;
@@ -4982,8 +4984,8 @@ 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)
+ if (SchemaUtil.NOT_ALLOWED_SCHEMA_LIST.contains(schemaName)) {
+ throw new SQLExceptionInfo.Builder(SQLExceptionCode.SCHEMA_NOT_ALLOWED)
.setSchemaName(schemaName).build().buildException(); }
}
@@ -5496,7 +5498,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);
}
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 dd4b6c3..445456d 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
@@ -141,8 +141,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);
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 5547ef3..3d06873 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
@@ -899,4 +899,16 @@ public class QueryParserTest {
parseQueryThatShouldFail("show tables in 'foo'");
parseQueryThatShouldFail("show tables like foo");
}
+
+ @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);
+ }
}