From 662e9fff412d9263a8f4967719c2b2e221a7dae4 Mon Sep 17 00:00:00 2001
From: Yuya Ebihara <ebyhry@gmail.com>
Date: Fri, 12 Aug 2022 14:38:55 +0900
Subject: [PATCH] Verify database name explicitly in File metastore

testCreateSchemaWithLongName and testRenameSchemaToLongName in
BaseConnectorTest expect throwing an exception when the schema
name length is longer than the limitation, but File metastore
sometimes doesn't throw an exception. This change ensures an
exception by verifying the name explicitly.
---
 .../hive/metastore/file/FileHiveMetastore.java       | 12 ++++++++++++
 .../io/trino/plugin/hive/BaseHiveConnectorTest.java  |  4 ++--
 .../plugin/iceberg/BaseIcebergConnectorTest.java     |  4 ++--
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java
index 42a36909dcfc..c9062a4f57be 100644
--- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java
+++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/file/FileHiveMetastore.java
@@ -141,6 +141,9 @@ public class FileHiveMetastore
     // todo there should be a way to manage the admins list
     private static final Set<String> ADMIN_USERS = ImmutableSet.of("admin", "hive", "hdfs");
 
+    // 128 is equals to the max database name length of Thrift Hive metastore
+    private static final int MAX_DATABASE_NAME_LENGTH = 128;
+
     private final String currentVersion;
     private final VersionCompatibility versionCompatibility;
     private final HdfsEnvironment hdfsEnvironment;
@@ -197,6 +200,7 @@ public synchronized void createDatabase(Database database)
             throw new TrinoException(HIVE_METASTORE_ERROR, "Database cannot be created with a location set");
         }
 
+        verifyDatabaseNameLength(database.getDatabaseName());
         verifyDatabaseNotExists(database.getDatabaseName());
 
         Path databaseMetadataDirectory = getDatabaseMetadataDirectory(database.getDatabaseName());
@@ -234,6 +238,7 @@ public synchronized void renameDatabase(String databaseName, String newDatabaseN
         requireNonNull(databaseName, "databaseName is null");
         requireNonNull(newDatabaseName, "newDatabaseName is null");
 
+        verifyDatabaseNameLength(newDatabaseName);
         getRequiredDatabase(databaseName);
         verifyDatabaseNotExists(newDatabaseName);
 
@@ -283,6 +288,13 @@ private Database getRequiredDatabase(String databaseName)
                 .orElseThrow(() -> new SchemaNotFoundException(databaseName));
     }
 
+    private void verifyDatabaseNameLength(String databaseName)
+    {
+        if (databaseName.length() > MAX_DATABASE_NAME_LENGTH) {
+            throw new TrinoException(NOT_SUPPORTED, format("Schema name must be shorter than or equal to '%s' characters but got '%s'", MAX_DATABASE_NAME_LENGTH, databaseName.length()));
+        }
+    }
+
     private void verifyDatabaseNotExists(String databaseName)
     {
         if (getDatabase(databaseName).isPresent()) {
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 bab7898a03b4..80b06434df7a 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
@@ -8486,13 +8486,13 @@ protected TestTable createTableWithDefaultColumns()
     protected OptionalInt maxSchemaNameLength()
     {
         // This value depends on metastore type
-        return OptionalInt.of(255 - "..".length() - ".trinoSchema.crc".length());
+        return OptionalInt.of(128);
     }
 
     @Override
     protected void verifySchemaNameLengthFailurePermissible(Throwable e)
     {
-        assertThat(e).hasMessageMatching("Could not (write|rename) database schema");
+        assertThat(e).hasMessageMatching("Schema name must be shorter than or equal to '128' characters but got '129'");
     }
 
     @Override
diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java
index 9c8e83ee60df..02341b99f401 100644
--- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java
+++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java
@@ -5611,13 +5611,13 @@ public Object[][] targetAndSourceWithDifferentPartitioning()
     protected OptionalInt maxSchemaNameLength()
     {
         // This value depends on metastore type
-        return OptionalInt.of(255 - "..".length() - ".trinoSchema.crc".length());
+        return OptionalInt.of(128);
     }
 
     @Override
     protected void verifySchemaNameLengthFailurePermissible(Throwable e)
     {
-        assertThat(e).hasMessageMatching("Could not (write|rename) database schema");
+        assertThat(e).hasMessageMatching("Schema name must be shorter than or equal to '128' characters but got '129'");
     }
 
     @Override