add test

sunxiaojian · sunxiaojian · commit c09eeee33747 · 2024-02-06T19:21:53.000+08:00
diff --git a/paimon-core/pom.xml b/paimon-core/pom.xml
@@ -189,6 +189,13 @@ under the License.
             <version>3.6.1</version>
         </dependency>
 
+        <dependency>
+            <groupId>org.xerial</groupId>
+            <artifactId>sqlite-jdbc</artifactId>
+            <version>3.44.0.0</version>
+            <scope>test</scope>
+        </dependency>
+
     </dependencies>
 
     <build>
diff --git a/paimon-core/src/main/java/org/apache/paimon/jdbc/JdbcCatalog.java b/paimon-core/src/main/java/org/apache/paimon/jdbc/JdbcCatalog.java
@@ -25,7 +25,6 @@
 import org.apache.paimon.fs.Path;
 import org.apache.paimon.operation.Lock;
 import org.apache.paimon.options.CatalogOptions;
-import org.apache.paimon.options.Options;
 import org.apache.paimon.schema.Schema;
 import org.apache.paimon.schema.SchemaChange;
 import org.apache.paimon.schema.SchemaManager;
@@ -46,6 +45,7 @@
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.AbstractMap;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -70,19 +70,23 @@ public class JdbcCatalog extends AbstractCatalog {
     private Map<String, String> configuration;
     private final String warehouse;
 
-    protected JdbcCatalog(FileIO fileIO, String catalogName, Options options, String warehouse) {
+    protected JdbcCatalog(
+            FileIO fileIO, String catalogName, Map<String, String> config, String warehouse) {
         super(fileIO);
         if (!StringUtils.isBlank(catalogName)) {
             this.catalogName = catalogName;
         }
-        this.configuration = options.toMap();
+        this.configuration = config;
         this.warehouse = warehouse;
         Preconditions.checkNotNull(configuration, "Invalid catalog properties: null");
         this.connections =
                 new JdbcClientPool(
-                        options.get(CatalogOptions.CLIENT_POOL_SIZE),
-                        options.get(CatalogOptions.URI),
-                        this.configuration);
+                        Integer.parseInt(
+                                config.getOrDefault(
+                                        CatalogOptions.CLIENT_POOL_SIZE.key(),
+                                        CatalogOptions.CLIENT_POOL_SIZE.defaultValue().toString())),
+                        configuration.get(CatalogOptions.URI.key()),
+                        configuration);
         try {
             initializeCatalogTablesIfNeed();
         } catch (SQLException e) {
@@ -97,7 +101,7 @@ private void initializeCatalogTablesIfNeed() throws SQLException, InterruptedExc
         boolean initializeCatalogTables =
                 Boolean.parseBoolean(
                         configuration.getOrDefault(INITIALIZE_CATALOG_TABLES.key(), "false"));
-        String uri = configuration.get(CatalogOptions.URI);
+        String uri = configuration.get(CatalogOptions.URI.key());
         Preconditions.checkNotNull(uri, "JDBC connection URI is required");
         if (initializeCatalogTables) {
             // Check and create catalog table.
@@ -133,6 +137,24 @@ private void initializeCatalogTablesIfNeed() throws SQLException, InterruptedExc
                         return conn.prepareStatement(JdbcUtils.CREATE_DATABASE_PROPERTIES_TABLE)
                                 .execute();
                     });
+
+            // Check and create distributed lock table.
+            connections.run(
+                    conn -> {
+                        DatabaseMetaData dbMeta = conn.getMetaData();
+                        ResultSet tableExists =
+                                dbMeta.getTables(
+                                        null /* catalog name */,
+                                        null /* schemaPattern */,
+                                        JdbcUtils
+                                                .DISTRIBUTED_LOCKS_TABLE_NAME /* tableNamePattern */,
+                                        null /* types */);
+                        if (tableExists.next()) {
+                            return true;
+                        }
+                        return conn.prepareStatement(JdbcUtils.CREATE_DISTRIBUTED_LOCK_TABLE_SQL)
+                                .execute();
+                    });
         }
     }
 
@@ -149,6 +171,12 @@ public List<String> listDatabases() {
                         row -> row.getString(JdbcUtils.TABLE_DATABASE),
                         JdbcUtils.LIST_ALL_TABLE_DATABASES_SQL,
                         catalogName));
+
+        namespaces.addAll(
+                fetch(
+                        row -> row.getString(JdbcUtils.DATABASE_NAME),
+                        JdbcUtils.LIST_ALL_PROPERTY_DATABASES_SQL,
+                        catalogName));
         return namespaces;
     }
 
