From b2f71c2b30a4ca8ef1b38baca593b3867352b405 Mon Sep 17 00:00:00 2001 From: Vikash Kumar Date: Wed, 15 Feb 2023 15:36:15 +0530 Subject: [PATCH] Fix create schema in glue catalog when username contains uppercase char TrinoPrincipal sets the owner name in lowercase. --- .../plugin/hive/BaseHiveConnectorTest.java | 9 +++++++++ .../hive/TestHiveConnectorSmokeTest.java | 9 +++++++++ .../iceberg/catalog/glue/TrinoGlueCatalog.java | 2 +- ...hDisabledInferSchemaConnectorSmokeTest.java | 7 +++++++ .../plugin/kudu/TestKuduConnectorTest.java | 7 +++++++ .../trino/testing/BaseConnectorSmokeTest.java | 18 ++++++++++++++++++ .../io/trino/testing/BaseConnectorTest.java | 17 +++++++++++++++++ 7 files changed, 68 insertions(+), 1 deletion(-) diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java index 967ee918fd9d..c52cb408d2ea 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java @@ -698,6 +698,15 @@ public void testSchemaAuthorizationForRole() assertUpdate(admin, "DROP ROLE authorized_users IN hive"); } + @Override + public void testCreateSchemaWithNonLowercaseOwnerName() + { + // Override because HivePrincipal's username is case-sensitive unlike TrinoPrincipal + assertThatThrownBy(super::testCreateSchemaWithNonLowercaseOwnerName) + .hasMessageContaining("Access Denied: Cannot create schema") + .hasStackTraceContaining("CREATE SCHEMA"); + } + @Test public void testCreateSchemaWithAuthorizationForUser() { diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveConnectorSmokeTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveConnectorSmokeTest.java index 82ca85c9f21b..bd06ac1035f9 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveConnectorSmokeTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveConnectorSmokeTest.java @@ -96,4 +96,13 @@ public void testShowCreateTable() " format = 'ORC'\n" + ")"); } + + @Override + public void testCreateSchemaWithNonLowercaseOwnerName() + { + // Override because HivePrincipal's username is case-sensitive unlike TrinoPrincipal + assertThatThrownBy(super::testCreateSchemaWithNonLowercaseOwnerName) + .hasMessageContaining("Access Denied: Cannot create schema") + .hasStackTraceContaining("CREATE SCHEMA"); + } } diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java index dee3ce1cdcba..903ce12f6364 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java @@ -253,7 +253,7 @@ public Optional getNamespacePrincipal(ConnectorSession session, public void createNamespace(ConnectorSession session, String namespace, Map properties, TrinoPrincipal owner) { checkArgument(owner.getType() == PrincipalType.USER, "Owner type must be USER"); - checkArgument(owner.getName().equals(session.getUser()), "Explicit schema owner is not supported"); + checkArgument(owner.getName().equals(session.getUser().toLowerCase(ENGLISH)), "Explicit schema owner is not supported"); try { stats.getCreateDatabase().call(() -> diff --git a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/BaseKuduWithDisabledInferSchemaConnectorSmokeTest.java b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/BaseKuduWithDisabledInferSchemaConnectorSmokeTest.java index 2b314b473352..713959a28f5e 100644 --- a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/BaseKuduWithDisabledInferSchemaConnectorSmokeTest.java +++ b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/BaseKuduWithDisabledInferSchemaConnectorSmokeTest.java @@ -57,6 +57,13 @@ public void testCreateSchema() .hasMessage("Creating schema in Kudu connector not allowed if schema emulation is disabled."); } + @Override + public void testCreateSchemaWithNonLowercaseOwnerName() + { + assertThatThrownBy(super::testCreateSchemaWithNonLowercaseOwnerName) + .hasMessage("Creating schema in Kudu connector not allowed if schema emulation is disabled."); + } + @Test @Override public void testRenameTableAcrossSchemas() diff --git a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduConnectorTest.java b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduConnectorTest.java index 68870bb00bc5..c7195b2d6a2c 100644 --- a/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduConnectorTest.java +++ b/plugin/trino-kudu/src/test/java/io/trino/plugin/kudu/TestKuduConnectorTest.java @@ -139,6 +139,13 @@ public void testCreateSchema() .hasMessage("Creating schema in Kudu connector not allowed if schema emulation is disabled."); } + @Override + public void testCreateSchemaWithNonLowercaseOwnerName() + { + assertThatThrownBy(super::testCreateSchemaWithNonLowercaseOwnerName) + .hasMessage("Creating schema in Kudu connector not allowed if schema emulation is disabled."); + } + @Override public void testCreateSchemaWithLongName() { diff --git a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorSmokeTest.java b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorSmokeTest.java index a30ef33c5e8b..5239377074c1 100644 --- a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorSmokeTest.java +++ b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorSmokeTest.java @@ -14,6 +14,8 @@ package io.trino.testing; import com.google.common.collect.ImmutableList; +import io.trino.Session; +import io.trino.spi.security.Identity; import io.trino.testing.sql.TestTable; import io.trino.tpch.TpchTable; import org.testng.SkipException; @@ -315,6 +317,22 @@ public void testCreateSchema() assertUpdate("DROP SCHEMA " + schemaName); } + @Test + public void testCreateSchemaWithNonLowercaseOwnerName() + { + skipTestUnless(hasBehavior(SUPPORTS_CREATE_SCHEMA)); + + Session newSession = Session.builder(getSession()) + .setIdentity(Identity.ofUser("ADMIN")) + .build(); + String schemaName = "test_schema_create_uppercase_owner_name_" + randomNameSuffix(); + assertUpdate(newSession, createSchemaSql(schemaName)); + assertThat(query(newSession, "SHOW SCHEMAS")) + .skippingTypesCheck() + .containsAll(format("VALUES '%s'", schemaName)); + assertUpdate(newSession, "DROP SCHEMA " + schemaName); + } + @Test public void testRenameSchema() { diff --git a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java index 8192b0a098f5..19fa4393d0da 100644 --- a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java +++ b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java @@ -34,6 +34,7 @@ import io.trino.server.BasicQueryInfo; import io.trino.spi.connector.ConnectorSession; import io.trino.spi.connector.MaterializedViewFreshness; +import io.trino.spi.security.Identity; import io.trino.sql.planner.OptimizerConfig.JoinDistributionType; import io.trino.sql.planner.Plan; import io.trino.sql.planner.assertions.PlanMatchPattern; @@ -2881,6 +2882,22 @@ public void testCreateTable() assertFalse(getQueryRunner().tableExists(getSession(), tableNameLike)); } + @Test + public void testCreateSchemaWithNonLowercaseOwnerName() + { + skipTestUnless(hasBehavior(SUPPORTS_CREATE_SCHEMA)); + + Session newSession = Session.builder(getSession()) + .setIdentity(Identity.ofUser("ADMIN")) + .build(); + String schemaName = "test_schema_create_uppercase_owner_name_" + randomNameSuffix(); + assertUpdate(newSession, createSchemaSql(schemaName)); + assertThat(query(newSession, "SHOW SCHEMAS")) + .skippingTypesCheck() + .containsAll(format("VALUES '%s'", schemaName)); + assertUpdate(newSession, "DROP SCHEMA " + schemaName); + } + @Test public void testCreateSchemaWithLongName() {