This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-3.0 by this push:
new 9b1f290ff7e [fix](catalog) should return error if try using a unknown
database (#40479) (#41850)
9b1f290ff7e is described below
commit 9b1f290ff7e42267e33b81c63553b2a6f1273cc2
Author: Rayner Chen <[email protected]>
AuthorDate: Wed Oct 16 17:33:02 2024 +0800
[fix](catalog) should return error if try using a unknown database (#40479)
(#41850)
bp #40479
---
.../apache/doris/datasource/ExternalCatalog.java | 38 +++++++++--
.../doris/datasource/hive/HMSExternalCatalog.java | 2 +-
.../doris/datasource/hive/HiveMetadataOps.java | 14 +++--
.../datasource/iceberg/IcebergMetadataOps.java | 12 +++-
.../java/org/apache/doris/mysql/MysqlProto.java | 6 +-
.../java/org/apache/doris/qe/ConnectProcessor.java | 6 +-
.../jdbc/test_jdbc_catalog_ddl.out | 27 ++++++++
...t_hive_insert_overwrite_with_empty_table.groovy | 11 ++--
.../jdbc/test_jdbc_catalog_ddl.groovy | 73 ++++++++++++++++++++++
9 files changed, 168 insertions(+), 21 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
index 37e44c63569..25329a1a829 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
@@ -33,6 +33,7 @@ import org.apache.doris.catalog.TableIf;
import org.apache.doris.cluster.ClusterNamespace;
import org.apache.doris.common.Config;
import org.apache.doris.common.DdlException;
+import org.apache.doris.common.FeConstants;
import org.apache.doris.common.Pair;
import org.apache.doris.common.UserException;
import org.apache.doris.common.Version;
@@ -251,7 +252,7 @@ public abstract class ExternalCatalog
Config.max_meta_object_cache_num,
ignored -> getFilteredDatabaseNames(),
dbName -> Optional.ofNullable(
- buildDbForInit(dbName,
Util.genIdByName(name, dbName), logType)),
+ buildDbForInit(dbName,
Util.genIdByName(name, dbName), logType, true)),
(key, value, cause) -> value.ifPresent(v ->
v.setUnInitialized(invalidCacheInInit)));
}
setLastUpdateTime(System.currentTimeMillis());
@@ -371,7 +372,7 @@ public abstract class ExternalCatalog
} else {
dbId = Env.getCurrentEnv().getNextId();
tmpDbNameToId.put(dbName, dbId);
- ExternalDatabase<? extends ExternalTable> db =
buildDbForInit(dbName, dbId, logType);
+ ExternalDatabase<? extends ExternalTable> db =
buildDbForInit(dbName, dbId, logType, false);
tmpIdToDb.put(dbId, db);
initCatalogLog.addCreateDb(dbId, dbName);
}
@@ -637,7 +638,7 @@ public abstract class ExternalCatalog
}
for (int i = 0; i < log.getCreateCount(); i++) {
ExternalDatabase<? extends ExternalTable> db =
- buildDbForInit(log.getCreateDbNames().get(i),
log.getCreateDbIds().get(i), log.getType());
+ buildDbForInit(log.getCreateDbNames().get(i),
log.getCreateDbIds().get(i), log.getType(), false);
if (db != null) {
tmpDbNameToId.put(db.getFullName(), db.getId());
tmpIdToDb.put(db.getId(), db);
@@ -660,8 +661,37 @@ public abstract class ExternalCatalog
}
}
+ /**
+ * Build a database instance.
+ * If checkExists is true, it will check if the database exists in the
remote system.
+ *
+ * @param dbName
+ * @param dbId
+ * @param logType
+ * @param checkExists
+ * @return
+ */
protected ExternalDatabase<? extends ExternalTable> buildDbForInit(String
dbName, long dbId,
- InitCatalogLog.Type logType) {
+ InitCatalogLog.Type logType, boolean checkExists) {
+ // When running ut, disable this check to make ut pass.
+ // Because in ut, the database is not created in remote system.
+ if (checkExists && !FeConstants.runningUnitTest) {
+ try {
+ List<String> dbNames = getDbNames();
+ if (!dbNames.contains(dbName)) {
+ dbNames = listDatabaseNames();
+ if (!dbNames.contains(dbName)) {
+ return null;
+ }
+ }
+ } catch (Throwable t) {
+ // If connection failed, it will throw exception.
+ // ignore it and treat it as not exist.
+ LOG.warn("Failed to check db {} exist in remote system, ignore
it.", dbName, t);
+ return null;
+ }
+ }
+
if (dbName.equals(InfoSchemaDb.DATABASE_NAME)) {
return new ExternalInfoSchemaDatabase(this, dbId);
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java
index 5173b414b8a..99fd96f65db 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java
@@ -256,7 +256,7 @@ public class HMSExternalCatalog extends ExternalCatalog {
LOG.debug("create database [{}]", dbName);
}
- ExternalDatabase<? extends ExternalTable> db = buildDbForInit(dbName,
dbId, logType);
+ ExternalDatabase<? extends ExternalTable> db = buildDbForInit(dbName,
dbId, logType, false);
if (useMetaCache.get()) {
if (isInitialized()) {
metaCache.updateCache(dbName, db,
Util.genIdByName(getQualifiedName(dbName)));
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HiveMetadataOps.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HiveMetadataOps.java
index 5ec49c380b7..022f646343d 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HiveMetadataOps.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HiveMetadataOps.java
@@ -248,20 +248,26 @@ public class HiveMetadataOps implements
ExternalMetadataOps {
@Override
public void dropTable(DropTableStmt stmt) throws DdlException {
String dbName = stmt.getDbName();
+ String tblName = stmt.getTableName();
ExternalDatabase<?> db = catalog.getDbNullable(stmt.getDbName());
if (db == null) {
- throw new DdlException("Failed to get database: '" + dbName + "'
in catalog: " + catalog.getName());
+ if (stmt.isSetIfExists()) {
+ LOG.info("database [{}] does not exist when drop table[{}]",
dbName, tblName);
+ return;
+ } else {
+ ErrorReport.reportDdlException(ErrorCode.ERR_BAD_DB_ERROR,
dbName);
+ }
}
- if (!tableExist(dbName, stmt.getTableName())) {
+ if (!tableExist(dbName, tblName)) {
if (stmt.isSetIfExists()) {
LOG.info("drop table[{}] which does not exist", dbName);
return;
} else {
- ErrorReport.reportDdlException(ErrorCode.ERR_UNKNOWN_TABLE,
stmt.getTableName(), dbName);
+ ErrorReport.reportDdlException(ErrorCode.ERR_UNKNOWN_TABLE,
tblName, dbName);
}
}
try {
- client.dropTable(dbName, stmt.getTableName());
+ client.dropTable(dbName, tblName);
db.setUnInitialized(true);
} catch (Exception e) {
throw new DdlException(e.getMessage(), e);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
index 66c57e37307..7fa7b5e84e8 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
@@ -167,14 +167,20 @@ public class IcebergMetadataOps implements
ExternalMetadataOps {
@Override
public void dropTable(DropTableStmt stmt) throws DdlException {
String dbName = stmt.getDbName();
+ String tableName = stmt.getTableName();
ExternalDatabase<?> db = dorisCatalog.getDbNullable(dbName);
if (db == null) {
- throw new DdlException("Failed to get database: '" + dbName + "'
in catalog: " + dorisCatalog.getName());
+ if (stmt.isSetIfExists()) {
+ LOG.info("database [{}] does not exist when drop table[{}]",
dbName, tableName);
+ return;
+ } else {
+ ErrorReport.reportDdlException(ErrorCode.ERR_BAD_DB_ERROR,
dbName);
+ }
}
- String tableName = stmt.getTableName();
+
if (!tableExist(dbName, tableName)) {
if (stmt.isSetIfExists()) {
- LOG.info("drop table[{}] which does not exist", dbName);
+ LOG.info("drop table[{}] which does not exist", tableName);
return;
} else {
ErrorReport.reportDdlException(ErrorCode.ERR_UNKNOWN_TABLE,
tableName, dbName);
diff --git a/fe/fe-core/src/main/java/org/apache/doris/mysql/MysqlProto.java
b/fe/fe-core/src/main/java/org/apache/doris/mysql/MysqlProto.java
index 09cfcb4ff4f..a672a217a33 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/mysql/MysqlProto.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/MysqlProto.java
@@ -243,11 +243,13 @@ public class MysqlProto {
if (catalogName != null) {
CatalogIf catalogIf =
context.getEnv().getCatalogMgr().getCatalog(catalogName);
if (catalogIf == null) {
- context.getState().setError(ErrorCode.ERR_BAD_DB_ERROR,
"No match catalog in doris: " + db);
+ context.getState()
+ .setError(ErrorCode.ERR_BAD_DB_ERROR,
ErrorCode.ERR_BAD_DB_ERROR.formatErrorMsg(db));
return false;
}
if (catalogIf.getDbNullable(dbFullName) == null) {
- context.getState().setError(ErrorCode.ERR_BAD_DB_ERROR,
"No match database in doris: " + db);
+ context.getState()
+ .setError(ErrorCode.ERR_BAD_DB_ERROR,
ErrorCode.ERR_BAD_DB_ERROR.formatErrorMsg(db));
return false;
}
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java
b/fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java
index 31be3184fde..6e495217c29 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java
@@ -151,11 +151,13 @@ public abstract class ConnectProcessor {
if (catalogName != null) {
CatalogIf catalogIf =
ctx.getEnv().getCatalogMgr().getCatalog(catalogName);
if (catalogIf == null) {
- ctx.getState().setError(ErrorCode.ERR_BAD_DB_ERROR, "No match
catalog in doris: " + fullDbName);
+ ctx.getState().setError(ErrorCode.ERR_BAD_DB_ERROR,
+ ErrorCode.ERR_BAD_DB_ERROR.formatErrorMsg(catalogName
+ "." + dbName));
return;
}
if (catalogIf.getDbNullable(dbName) == null) {
- ctx.getState().setError(ErrorCode.ERR_BAD_DB_ERROR, "No match
database in doris: " + fullDbName);
+ ctx.getState().setError(ErrorCode.ERR_BAD_DB_ERROR,
+ ErrorCode.ERR_BAD_DB_ERROR.formatErrorMsg(catalogName
+ "." + dbName));
return;
}
}
diff --git
a/regression-test/data/external_table_p0/jdbc/test_jdbc_catalog_ddl.out
b/regression-test/data/external_table_p0/jdbc/test_jdbc_catalog_ddl.out
new file mode 100644
index 00000000000..9629269ea3c
--- /dev/null
+++ b/regression-test/data/external_table_p0/jdbc/test_jdbc_catalog_ddl.out
@@ -0,0 +1,27 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !show_db --
+DORIS
+Doris
+doris
+doris_test
+information_schema
+init_db
+mysql
+show_test_do_not_modify
+
+-- !sql01 --
+12345
+
+-- !show_db --
+DORIS
+Doris
+doris
+doris_test
+information_schema
+init_db
+mysql
+show_test_do_not_modify
+
+-- !sql01 --
+12345
+
diff --git
a/regression-test/suites/external_table_p0/hive/write/test_hive_insert_overwrite_with_empty_table.groovy
b/regression-test/suites/external_table_p0/hive/write/test_hive_insert_overwrite_with_empty_table.groovy
index eea2e7a4864..9e195d0aff6 100644
---
a/regression-test/suites/external_table_p0/hive/write/test_hive_insert_overwrite_with_empty_table.groovy
+++
b/regression-test/suites/external_table_p0/hive/write/test_hive_insert_overwrite_with_empty_table.groovy
@@ -43,12 +43,13 @@ suite("test_hive_insert_overwrite_with_empty_table",
"p0,external,hive,external_
'hive.metastore.uris' = 'thrift://${externalEnvIp}:${hms_port}'
);"""
- sql """ use ${catalog}.${db1} """
-
- sql """ drop table if exists ${db1}.${tb1} """
- sql """ drop table if exists ${db1}.${tb2} """
- sql """ drop database if exists ${db1} """
+ sql """ drop database if exists ${catalog}.${db1} """
+ test {
+ sql """ use ${catalog}.${db1} """
+ exception "Unknown database"
+ }
+ sql """ switch ${catalog}"""
sql """ create database ${db1} """
sql """ create table ${db1}.${tb1} (id int, val int) partition by
list (val)() """
sql """ create table ${db1}.${tb2} (id int, val int) """
diff --git
a/regression-test/suites/external_table_p0/jdbc/test_jdbc_catalog_ddl.groovy
b/regression-test/suites/external_table_p0/jdbc/test_jdbc_catalog_ddl.groovy
new file mode 100644
index 00000000000..1b25362edd7
--- /dev/null
+++ b/regression-test/suites/external_table_p0/jdbc/test_jdbc_catalog_ddl.groovy
@@ -0,0 +1,73 @@
+// 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.
+
+suite("test_jdbc_catalog_ddl",
"p0,external,mysql,external_docker,external_docker_mysql") {
+
+ String enabled = context.config.otherConfigs.get("enableJdbcTest")
+ String externalEnvIp = context.config.otherConfigs.get("externalEnvIp")
+ String s3_endpoint = getS3Endpoint()
+ String bucket = getS3BucketName()
+ String driver_url =
"https://${bucket}.${s3_endpoint}/regression/jdbc_driver/mysql-connector-java-5.1.49.jar";
+ String mysql_port = context.config.otherConfigs.get("mysql_57_port");
+ // String driver_url = "mysql-connector-java-5.1.49.jar"
+ if (enabled != null && enabled.equalsIgnoreCase("true")) {
+ String catalog_name = "mysql_jdbc5_catalog";
+
+ for (String useMetaCache : ["true", "false"]) {
+ sql """drop catalog if exists ${catalog_name} """
+ sql """create catalog if not exists ${catalog_name} properties(
+ "type"="jdbc",
+ "user"="root",
+ "password"="123456",
+ "jdbc_url" =
"jdbc:mysql://${externalEnvIp}:${mysql_port}/doris_test?useSSL=false&zeroDateTimeBehavior=convertToNull",
+ "driver_url" = "${driver_url}",
+ "driver_class" = "com.mysql.jdbc.Driver",
+ "use_meta_cache" = "${useMetaCache}"
+ );"""
+ order_qt_show_db """ show databases from ${catalog_name}; """
+
+ // test wrong catalog and db
+ test {
+ sql """switch unknown_catalog"""
+ exception "Unknown catalog 'unknown_catalog'"
+ }
+ test {
+ sql """use unknown_catalog.db1"""
+ exception """Unknown catalog 'unknown_catalog'"""
+ }
+ test {
+ sql """use ${catalog_name}.unknown_db"""
+ exception """Unknown database 'unknown_db'"""
+ }
+
+ // create a database in mysql
+ sql """CALL EXECUTE_STMT("${catalog_name}", "drop database if
exists temp_database")"""
+ sql """CALL EXECUTE_STMT("${catalog_name}", "create database
temp_database")"""
+ sql """CALL EXECUTE_STMT("${catalog_name}", "drop table if exists
temp_database.temp_table")"""
+ sql """CALL EXECUTE_STMT("${catalog_name}", "create table
temp_database.temp_table (k1 int)")"""
+ sql """CALL EXECUTE_STMT("${catalog_name}", "insert into
temp_database.temp_table values(12345)")"""
+
+ if (useMetaCache.equals("false")) {
+ sql """refresh catalog ${catalog_name}"""
+ }
+ sql "use ${catalog_name}.temp_database"
+ qt_sql01 """select * from temp_table"""
+ sql """CALL EXECUTE_STMT("${catalog_name}", "drop database if
exists temp_database")"""
+ }
+ }
+}
+
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