@@ -177,15 +205,10 @@ protected void createDatabaseImpl(String name, Map<String, String> properties) {
             throw new RuntimeException(String.format("Database already exists: %s", name));
         }
 
-        Map<String, String> createProps;
-        if (properties == null || properties.isEmpty()) {
-            createProps = ImmutableMap.of(DATABASE_EXISTS_PROPERTY, "true");
-        } else {
-            createProps =
-                    ImmutableMap.<String, String>builder()
-                            .putAll(properties)
-                            .put(DATABASE_EXISTS_PROPERTY, "true")
-                            .build();
+        Map<String, String> createProps = new HashMap<>();
+        createProps.put(DATABASE_EXISTS_PROPERTY, "true");
+        if (properties != null && !properties.isEmpty()) {
+            createProps.putAll(properties);
         }
 
         if (!createProps.containsKey(DB_LOCATION_PROP)) {
@@ -197,21 +220,9 @@ protected void createDatabaseImpl(String name, Map<String, String> properties) {
 
     @Override
     protected void dropDatabaseImpl(String name) {
-        if (!databaseExists(name)) {
-            return;
-        }
-        try {
-            List<String> tablesInDatabase = listTables(name);
-            // Database cannot be deleted when a table exists
-            if (tablesInDatabase != null && !tablesInDatabase.isEmpty()) {
-                throw new RuntimeException(
-                        String.format(
-                                "Database %s is not empty. %s tables exist.",
-                                name, tablesInDatabase.size()));
-            }
-        } catch (DatabaseNotExistException e) {
-            throw new RuntimeException(e);
-        }
+        // delete table
+        execute(connections, JdbcUtils.DELETE_TABLES_SQL, catalogName, name);
+        // delete properties
         execute(connections, JdbcUtils.DELETE_ALL_DATABASE_PROPERTIES_SQL, catalogName, name);
     }
 
@@ -315,11 +326,7 @@ protected void renameTableImpl(Identifier fromTable, Identifier toTable) {
                 }
                 // Update table metadata
                 updateTableMetadataLocation(
-                        connections,
-                        catalogName,
-                        fromTable,
-                        toPath.toString(),
-                        fromPath.toString());
+                        connections, catalogName, toTable, toPath.toString(), fromPath.toString());
             }
         } catch (Exception e) {
             throw new RuntimeException("Failed to rename table " + fromTable.getFullName(), e);
@@ -380,6 +387,11 @@ public boolean tableExists(Identifier identifier) {
                 connections, catalogName, identifier.getDatabaseName(), identifier.getObjectName());
     }
 
