Skip to content

Commit

Permalink
[fix](catalog) should return error if try using a unknown database (a…
Browse files Browse the repository at this point in the history
  • Loading branch information
morningman authored Oct 16, 2024
1 parent 63868b4 commit 9b1f290
Show file tree
Hide file tree
Showing 9 changed files with 168 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
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;
Expand Down Expand Up @@ -251,7 +252,7 @@ public final synchronized void makeSureInitialized() {
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());
Expand Down Expand Up @@ -371,7 +372,7 @@ private void init() {
} 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);
}
Expand Down Expand Up @@ -637,7 +638,7 @@ public void replayInitCatalog(InitCatalogLog log) {
}
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);
Expand All @@ -660,8 +661,37 @@ public Optional<ExternalDatabase<? extends ExternalTable>> getDbForReplay(long d
}
}

/**
* 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ public void registerDatabase(long dbId, String dbName) {
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)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,20 +248,26 @@ public boolean createTable(CreateTableStmt stmt) throws UserException {
@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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,20 @@ public boolean createTable(CreateTableStmt stmt) throws UserException {
@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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,13 @@ public static boolean negotiate(ConnectContext context) throws IOException {
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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,13 @@ protected void handleInitDb(String fullDbName) {
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;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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) """
Expand Down
Original file line number Diff line number Diff line change
@@ -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")"""
}
}
}

0 comments on commit 9b1f290

Please sign in to comment.