+    @Override
+    public boolean caseSensitive() {
+        return false;
+    }
+
     @Override
     public Optional<CatalogLock.Factory> lockFactory() {
         return lockEnabled()
diff --git a/paimon-core/src/main/java/org/apache/paimon/jdbc/JdbcCatalogFactory.java b/paimon-core/src/main/java/org/apache/paimon/jdbc/JdbcCatalogFactory.java
@@ -36,6 +36,6 @@ public String identifier() {
 
     @Override
     public Catalog create(FileIO fileIO, Path warehouse, CatalogContext context) {
-        return new JdbcCatalog(fileIO, null, context.options(), warehouse.getName());
+        return new JdbcCatalog(fileIO, null, context.options().toMap(), warehouse.getName());
     }
 }
diff --git a/paimon-core/src/main/java/org/apache/paimon/jdbc/JdbcUtils.java b/paimon-core/src/main/java/org/apache/paimon/jdbc/JdbcUtils.java
@@ -104,6 +104,15 @@ public class JdbcUtils {
                     + " = ? AND "
                     + TABLE_DATABASE
                     + " = ?";
+
+    static final String DELETE_TABLES_SQL =
+            "DELETE FROM  "
+                    + CATALOG_TABLE_NAME
+                    + " WHERE "
+                    + CATALOG_NAME
+                    + " = ? AND "
+                    + TABLE_DATABASE
+                    + " = ?";
     static final String RENAME_TABLE_SQL =
             "UPDATE "
                     + CATALOG_TABLE_NAME
@@ -273,7 +282,7 @@ public class JdbcUtils {
     static final String LOCK_NAME = "lock_name";
     static final String ACQUIRED_AT = "acquired_at";
 
-    static final String CREATE_DISTRIBUTED_LOCK_TABLE =
+    static final String CREATE_DISTRIBUTED_LOCK_TABLE_SQL =
             "CREATE TABLE "
                     + DISTRIBUTED_LOCKS_TABLE_NAME
                     + "("
@@ -501,7 +510,7 @@ public static int execute(
             return connections.run(
                     conn -> {
                         try (PreparedStatement preparedStatement = conn.prepareStatement(sql)) {
-                            for (int pos = 0; pos < args.length; pos += 1) {
+                            for (int pos = 0; pos < args.length; pos++) {
                                 preparedStatement.setString(pos + 1, args[pos]);
                             }
                             return preparedStatement.executeUpdate();
diff --git a/paimon-core/src/main/resources/META-INF/services/org.apache.paimon.factories.Factory b/paimon-core/src/main/resources/META-INF/services/org.apache.paimon.factories.Factory
@@ -14,3 +14,4 @@
 # limitations under the License.
 
 org.apache.paimon.catalog.FileSystemCatalogFactory
+org.apache.paimon.jdbc.JdbcCatalogFactory
diff --git a/paimon-core/src/test/java/org/apache/paimon/jdbc/JdbcCatalogTest.java b/paimon-core/src/test/java/org/apache/paimon/jdbc/JdbcCatalogTest.java
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.paimon.jdbc;
+
+import org.apache.paimon.catalog.CatalogTestBase;
+import org.apache.paimon.catalog.Identifier;
+import org.apache.paimon.options.CatalogOptions;
+
+import org.apache.paimon.shade.guava30.com.google.common.collect.Maps;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+/** Tests for {@link JdbcCatalog}. */
+public class JdbcCatalogTest extends CatalogTestBase {
+
+    @BeforeEach
+    public void setUp() throws Exception {
+        super.setUp();
+        catalog = initCatalog("test-jdbc-catalog", Maps.newHashMap());
+    }
+
+    private JdbcCatalog initCatalog(String catalogName, Map<String, String> props) {
+        Map<String, String> properties = Maps.newHashMap();
+        properties.put(
+                CatalogOptions.URI.key(),
+                "jdbc:sqlite:file::memory:?ic" + UUID.randomUUID().toString().replace("-", ""));
+
+        properties.put(JdbcCatalog.PROPERTY_PREFIX + "username", "user");
+        properties.put(JdbcCatalog.PROPERTY_PREFIX + "password", "password");
+        properties.put(CatalogOptions.WAREHOUSE.key(), warehouse);
+        properties.put(JdbcCatalogOptions.INITIALIZE_CATALOG_TABLES.key(), "true");
+        properties.putAll(props);
+        JdbcCatalog jdbcCatalog = new JdbcCatalog(fileIO, catalogName, properties, warehouse);
+        return jdbcCatalog;
+    }
+
+    @Test
+    @Override
+    public void testListDatabasesWhenNoDatabases() {
+        List<String> databases = catalog.listDatabases();
+        assertThat(databases).isEqualTo(new ArrayList<>());
+    }
+
+    @Test
+    public void testCheckIdentifierUpperCase() throws Exception {
+        catalog.createDatabase("test_db", false);
+        assertThatThrownBy(
+                        () ->
+                                catalog.createTable(
+                                        Identifier.create("TEST_DB", "new_table"),
+                                        DEFAULT_TABLE_SCHEMA,
+                                        false))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("Database name [TEST_DB] cannot contain upper case in the catalog.");
+
+        assertThatThrownBy(
+                        () ->
+                                catalog.createTable(
+                                        Identifier.create("test_db", "NEW_TABLE"),
+                                        DEFAULT_TABLE_SCHEMA,
+                                        false))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("Table name [NEW_TABLE] cannot contain upper case in the catalog.");
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -36,6 +36,6 @@ public String identifier() {`
`36`	`36`
`37`	`37`	`@Override`
`38`	`38`	`public Catalog create(FileIO fileIO, Path warehouse, CatalogContext context) {`
`39`		`- return new JdbcCatalog(fileIO, null, context.options(), warehouse.getName());`
	`39`	`+ return new JdbcCatalog(fileIO, null, context.options().toMap(), warehouse.getName());`
`40`	`40`	`}`
`41`	`41`	`}`
Original file line number	Diff line number	Diff line change
`@@ -14,3 +14,4 @@`
`14`	`14`	`# limitations under the License.`
`15`	`15`
`16`	`16`	`org.apache.paimon.catalog.FileSystemCatalogFactory`
	`17`	`+org.apache.paimon.jdbc.JdbcCatalogFactory`