diff --git a/src/common/exception/src/exception_code.rs b/src/common/exception/src/exception_code.rs index 4bd496b096d77..3d469f892e5c4 100644 --- a/src/common/exception/src/exception_code.rs +++ b/src/common/exception/src/exception_code.rs @@ -501,6 +501,8 @@ build_exceptions! { DatabaseAlreadyExists(2301), /// Table already exists TableAlreadyExists(2302), + /// Database version mismatch + DatabaseVersionMismatched(2303), /// View already exists ViewAlreadyExists(2306), /// Create table with drop time diff --git a/src/meta/api/src/database_api.rs b/src/meta/api/src/database_api.rs index a1354967b2b81..ad7172576fcaa 100644 --- a/src/meta/api/src/database_api.rs +++ b/src/meta/api/src/database_api.rs @@ -20,6 +20,7 @@ use chrono::Utc; use databend_common_meta_app::app_error::AppError; use databend_common_meta_app::app_error::CreateDatabaseWithDropTime; use databend_common_meta_app::app_error::DatabaseAlreadyExists; +use databend_common_meta_app::app_error::DatabaseVersionMismatched; use databend_common_meta_app::app_error::UndropDbHasNoHistory; use databend_common_meta_app::app_error::UndropDbWithNoDropTime; use databend_common_meta_app::app_error::UnknownDatabase; @@ -43,10 +44,13 @@ use databend_common_meta_app::schema::RenameDatabaseReply; use databend_common_meta_app::schema::RenameDatabaseReq; use databend_common_meta_app::schema::UndropDatabaseReply; use databend_common_meta_app::schema::UndropDatabaseReq; +use databend_common_meta_app::schema::UpdateDatabaseOptionsReply; +use databend_common_meta_app::schema::UpdateDatabaseOptionsReq; use databend_common_meta_app::KeyWithTenant; use databend_common_meta_kvapi::kvapi; use databend_common_meta_kvapi::kvapi::DirName; use databend_common_meta_types::ConditionResult::Eq; +use databend_common_meta_types::MatchSeq; use databend_common_meta_types::MetaError; use databend_common_meta_types::MetaId; use databend_common_meta_types::SeqV; @@ -65,6 +69,7 @@ use crate::error_util::db_has_to_not_exist; use crate::fetch_id; use crate::kv_app_error::KVAppError; use crate::kv_pb_api::KVPbApi; +use crate::kv_pb_api::UpsertPB; use crate::serialize_struct; use crate::serialize_u64; use crate::txn_backoff::txn_backoff; @@ -494,6 +499,64 @@ where } } + #[logcall::logcall] + #[fastrace::trace] + async fn update_database_options( + &self, + req: UpdateDatabaseOptionsReq, + ) -> Result { + debug!(req :? =(&req); "SchemaApi: {}", func_name!()); + + let db_id = req.db_id; + let expected_seq = req.expected_meta_seq; + let new_options = req.options.clone(); + let db_key = DatabaseId::new(db_id); + + let seq_meta = self.get_pb(&db_key).await?; + let Some(seq_meta) = seq_meta else { + return Err(KVAppError::AppError(AppError::UnknownDatabaseId( + UnknownDatabaseId::new(db_id, "update_database_options"), + ))); + }; + + if seq_meta.seq != expected_seq { + return Err(KVAppError::AppError(AppError::DatabaseVersionMismatched( + DatabaseVersionMismatched::new( + db_id, + MatchSeq::Exact(expected_seq), + seq_meta.seq, + "update_database_options", + ), + ))); + } + + let mut meta = seq_meta.data; + meta.options = new_options; + meta.updated_on = Utc::now(); + + let upsert = UpsertPB::update_exact(db_key, SeqV::new(expected_seq, meta)); + let transition = self.upsert_pb(&upsert).await?; + + if !transition.is_changed() { + let curr_seq = self + .get_pb(&db_key) + .await? + .map(|v| v.seq()) + .unwrap_or_default(); + + return Err(KVAppError::AppError(AppError::DatabaseVersionMismatched( + DatabaseVersionMismatched::new( + db_id, + MatchSeq::Exact(expected_seq), + curr_seq, + "update_database_options", + ), + ))); + } + + Ok(UpdateDatabaseOptionsReply {}) + } + #[logcall::logcall] #[fastrace::trace] async fn get_database(&self, req: GetDatabaseReq) -> Result, KVAppError> { diff --git a/src/meta/app/src/app_error.rs b/src/meta/app/src/app_error.rs index 704f61c0da96c..b379949ca45e4 100644 --- a/src/meta/app/src/app_error.rs +++ b/src/meta/app/src/app_error.rs @@ -313,6 +313,26 @@ impl TableVersionMismatched { } } +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +#[error("DatabaseVersionMismatched: {db_id} expect `{expect}` but `{curr}` while `{context}`")] +pub struct DatabaseVersionMismatched { + db_id: u64, + expect: MatchSeq, + curr: u64, + context: String, +} + +impl DatabaseVersionMismatched { + pub fn new(db_id: u64, expect: MatchSeq, curr: u64, context: impl Into) -> Self { + Self { + db_id, + expect, + curr, + context: context.into(), + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] #[error("StreamAlreadyExists: {name} while {context}")] pub struct StreamAlreadyExists { @@ -1010,6 +1030,9 @@ pub enum AppError { #[error(transparent)] TableVersionMismatched(#[from] TableVersionMismatched), + #[error(transparent)] + DatabaseVersionMismatched(#[from] DatabaseVersionMismatched), + #[error(transparent)] DuplicatedUpsertFiles(#[from] DuplicatedUpsertFiles), @@ -1293,6 +1316,8 @@ impl AppErrorMessage for UnknownDatabaseId {} impl AppErrorMessage for TableVersionMismatched {} +impl AppErrorMessage for DatabaseVersionMismatched {} + impl AppErrorMessage for StreamAlreadyExists { fn message(&self) -> String { format!("'{}' as stream Already Exists", self.name) @@ -1653,6 +1678,9 @@ impl From for ErrorCode { AppError::UndropTableHasNoHistory(err) => { ErrorCode::UndropTableHasNoHistory(err.message()) } + AppError::DatabaseVersionMismatched(err) => { + ErrorCode::DatabaseVersionMismatched(err.message()) + } AppError::TableVersionMismatched(err) => { ErrorCode::TableVersionMismatched(err.message()) } diff --git a/src/meta/app/src/schema/database.rs b/src/meta/app/src/schema/database.rs index 3cf187b37e731..26792c116a37f 100644 --- a/src/meta/app/src/schema/database.rs +++ b/src/meta/app/src/schema/database.rs @@ -265,6 +265,29 @@ pub struct DropDatabaseReply { pub db_id: u64, } +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct UpdateDatabaseOptionsReq { + pub db_id: u64, + /// The database meta sequence the caller observed. Used for CAS semantics. + pub expected_meta_seq: u64, + /// The complete option map that should replace the existing options when the + /// expected meta sequence still matches. + pub options: BTreeMap, +} + +impl Display for UpdateDatabaseOptionsReq { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + write!( + f, + "update_db_options:{}@{}={:?}", + self.db_id, self.expected_meta_seq, self.options + ) + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct UpdateDatabaseOptionsReply {} + #[derive(Clone, Debug, PartialEq, Eq)] pub struct UndropDatabaseReq { pub name_ident: DatabaseNameIdent, diff --git a/src/meta/app/src/schema/mod.rs b/src/meta/app/src/schema/mod.rs index 12597512975ef..f46fb27370f77 100644 --- a/src/meta/app/src/schema/mod.rs +++ b/src/meta/app/src/schema/mod.rs @@ -76,6 +76,8 @@ pub use database::RenameDatabaseReq; pub use database::ShareDbId; pub use database::UndropDatabaseReply; pub use database::UndropDatabaseReq; +pub use database::UpdateDatabaseOptionsReply; +pub use database::UpdateDatabaseOptionsReq; pub use database_id::DatabaseId; pub use database_id_history_ident::DatabaseIdHistoryIdent; pub use dictionary::*; diff --git a/src/query/ast/src/ast/statements/database.rs b/src/query/ast/src/ast/statements/database.rs index 5636a9cbf0cba..7edb4e579cd38 100644 --- a/src/query/ast/src/ast/statements/database.rs +++ b/src/query/ast/src/ast/statements/database.rs @@ -109,7 +109,17 @@ impl Display for CreateDatabaseStmt { write!(f, " ENGINE = {engine}")?; } - // TODO(leiysky): display rest information + if !self.options.is_empty() { + write!(f, " OPTIONS (")?; + for (i, option) in self.options.iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + write!(f, "{} = '{}'", option.name, option.value)?; + } + write!(f, ")")?; + } + Ok(()) } } @@ -169,6 +179,16 @@ impl Display for AlterDatabaseStmt { AlterDatabaseAction::RefreshDatabaseCache => { write!(f, " REFRESH CACHE")?; } + AlterDatabaseAction::SetOptions { options } => { + write!(f, " SET OPTIONS (")?; + for (i, option) in options.iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + write!(f, "{} = '{}'", option.name, option.value)?; + } + write!(f, ")")?; + } } Ok(()) @@ -179,6 +199,7 @@ impl Display for AlterDatabaseStmt { pub enum AlterDatabaseAction { RenameDatabase { new_db: Identifier }, RefreshDatabaseCache, + SetOptions { options: Vec }, } #[derive(Debug, Clone, PartialEq, Eq, Drive, DriveMut)] diff --git a/src/query/ast/src/parser/statement.rs b/src/query/ast/src/parser/statement.rs index 0fa1d8838ac36..eaf34d829f8ed 100644 --- a/src/query/ast/src/parser/statement.rs +++ b/src/query/ast/src/parser/statement.rs @@ -55,6 +55,7 @@ pub type ShareDatabaseParams = (ShareNameIdent, Identifier); #[derive(Clone)] pub enum CreateDatabaseOption { DatabaseEngine(DatabaseEngine), + Options(Vec), } fn procedure_type_name(i: Input) -> IResult> { @@ -796,28 +797,24 @@ pub fn statement_body(i: Input) -> IResult { ~ ( DATABASE | SCHEMA ) ~ ( IF ~ ^NOT ~ ^EXISTS )? ~ #database_ref - ~ #create_database_option? + ~ ( ENGINE ~ ^"=" ~ ^#database_engine )? + ~ ( OPTIONS ~ ^"(" ~ ^#sql_property_list ~ ^")" )? }, - |(_, opt_or_replace, _, opt_if_not_exists, database, create_database_option)| { + |(_, opt_or_replace, _, opt_if_not_exists, database, engine_opt, options_opt)| { let create_option = parse_create_option(opt_or_replace.is_some(), opt_if_not_exists.is_some())?; - let statement = match create_database_option { - Some(CreateDatabaseOption::DatabaseEngine(engine)) => { - Statement::CreateDatabase(CreateDatabaseStmt { - create_option, - database, - engine: Some(engine), - options: vec![], - }) - } - None => Statement::CreateDatabase(CreateDatabaseStmt { - create_option, - database, - engine: None, - options: vec![], - }), - }; + let engine = engine_opt.map(|(_, _, engine)| engine); + let options = options_opt + .map(|(_, _, options, _)| options) + .unwrap_or_default(); + + let statement = Statement::CreateDatabase(CreateDatabaseStmt { + create_option, + database, + engine, + options, + }); Ok(statement) }, @@ -4133,9 +4130,17 @@ pub fn alter_database_action(i: Input) -> IResult { |(_, _)| AlterDatabaseAction::RefreshDatabaseCache, ); + let set_options = map( + rule! { + SET ~ OPTIONS ~ "(" ~ #sql_property_list ~ ")" + }, + |(_, _, _, options, _)| AlterDatabaseAction::SetOptions { options }, + ); + rule!( #rename_database | #refresh_cache + | #set_options )(i) } @@ -5061,18 +5066,40 @@ pub fn database_engine(i: Input) -> IResult { } pub fn create_database_option(i: Input) -> IResult { - let mut create_db_engine = map( + let create_db_engine = map( rule! { ENGINE ~ ^"=" ~ ^#database_engine }, |(_, _, option)| CreateDatabaseOption::DatabaseEngine(option), ); + let create_db_options = map( + rule! { + OPTIONS ~ "(" ~ #sql_property_list ~ ")" + }, + |(_, _, options, _)| CreateDatabaseOption::Options(options), + ); + rule!( #create_db_engine + | #create_db_options )(i) } +pub fn sql_property_list(i: Input) -> IResult> { + let property = map( + rule! { + #ident ~ "=" ~ #option_to_string + }, + |(name, _, value)| SQLProperty { + name: name.name, + value, + }, + ); + + comma_separated_list1(property)(i) +} + pub fn catalog_type(i: Input) -> IResult { alt(( value(CatalogType::Default, rule! { DEFAULT }), diff --git a/src/query/ast/tests/it/parser.rs b/src/query/ast/tests/it/parser.rs index 00ea5ea06d00b..98cedacb91b99 100644 --- a/src/query/ast/tests/it/parser.rs +++ b/src/query/ast/tests/it/parser.rs @@ -157,6 +157,8 @@ fn test_statement() { r#"create database if not exists a;"#, r#"create database ctl.t engine = Default;"#, r#"create database t engine = Default;"#, + r#"create database test_db OPTIONS (DEFAULT_STORAGE_CONNECTION = 'my_conn', DEFAULT_STORAGE_PATH = 's3://bucket/path');"#, + r#"create database mydb ENGINE = DEFAULT OPTIONS (DEFAULT_STORAGE_CONNECTION = 'test_conn', DEFAULT_STORAGE_PATH = 's3://test/path');"#, r#"CREATE TABLE `t3`(a int not null, b int not null, c int not null) bloom_index_columns='a,b,c' COMPRESSION='zstd' STORAGE_FORMAT='native';"#, r#"create or replace database a;"#, r#"drop database ctl.t;"#, @@ -168,6 +170,7 @@ fn test_statement() { r#"create view v1(c1) as select number % 3 as a from numbers(1000);"#, r#"create or replace view v1(c1) as select number % 3 as a from numbers(1000);"#, r#"alter view v1(c2) as select number % 3 as a from numbers(1000);"#, + r#"alter database test_db SET OPTIONS (DEFAULT_STORAGE_CONNECTION = 'updated_conn');"#, r#"show views"#, r#"show views format TabSeparatedWithNamesAndTypes;"#, r#"show full views"#, diff --git a/src/query/ast/tests/it/testdata/stmt-error.txt b/src/query/ast/tests/it/testdata/stmt-error.txt index bf6b46d2d65ec..e6ee3a1b949c2 100644 --- a/src/query/ast/tests/it/testdata/stmt-error.txt +++ b/src/query/ast/tests/it/testdata/stmt-error.txt @@ -249,7 +249,7 @@ error: --> SQL:1:23 | 1 | alter database system x rename to db - | ----- ^ unexpected `x`, expecting `RENAME`, `REFRESH`, or `.` + | ----- ^ unexpected `x`, expecting `RENAME`, `REFRESH`, `SET`, or `.` | | | while parsing `ALTER DATABASE [IF EXISTS] ` diff --git a/src/query/ast/tests/it/testdata/stmt.txt b/src/query/ast/tests/it/testdata/stmt.txt index 17c899dd816ea..2ae2911e4eaef 100644 --- a/src/query/ast/tests/it/testdata/stmt.txt +++ b/src/query/ast/tests/it/testdata/stmt.txt @@ -3773,6 +3773,76 @@ CreateDatabase( ) +---------- Input ---------- +create database test_db OPTIONS (DEFAULT_STORAGE_CONNECTION = 'my_conn', DEFAULT_STORAGE_PATH = 's3://bucket/path'); +---------- Output --------- +CREATE DATABASE test_db OPTIONS (DEFAULT_STORAGE_CONNECTION = 'my_conn', DEFAULT_STORAGE_PATH = 's3://bucket/path') +---------- AST ------------ +CreateDatabase( + CreateDatabaseStmt { + create_option: Create, + database: DatabaseRef { + catalog: None, + database: Identifier { + span: Some( + 16..23, + ), + name: "test_db", + quote: None, + ident_type: None, + }, + }, + engine: None, + options: [ + SQLProperty { + name: "DEFAULT_STORAGE_CONNECTION", + value: "my_conn", + }, + SQLProperty { + name: "DEFAULT_STORAGE_PATH", + value: "s3://bucket/path", + }, + ], + }, +) + + +---------- Input ---------- +create database mydb ENGINE = DEFAULT OPTIONS (DEFAULT_STORAGE_CONNECTION = 'test_conn', DEFAULT_STORAGE_PATH = 's3://test/path'); +---------- Output --------- +CREATE DATABASE mydb ENGINE = DEFAULT OPTIONS (DEFAULT_STORAGE_CONNECTION = 'test_conn', DEFAULT_STORAGE_PATH = 's3://test/path') +---------- AST ------------ +CreateDatabase( + CreateDatabaseStmt { + create_option: Create, + database: DatabaseRef { + catalog: None, + database: Identifier { + span: Some( + 16..20, + ), + name: "mydb", + quote: None, + ident_type: None, + }, + }, + engine: Some( + Default, + ), + options: [ + SQLProperty { + name: "DEFAULT_STORAGE_CONNECTION", + value: "test_conn", + }, + SQLProperty { + name: "DEFAULT_STORAGE_PATH", + value: "s3://test/path", + }, + ], + }, +) + + ---------- Input ---------- CREATE TABLE `t3`(a int not null, b int not null, c int not null) bloom_index_columns='a,b,c' COMPRESSION='zstd' STORAGE_FORMAT='native'; ---------- Output --------- @@ -4678,6 +4748,35 @@ AlterView( ) +---------- Input ---------- +alter database test_db SET OPTIONS (DEFAULT_STORAGE_CONNECTION = 'updated_conn'); +---------- Output --------- +ALTER DATABASE test_db SET OPTIONS (DEFAULT_STORAGE_CONNECTION = 'updated_conn') +---------- AST ------------ +AlterDatabase( + AlterDatabaseStmt { + if_exists: false, + catalog: None, + database: Identifier { + span: Some( + 15..22, + ), + name: "test_db", + quote: None, + ident_type: None, + }, + action: SetOptions { + options: [ + SQLProperty { + name: "DEFAULT_STORAGE_CONNECTION", + value: "updated_conn", + }, + ], + }, + }, +) + + ---------- Input ---------- show views ---------- Output --------- diff --git a/src/query/catalog/src/database.rs b/src/query/catalog/src/database.rs index ce012ccd4be7e..c97fb321c9f40 100644 --- a/src/query/catalog/src/database.rs +++ b/src/query/catalog/src/database.rs @@ -187,6 +187,18 @@ pub trait Database: DynClone + Sync + Send { ))) } + #[async_backtrace::framed] + async fn update_options( + &self, + _expected_meta_seq: u64, + _options: BTreeMap, + ) -> Result<()> { + Err(ErrorCode::Unimplemented(format!( + "UnImplement update_options in {} Database", + self.name() + ))) + } + #[async_backtrace::framed] async fn upsert_table_option( &self, diff --git a/src/query/service/src/databases/default/default_database.rs b/src/query/service/src/databases/default/default_database.rs index f2d8764bd7e88..d41f0e4f6f8aa 100644 --- a/src/query/service/src/databases/default/default_database.rs +++ b/src/query/service/src/databases/default/default_database.rs @@ -12,10 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::BTreeMap; use std::sync::Arc; use databend_common_catalog::table::Table; use databend_common_exception::Result; +use databend_common_meta_api::DatabaseApi; use databend_common_meta_api::SecurityApi; use databend_common_meta_api::TableApi; use databend_common_meta_app::app_error::AppError; @@ -46,6 +48,7 @@ use databend_common_meta_app::schema::TableInfo; use databend_common_meta_app::schema::TruncateTableReply; use databend_common_meta_app::schema::TruncateTableReq; use databend_common_meta_app::schema::UndropTableReq; +use databend_common_meta_app::schema::UpdateDatabaseOptionsReq; use databend_common_meta_app::schema::UpsertTableOptionReply; use databend_common_meta_app::schema::UpsertTableOptionReq; @@ -268,6 +271,21 @@ impl Database for DefaultDatabase { Ok(res) } + #[async_backtrace::framed] + async fn update_options( + &self, + expected_meta_seq: u64, + options: BTreeMap, + ) -> Result<()> { + let req = UpdateDatabaseOptionsReq { + db_id: self.db_info.database_id.db_id, + expected_meta_seq, + options, + }; + self.ctx.meta.update_database_options(req).await?; + Ok(()) + } + #[async_backtrace::framed] async fn upsert_table_option( &self, diff --git a/src/query/service/src/interpreters/access/privilege_access.rs b/src/query/service/src/interpreters/access/privilege_access.rs index 2ca845e0532a0..cc34682c618a3 100644 --- a/src/query/service/src/interpreters/access/privilege_access.rs +++ b/src/query/service/src/interpreters/access/privilege_access.rs @@ -246,6 +246,10 @@ impl PrivilegeAccess { ) -> Result<()> { self.access_system_history(Some(catalog_name), Some(db_name), None, privileges)?; let tenant = self.ctx.get_tenant(); + let catalog = self.ctx.get_catalog(catalog_name).await?; + if if_exists && !catalog.exists_database(&tenant, db_name).await? { + return Ok(()); + } let check_current_role_only = match privileges { // create table/stream need check db's Create Privilege UserPrivilegeType::Create => true, @@ -264,7 +268,6 @@ impl PrivilegeAccess { return Ok(()); } Err(_err) => { - let catalog = self.ctx.get_catalog(catalog_name).await?; match self .convert_to_id(&tenant, &catalog, db_name, None, false) .await @@ -1693,6 +1696,16 @@ impl AccessChecker for PrivilegeAccess { Plan::RenameWorkloadGroup(_) => {} Plan::SetWorkloadGroupQuotas(_) => {} Plan::UnsetWorkloadGroupQuotas(_) => {} + Plan::AlterDatabase(plan) => { + self + .validate_db_access( + &plan.catalog, + &plan.database, + UserPrivilegeType::Alter, + plan.if_exists, + ) + .await?; + } } Ok(()) diff --git a/src/query/service/src/interpreters/interpreter_alter_database.rs b/src/query/service/src/interpreters/interpreter_alter_database.rs new file mode 100644 index 0000000000000..2996caf04f58d --- /dev/null +++ b/src/query/service/src/interpreters/interpreter_alter_database.rs @@ -0,0 +1,184 @@ +// Copyright 2021 Datafuse Labs +// +// Licensed 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. + +use std::collections::BTreeMap; +use std::sync::Arc; + +use databend_common_catalog::table_context::TableContext; +use databend_common_exception::Result; +use databend_common_sql::planner::binder::ddl::database::DEFAULT_STORAGE_CONNECTION; +use databend_common_sql::planner::binder::ddl::database::DEFAULT_STORAGE_PATH; +use databend_common_sql::plans::AlterDatabasePlan; +use log::debug; + +use crate::interpreters::Interpreter; +use crate::pipelines::PipelineBuildResult; +use crate::sessions::QueryContext; + +#[derive(Debug)] +pub struct AlterDatabaseInterpreter { + ctx: Arc, + plan: AlterDatabasePlan, +} + +impl AlterDatabaseInterpreter { + pub fn try_create(ctx: Arc, plan: AlterDatabasePlan) -> Result { + Ok(AlterDatabaseInterpreter { ctx, plan }) + } +} + +#[async_trait::async_trait] +impl Interpreter for AlterDatabaseInterpreter { + fn name(&self) -> &str { + "AlterDatabaseInterpreter" + } + + fn is_ddl(&self) -> bool { + true + } + + #[fastrace::trace] + #[async_backtrace::framed] + async fn execute2(&self) -> Result { + debug!("ctx.id" = self.ctx.get_id().as_str(); "alter_database_execute"); + let catalog = self.ctx.get_catalog(&self.plan.catalog).await?; + let database = match catalog + .get_database(&self.plan.tenant, &self.plan.database) + .await + { + Ok(db) => db, + Err(err) => { + if self.plan.if_exists + && err.code() == databend_common_exception::ErrorCode::UNKNOWN_DATABASE + { + return Ok(PipelineBuildResult::create()); + } + return Err(err); + } + }; + + // Merge provided options with the existing database options + let mut merged_options = database.options().clone(); + for (key, value) in &self.plan.options { + merged_options.insert(key.clone(), value.clone()); + } + + let connection_value = merged_options.get(DEFAULT_STORAGE_CONNECTION).cloned(); + let path_value = merged_options.get(DEFAULT_STORAGE_PATH).cloned(); + + // Check if both options are present together in the final merged state + // This ensures that after ALTER, the database still has both options configured + if connection_value.is_some() != path_value.is_some() { + return Err(databend_common_exception::ErrorCode::BadArguments( + "DEFAULT_STORAGE_CONNECTION and DEFAULT_STORAGE_PATH options must be used together", + )); + } + + let connection = if let Some(ref connection_name) = connection_value { + match self.ctx.get_connection(connection_name).await { + Ok(conn) => Some(conn), + Err(_) => { + return Err(databend_common_exception::ErrorCode::BadArguments(format!( + "Connection '{}' does not exist. Please create the connection first using CREATE CONNECTION", + connection_name + ))); + } + } + } else { + None + }; + + if let (Some(connection), Some(path)) = (connection, path_value.clone()) { + let connection_name = connection_value + .as_deref() + .expect("connection name must exist when connection is Some"); + + let uri_for_scheme = + databend_common_ast::ast::UriLocation::from_uri(path.clone(), BTreeMap::new()) + .map_err(|e| { + databend_common_exception::ErrorCode::BadArguments(format!( + "Invalid storage path '{}': {}", + path, e + )) + })?; + + let path_protocol = uri_for_scheme.protocol.to_ascii_lowercase(); + let connection_protocol = connection.storage_type.to_ascii_lowercase(); + + if path_protocol != connection_protocol { + return Err(databend_common_exception::ErrorCode::BadArguments(format!( + "{} protocol '{}' does not match connection '{}' protocol '{}'", + DEFAULT_STORAGE_PATH, + uri_for_scheme.protocol, + connection_name, + connection.storage_type + ))); + } + + let mut uri_location = databend_common_ast::ast::UriLocation::from_uri( + path.clone(), + connection.storage_params, + )?; + + let storage_params = databend_common_sql::binder::parse_storage_params_from_uri( + &mut uri_location, + Some(&*self.ctx), + "when setting database DEFAULT_STORAGE_PATH", + ) + .await + .map_err(|e| { + databend_common_exception::ErrorCode::BadArguments(format!( + "Invalid storage path '{}': {}", + path, e + )) + })?; + + if !storage_params.is_secure() + && !databend_common_config::GlobalConfig::instance() + .storage + .allow_insecure + { + return Err(databend_common_exception::ErrorCode::StorageInsecure( + "Database default storage path points to insecure storage, which is not allowed", + )); + } + + let operator = + databend_common_storage::init_operator(&storage_params).map_err(|e| { + databend_common_exception::ErrorCode::BadArguments(format!( + "Failed to access storage location '{}': {}", + path, e + )) + })?; + + databend_common_sql::binder::verify_external_location_privileges(operator) + .await + .map_err(|e| { + databend_common_exception::ErrorCode::BadArguments(format!( + "Failed to verify permissions for database default storage location '{}': {}", + path, e + )) + })?; + } + + let expected_meta_seq = database.get_db_info().meta.seq; + + // Persist the fully merged options with CAS semantics in the meta store + database + .update_options(expected_meta_seq, merged_options) + .await?; + + Ok(PipelineBuildResult::create()) + } +} diff --git a/src/query/service/src/interpreters/interpreter_database_show_create.rs b/src/query/service/src/interpreters/interpreter_database_show_create.rs index 262575f510bbc..3d1b63ac64a56 100644 --- a/src/query/service/src/interpreters/interpreter_database_show_create.rs +++ b/src/query/service/src/interpreters/interpreter_database_show_create.rs @@ -69,6 +69,16 @@ impl Interpreter for ShowCreateDatabaseInterpreter { } else { info.push_str(&engine); } + + let options = db + .options() + .iter() + .map(|(k, v)| format!("{}='{}'", k, v)) + .collect::>() + .join(", "); + if !db.options().is_empty() { + write!(info, " OPTIONS ({})", options).expect("failed to format database options"); + } } PipelineBuildResult::from_blocks(vec![DataBlock::new( diff --git a/src/query/service/src/interpreters/interpreter_factory.rs b/src/query/service/src/interpreters/interpreter_factory.rs index a43aca4c76fac..d09c3e1d46dc9 100644 --- a/src/query/service/src/interpreters/interpreter_factory.rs +++ b/src/query/service/src/interpreters/interpreter_factory.rs @@ -103,6 +103,7 @@ use crate::interpreters::interpreter_unassign_warehouse_nodes::UnassignWarehouse use crate::interpreters::interpreter_unset_workload_group_quotas::UnsetWorkloadGroupQuotasInterpreter; use crate::interpreters::interpreter_use_warehouse::UseWarehouseInterpreter; use crate::interpreters::interpreter_view_describe::DescribeViewInterpreter; +use crate::interpreters::AlterDatabaseInterpreter; use crate::interpreters::AlterUserInterpreter; use crate::interpreters::CreateStreamInterpreter; use crate::interpreters::DescUserInterpreter; @@ -632,6 +633,9 @@ impl InterpreterFactory { Plan::RefreshDatabaseCache(refresh_database_cache) => Ok(Arc::new( RefreshDatabaseCacheInterpreter::try_create(ctx, *refresh_database_cache.clone())?, )), + Plan::AlterDatabase(alter_database) => Ok(Arc::new( + AlterDatabaseInterpreter::try_create(ctx, *alter_database.clone())?, + )), Plan::Kill(p) => Ok(Arc::new(KillInterpreter::try_create(ctx, *p.clone())?)), Plan::RevertTable(p) => Ok(Arc::new(RevertTableInterpreter::try_create( diff --git a/src/query/service/src/interpreters/mod.rs b/src/query/service/src/interpreters/mod.rs index 1d014c1965c99..417e9fd5e2611 100644 --- a/src/query/service/src/interpreters/mod.rs +++ b/src/query/service/src/interpreters/mod.rs @@ -18,6 +18,7 @@ pub(crate) mod common; mod hook; mod interpreter; mod interpreter_add_warehouse_cluster; +mod interpreter_alter_database; mod interpreter_assign_warehouse_nodes; mod interpreter_catalog_create; mod interpreter_catalog_drop; @@ -190,6 +191,7 @@ pub use hook::HookOperator; pub use interpreter::interpreter_plan_sql; pub use interpreter::Interpreter; pub use interpreter::InterpreterPtr; +pub use interpreter_alter_database::AlterDatabaseInterpreter; pub use interpreter_catalog_use::UseCatalogInterpreter; pub use interpreter_cluster_key_alter::AlterTableClusterKeyInterpreter; pub use interpreter_cluster_key_drop::DropTableClusterKeyInterpreter; diff --git a/src/query/sql/src/planner/binder/ddl/database.rs b/src/query/sql/src/planner/binder/ddl/database.rs index a071a8d39284f..460f9ea9ec0bf 100644 --- a/src/query/sql/src/planner/binder/ddl/database.rs +++ b/src/query/sql/src/planner/binder/ddl/database.rs @@ -36,6 +36,7 @@ use log::debug; use crate::binder::Binder; use crate::planner::semantic::normalize_identifier; +use crate::plans::AlterDatabasePlan; use crate::plans::CreateDatabasePlan; use crate::plans::DropDatabasePlan; use crate::plans::Plan; @@ -48,6 +49,9 @@ use crate::plans::UndropDatabasePlan; use crate::BindContext; use crate::SelectBuilder; +pub const DEFAULT_STORAGE_CONNECTION: &str = "DEFAULT_STORAGE_CONNECTION"; +pub const DEFAULT_STORAGE_PATH: &str = "DEFAULT_STORAGE_PATH"; + impl Binder { #[async_backtrace::framed] pub(in crate::planner::binder) async fn bind_show_databases( @@ -214,6 +218,38 @@ impl Binder { database, }, ))), + + AlterDatabaseAction::SetOptions { options } => { + let catalog_arc = self.ctx.get_catalog(&catalog).await?; + let db_exists = catalog_arc.exists_database(&tenant, &database).await?; + if !db_exists && !*if_exists { + return Err(ErrorCode::UnknownDatabase(format!( + "Unknown database '{}'", + database + ))); + } + + // Validate database options only when the database exists. + let db_options = if db_exists { + // For ALTER DATABASE, allow modifying single option (the other already exists) + self.validate_database_options(options, false).await?; + + options + .iter() + .map(|property| (property.name.clone(), property.value.clone())) + .collect::>() + } else { + BTreeMap::new() + }; + + Ok(Plan::AlterDatabase(Box::new(AlterDatabasePlan { + tenant, + catalog, + database, + if_exists: *if_exists, + options: db_options, + }))) + } } } @@ -283,6 +319,10 @@ impl Binder { .unwrap_or_else(|| self.ctx.get_current_catalog()); let database = normalize_identifier(database, &self.name_resolution_ctx).name; + // Validate database options (connection, URI, storage access) + // For CREATE DATABASE, require both options to be specified together + self.validate_database_options(options, true).await?; + let meta = self.database_meta(engine, options)?; Ok(Plan::CreateDatabase(Box::new(CreateDatabasePlan { @@ -294,11 +334,187 @@ impl Binder { }))) } + /// Validate database options including connection existence, URI location, and storage access + /// + /// # Arguments + /// * `options` - The database options to validate + /// * `require_both` - If true (CREATE), both options must be present together. + /// If false (ALTER), allows modifying single option. + #[async_backtrace::framed] + async fn validate_database_options( + &self, + options: &[SQLProperty], + require_both: bool, + ) -> Result<()> { + // Validate database options - only allow specific connection-related options + const VALID_DATABASE_OPTIONS: &[&str] = &[DEFAULT_STORAGE_CONNECTION, DEFAULT_STORAGE_PATH]; + + // Check for duplicate options + let mut seen_options = std::collections::HashSet::new(); + for property in options { + if !seen_options.insert(&property.name) { + return Err(ErrorCode::InvalidArgument(format!( + "Duplicate database option '{}' is not allowed", + property.name + ))); + } + + if !VALID_DATABASE_OPTIONS.contains(&property.name.as_str()) { + return Err(ErrorCode::InvalidArgument(format!( + "Invalid database option '{}'. Valid options are: {}", + property.name, + VALID_DATABASE_OPTIONS.join(", ") + ))); + } + } + + // Validate pairing requirement based on operation type + let has_connection = options.iter().any(|p| p.name == DEFAULT_STORAGE_CONNECTION); + let has_path = options.iter().any(|p| p.name == DEFAULT_STORAGE_PATH); + + if require_both { + // For CREATE DATABASE: both options must be specified together + if has_connection && !has_path { + return Err(ErrorCode::BadArguments(format!( + "{} requires {} to be specified", + DEFAULT_STORAGE_CONNECTION, DEFAULT_STORAGE_PATH + ))); + } + + if has_path && !has_connection { + return Err(ErrorCode::BadArguments(format!( + "{} requires {} to be specified", + DEFAULT_STORAGE_PATH, DEFAULT_STORAGE_CONNECTION + ))); + } + } + // For ALTER DATABASE: allow modifying single option (the other one already exists in database) + + // Validate that the specified connection exists + if let Some(connection_property) = options + .iter() + .find(|p| p.name == DEFAULT_STORAGE_CONNECTION) + { + let connection_name = &connection_property.value; + + // Check if the connection exists by trying to get it through the context + match self.ctx.get_connection(connection_name).await { + Ok(_) => { + // Connection exists, continue + } + Err(_) => { + return Err(ErrorCode::BadArguments(format!( + "Connection '{}' does not exist. Please create the connection first using CREATE CONNECTION", + connection_name + ))); + } + } + } + + // Validate storage path accessibility when both connection and path are specified + if let (Some(connection_prop), Some(path_prop)) = ( + options + .iter() + .find(|p| p.name == DEFAULT_STORAGE_CONNECTION), + options.iter().find(|p| p.name == DEFAULT_STORAGE_PATH), + ) { + // Validate the storage path is accessible and matches the connection protocol + let connection = self.ctx.get_connection(&connection_prop.value).await?; + + let uri_for_scheme = databend_common_ast::ast::UriLocation::from_uri( + path_prop.value.clone(), + BTreeMap::new(), + ) + .map_err(|e| { + ErrorCode::BadArguments(format!( + "Invalid storage path '{}': {}", + path_prop.value, e + )) + })?; + + let path_protocol = uri_for_scheme.protocol.to_ascii_lowercase(); + let connection_protocol = connection.storage_type.to_ascii_lowercase(); + + if path_protocol != connection_protocol { + return Err(ErrorCode::BadArguments(format!( + "{} protocol '{}' does not match connection '{}' protocol '{}'", + DEFAULT_STORAGE_PATH, + uri_for_scheme.protocol, + connection_prop.value, + connection.storage_type + ))); + } + + let mut uri_location = databend_common_ast::ast::UriLocation::from_uri( + path_prop.value.clone(), + connection.storage_params.clone(), + ) + .map_err(|e| { + ErrorCode::BadArguments(format!( + "Invalid storage path '{}': {}", + path_prop.value, e + )) + })?; + + // Parse and validate the URI location using parse_storage_params_from_uri + // This enforces that the path must end with '/' (directory requirement) + let storage_params = crate::binder::parse_storage_params_from_uri( + &mut uri_location, + Some(&*self.ctx), + "when setting database DEFAULT_STORAGE_PATH", + ) + .await + .map_err(|e| { + ErrorCode::BadArguments(format!( + "Invalid storage path '{}': {}", + path_prop.value, e + )) + })?; + + // Check if storage is secure when required + if !storage_params.is_secure() + && !databend_common_config::GlobalConfig::instance() + .storage + .allow_insecure + { + return Err(ErrorCode::StorageInsecure( + "Database default storage path points to insecure storage, which is not allowed" + )); + } + + // Verify essential privileges for the external storage location + // Similar to table creation, we test basic storage operations + let operator = + databend_common_storage::init_operator(&storage_params).map_err(|e| { + ErrorCode::BadArguments(format!( + "Failed to access storage location '{}': {}", + path_prop.value, e + )) + })?; + + // Test storage accessibility with basic operations + // Reuse the existing verify_external_location_privileges function from table.rs + crate::binder::verify_external_location_privileges(operator) + .await + .map_err(|e| { + ErrorCode::BadArguments(format!( + "Failed to verify permissions for database default storage location '{}': {}", + path_prop.value, e + )) + })?; + } + + Ok(()) + } + fn database_meta( &self, engine: &Option, options: &[SQLProperty], ) -> Result { + // Note: Options validation is done in validate_database_options() + // This function only creates the DatabaseMeta structure + let options = options .iter() .map(|property| (property.name.clone(), property.value.clone())) diff --git a/src/query/sql/src/planner/binder/ddl/mod.rs b/src/query/sql/src/planner/binder/ddl/mod.rs index 08615413c9260..dc6f1c1636c5e 100644 --- a/src/query/sql/src/planner/binder/ddl/mod.rs +++ b/src/query/sql/src/planner/binder/ddl/mod.rs @@ -17,7 +17,7 @@ mod catalog; mod column; mod connection; mod data_mask; -mod database; +pub mod database; mod dictionary; mod dynamic_table; mod index; @@ -30,7 +30,7 @@ mod row_access_policy; mod sequence; mod stage; mod stream; -mod table; +pub mod table; mod task; mod view; mod warehouse; diff --git a/src/query/sql/src/planner/binder/ddl/table.rs b/src/query/sql/src/planner/binder/ddl/table.rs index ff03503254eb1..c22e938dd0f19 100644 --- a/src/query/sql/src/planner/binder/ddl/table.rs +++ b/src/query/sql/src/planner/binder/ddl/table.rs @@ -117,6 +117,8 @@ use crate::binder::ConstraintExprBinder; use crate::binder::Visibility; use crate::optimizer::ir::SExpr; use crate::parse_computed_expr_to_string; +use crate::planner::binder::ddl::database::DEFAULT_STORAGE_CONNECTION; +use crate::planner::binder::ddl::database::DEFAULT_STORAGE_PATH; use crate::planner::semantic::normalize_identifier; use crate::planner::semantic::resolve_type_name; use crate::planner::semantic::IdentifierNormalizer; @@ -547,7 +549,59 @@ impl Binder { let catalog = self.ctx.get_catalog(&catalog).await?; + let mut options: BTreeMap = BTreeMap::new(); + + // FUSE tables can inherit database connection defaults for external storage let engine = engine.unwrap_or(catalog.default_table_engine()); + + // Construct a UriLocation from database defaults if table doesn't have explicit location + let uri_location_to_use: Option = if uri_location.is_none() + && matches!(engine, Engine::Fuse) + { + if let Ok(database_info) = catalog + .get_database(&self.ctx.get_tenant(), &database) + .await + { + // Extract database-level default connection options + let default_connection_name = + database_info.options().get(DEFAULT_STORAGE_CONNECTION); + let default_path = database_info.options().get(DEFAULT_STORAGE_PATH); + + // If both database defaults exist, construct UriLocation + if let (Some(connection_name), Some(path)) = (default_connection_name, default_path) + { + // Get the connection object to access its storage_params + match self.ctx.get_connection(connection_name).await { + Ok(connection) => { + // Construct UriLocation using the database defaults + match UriLocation::from_uri(path.clone(), connection.storage_params) { + Ok(uri) => Some(uri), + Err(e) => { + return Err(ErrorCode::BadArguments(format!( + "Failed to parse database default storage path '{}': {}", + path, e + ))); + } + } + } + Err(e) => { + return Err(ErrorCode::BadArguments(format!( + "Database default connection '{}' does not exist: {}", + connection_name, e + ))); + } + } + } else { + None + } + } else { + None + } + } else { + // Use the provided uri_location by cloning it + uri_location.clone() + }; + if catalog.support_partition() != (engine == Engine::Iceberg) { return Err(ErrorCode::TableEngineNotSupported(format!( "Catalog '{}' engine type is {:?} but table {} engine type is {}", @@ -558,8 +612,8 @@ impl Binder { ))); } - let mut options: BTreeMap = BTreeMap::new(); let mut engine_options: BTreeMap = BTreeMap::new(); + // Table-specific options override database defaults for table_option in table_options.iter() { self.insert_table_option_with_validation( &mut options, @@ -590,7 +644,7 @@ impl Binder { .collect::>() }); - let mut storage_params = match (uri_location, engine) { + let mut storage_params = match (uri_location_to_use.as_ref(), engine) { (Some(uri), Engine::Fuse) => { let mut uri = UriLocation { protocol: uri.protocol.clone(), @@ -2207,7 +2261,7 @@ const VERIFICATION_KEY_DEL: &str = "_v_d77aa11285c22e0e1d4593a035c98c0d_del"; // // The permission check might fail for reasons other than the permissions themselves, // such as network communication issues. -async fn verify_external_location_privileges(dal: Operator) -> Result<()> { +pub async fn verify_external_location_privileges(dal: Operator) -> Result<()> { let verification_task = async move { // verify privilege to put let mut errors = Vec::new(); diff --git a/src/query/sql/src/planner/binder/mod.rs b/src/query/sql/src/planner/binder/mod.rs index 2b96a0ecd239d..4a389f4695fa1 100644 --- a/src/query/sql/src/planner/binder/mod.rs +++ b/src/query/sql/src/planner/binder/mod.rs @@ -27,7 +27,7 @@ mod column_binding; mod constraint_expr; mod copy_into_location; mod copy_into_table; -mod ddl; +pub mod ddl; mod default_expr; mod distinct; mod explain; @@ -78,6 +78,9 @@ pub use constraint_expr::ConstraintExprBinder; pub use copy_into_table::resolve_file_location; pub use copy_into_table::resolve_stage_location; pub use copy_into_table::resolve_stage_locations; +pub use ddl::database::DEFAULT_STORAGE_CONNECTION; +pub use ddl::database::DEFAULT_STORAGE_PATH; +pub use ddl::table::verify_external_location_privileges; pub use default_expr::DefaultExprBinder; pub use explain::ExplainConfig; pub use internal_column_factory::INTERNAL_COLUMN_FACTORY; diff --git a/src/query/sql/src/planner/format/display_plan.rs b/src/query/sql/src/planner/format/display_plan.rs index 596384f93e1f5..edfb66eea6a3d 100644 --- a/src/query/sql/src/planner/format/display_plan.rs +++ b/src/query/sql/src/planner/format/display_plan.rs @@ -274,6 +274,7 @@ impl Plan { Plan::SetWorkloadGroupQuotas(_) => Ok("SetWorkloadGroupQuotas".to_string()), Plan::UnsetWorkloadGroupQuotas(_) => Ok("UnsetWorkloadGroupQuotas".to_string()), Plan::AlterRole(_) => Ok("AlterRole".to_string()), + Plan::AlterDatabase(_) => Ok("AlterDatabase".to_string()), } } } diff --git a/src/query/sql/src/planner/plans/ddl/database.rs b/src/query/sql/src/planner/plans/ddl/database.rs index eae2df5e4b8fa..ee57b5c8c30b0 100644 --- a/src/query/sql/src/planner/plans/ddl/database.rs +++ b/src/query/sql/src/planner/plans/ddl/database.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::BTreeMap; use std::sync::Arc; use databend_common_expression::DataSchema; @@ -154,3 +155,18 @@ impl RefreshDatabaseCachePlan { Arc::new(DataSchema::empty()) } } + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct AlterDatabasePlan { + pub tenant: Tenant, + pub catalog: String, + pub database: String, + pub if_exists: bool, + pub options: BTreeMap, +} + +impl AlterDatabasePlan { + pub fn schema(&self) -> DataSchemaRef { + Arc::new(DataSchema::empty()) + } +} diff --git a/src/query/sql/src/planner/plans/plan.rs b/src/query/sql/src/planner/plans/plan.rs index c22570ee71347..dfff0d48c70c1 100644 --- a/src/query/sql/src/planner/plans/plan.rs +++ b/src/query/sql/src/planner/plans/plan.rs @@ -38,6 +38,7 @@ use crate::plans::AddTableColumnPlan; use crate::plans::AddTableConstraintPlan; use crate::plans::AddTableRowAccessPolicyPlan; use crate::plans::AddWarehouseClusterPlan; +use crate::plans::AlterDatabasePlan; use crate::plans::AlterNetworkPolicyPlan; use crate::plans::AlterNotificationPlan; use crate::plans::AlterPasswordPolicyPlan; @@ -261,6 +262,7 @@ pub enum Plan { RenameDatabase(Box), UseDatabase(Box), RefreshDatabaseCache(Box), + AlterDatabase(Box), // Tables ShowCreateTable(Box), diff --git a/tests/sqllogictests/suites/base/05_ddl/05_0001_ddl_create_database_options.test b/tests/sqllogictests/suites/base/05_ddl/05_0001_ddl_create_database_options.test new file mode 100644 index 0000000000000..74f2acdae1937 --- /dev/null +++ b/tests/sqllogictests/suites/base/05_ddl/05_0001_ddl_create_database_options.test @@ -0,0 +1,203 @@ +# Test database-level default connection options + +# First create the required connection +statement ok +DROP CONNECTION IF EXISTS my_s3_connection + +statement ok +CREATE CONNECTION IF NOT EXISTS my_s3_connection STORAGE_TYPE = 'fs' + +statement ok +DROP DATABASE IF EXISTS test_db_options + +statement ok +CREATE DATABASE test_db_options OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'my_s3_connection', + DEFAULT_STORAGE_PATH = 'fs:///tmp/test-bucket/data/' +) + +query I +show create database test_db_options +---- +test_db_options CREATE DATABASE `test_db_options` ENGINE=DEFAULT OPTIONS (DEFAULT_STORAGE_CONNECTION='my_s3_connection', DEFAULT_STORAGE_PATH='fs:///tmp/test-bucket/data/') + +# Test ALTER DATABASE SET OPTIONS with valid paired options +statement ok +ALTER DATABASE test_db_options SET OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'my_s3_connection', + DEFAULT_STORAGE_PATH = 'fs:///tmp/updated-bucket/path/' +) + +# Test ALTER DATABASE with invalid DEFAULT_STORAGE_CONNECTION which does not exist +# ERROR HY000 (1105): BadArguments. Code: 1006, Text = Connection 'only_connection' does not exist. Please create the connection first using CREATE CONNECTION. +statement error 1006 +ALTER DATABASE test_db_options SET OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'only_connection' +) + +# Cleanup +statement ok +DROP DATABASE test_db_options + +# Test CREATE DATABASE with both ENGINE and OPTIONS +# Create another connection for this test +statement ok +DROP CONNECTION IF EXISTS my_connection + +statement ok +CREATE CONNECTION IF NOT EXISTS my_connection STORAGE_TYPE = 'fs' + +statement ok +CREATE OR REPLACE DATABASE test_db_mixed ENGINE = DEFAULT OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'my_connection', + DEFAULT_STORAGE_PATH = 'fs:///tmp/mixed_path/' +) + +query I +show create database test_db_mixed +---- +test_db_mixed CREATE DATABASE `test_db_mixed` ENGINE=DEFAULT OPTIONS (DEFAULT_STORAGE_CONNECTION='my_connection', DEFAULT_STORAGE_PATH='fs:///tmp/mixed_path/') + +statement ok +DROP DATABASE test_db_mixed + + +# Test invalid database option - should fail +# ERROR HY000 (1105): InvalidArgument. Code: 2004, Text = Invalid database option 'invalid_option'. Valid options are: DEFAULT_STORAGE_CONNECTION, DEFAULT_STORAGE_PATH. +statement error 2004 +CREATE DATABASE test_db_invalid OPTIONS ( + invalid_option = 'test' +) + +# Test mismatched connection and path protocols - should fail +statement error 1006 +CREATE DATABASE test_db_path_mismatch OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'my_connection', + DEFAULT_STORAGE_PATH = 's3://test-bucket/mismatch/' +) + +# Test invalid path format - should fail +statement error 1006 +CREATE DATABASE test_db_invalid_url OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'my_connection', + DEFAULT_STORAGE_PATH = 'not a valid url' +) + +# Test mixed invalid database options +# ERROR HY000 (1105): InvalidArgument. Code: 2004, Text = Invalid database option 'conn'. Valid options are: DEFAULT_STORAGE_CONNECTION, DEFAULT_STORAGE_PATH. +statement error 2004 +CREATE DATABASE test_db_invalid2 OPTIONS ( + conn = 'test', + path = 'fs:///tmp/path', + DEFAULT_STORAGE_CONNECTION = 'my_conn', + DEFAULT_STORAGE_PATH = 'fs:///tmp/bucket/path' +) + +# ALTER DATABASE IF EXISTS should be a no-op for missing database +statement ok +ALTER DATABASE IF EXISTS missing_db SET OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'my_connection', + DEFAULT_STORAGE_PATH = 'fs:///tmp/if_exists_check/' +) + +# Test incomplete options - DEFAULT_STORAGE_CONNECTION without DEFAULT_STORAGE_PATH +# ERROR HY000 (1105): BadArguments. Code: 1006, Text = DEFAULT_STORAGE_CONNECTION requires DEFAULT_STORAGE_PATH to be specified. +statement error 1006 +CREATE DATABASE test_db_incomplete1 OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'my_conn' +) + +# Test incomplete options - DEFAULT_STORAGE_PATH without DEFAULT_STORAGE_CONNECTION +statement error 1006 +CREATE DATABASE test_db_incomplete2 OPTIONS ( + DEFAULT_STORAGE_PATH = 'fs:///tmp/bucket/path' +) + +# Test ALTER DATABASE options +# Create connection for this test +statement ok +CREATE CONNECTION IF NOT EXISTS temp_test STORAGE_TYPE = 'fs' + +statement ok +CREATE OR REPLACE DATABASE temp_db OPTIONS (DEFAULT_STORAGE_CONNECTION = 'temp_test', DEFAULT_STORAGE_PATH = 'fs:///tmp/test_db/') + +# ALTER with single option should now succeed (the other option already exists in database) +statement ok +ALTER DATABASE temp_db SET OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'temp_test' +) + +statement ok +CREATE OR REPLACE DATABASE temp_db + +# ALTER with single option should now fail (the other option does not exist in database) +# ERROR HY000 (1105): BadArguments. Code: 1006, Text = DEFAULT_STORAGE_CONNECTION and DEFAULT_STORAGE_PATH options must be used together. +statement error 1006 +ALTER DATABASE temp_db SET OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'temp_test' +) + +statement ok +DROP DATABASE temp_db + +# Test ALTER DATABASE can modify single option (the other already exists in database) +statement ok +CREATE CONNECTION IF NOT EXISTS alter_test_conn STORAGE_TYPE = 'fs' + +statement ok +CREATE OR REPLACE DATABASE test_alter_single OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'alter_test_conn', + DEFAULT_STORAGE_PATH = 'fs:///tmp/original_path/' +) + +# Alter only connection - should succeed now +statement ok +ALTER DATABASE test_alter_single SET OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'alter_test_conn' +) + +# Alter only path - should succeed now +statement ok +ALTER DATABASE test_alter_single SET OPTIONS ( + DEFAULT_STORAGE_PATH = 'fs:///tmp/new_path/' +) + +# Test with non-existent connection +statement ok +DROP DATABASE IF EXISTS test_db_nonexistent_conn + +statement error 1006 +CREATE DATABASE test_db_nonexistent_conn OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'nonexistent_connection', + DEFAULT_STORAGE_PATH = 'fs:///tmp/test-path/' +) + +# Test duplicate options - should fail +statement error 2004 +CREATE DATABASE test_db_duplicate OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'test', + DEFAULT_STORAGE_CONNECTION = 'test2', + DEFAULT_STORAGE_PATH = 'fs:///tmp/dup/' +) + +statement error 2004 +CREATE DATABASE test_db_duplicate2 OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'test', + DEFAULT_STORAGE_PATH = 'fs:///tmp/dup1/', + DEFAULT_STORAGE_PATH = 'fs:///tmp/dup2/' +) + +statement ok +DROP DATABASE test_alter_single + +statement ok +DROP CONNECTION IF EXISTS my_connection + +statement ok +DROP CONNECTION IF EXISTS my_s3_connection + +statement ok +DROP CONNECTION IF EXISTS alter_test_conn + +statement ok +DROP CONNECTION IF EXISTS temp_test diff --git a/tests/sqllogictests/suites/base/05_ddl/05_0002_ddl_table_inherit_database_defaults.test b/tests/sqllogictests/suites/base/05_ddl/05_0002_ddl_table_inherit_database_defaults.test new file mode 100644 index 0000000000000..5d992bff295e4 --- /dev/null +++ b/tests/sqllogictests/suites/base/05_ddl/05_0002_ddl_table_inherit_database_defaults.test @@ -0,0 +1,76 @@ +# Test table creation inheriting database default connection options + +statement ok +DROP DATABASE IF EXISTS test_db_inherit + +# First create the required connection +statement ok +CREATE CONNECTION IF NOT EXISTS default_conn STORAGE_TYPE = 'fs' + +# Create database with default connection options +statement ok +CREATE DATABASE test_db_inherit OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'default_conn', + DEFAULT_STORAGE_PATH = 'fs:///tmp/05_0002/' +) + +# Create a table without explicit connection options +# This table should inherit the database defaults +statement ok +CREATE TABLE test_db_inherit.table_with_defaults ( + id INT, + name VARCHAR(100) +) ENGINE = Fuse + +# Verify table metadata inherits the database defaults +query I +SELECT storage_param FROM system.tables WHERE database = 'test_db_inherit' AND name = 'table_with_defaults' +---- +fs | root=/tmp/05_0002/ + +# Alter database defaults and ensure new tables pick up the updated settings +statement ok +ALTER DATABASE test_db_inherit SET OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'default_conn', + DEFAULT_STORAGE_PATH = 'fs:///tmp/05_0002_updated/' +) + +statement ok +CREATE TABLE test_db_inherit.table_after_alter ( + id INT, + name VARCHAR(100) +) ENGINE = Fuse + +query I +SELECT storage_param FROM system.tables WHERE database = 'test_db_inherit' AND name = 'table_after_alter' +---- +fs | root=/tmp/05_0002_updated/ + +# Create a table with explicit location +# This should override the database defaults +statement ok +DROP CONNECTION IF EXISTS explicit_conn + +statement ok +CREATE CONNECTION IF NOT EXISTS explicit_conn STORAGE_TYPE = 'fs' + +statement ok +CREATE TABLE test_db_inherit.table_with_explicit ( + id INT, + name VARCHAR(100) +) ENGINE = Fuse 'fs:///tmp/05_0002_explicit/' CONNECTION = (connection_name = 'explicit_conn') + +query I +SELECT storage_param FROM system.tables WHERE database = 'test_db_inherit' AND name = 'table_with_explicit' +---- +fs | root=/tmp/05_0002_explicit/ + +# Cleanup +statement ok +DROP DATABASE test_db_inherit + +statement ok +DROP CONNECTION IF EXISTS explicit_conn + +statement ok +DROP CONNECTION IF EXISTS default_conn diff --git a/tests/sqllogictests/suites/base/05_ddl/05_0003_ddl_database_connection_validation.test b/tests/sqllogictests/suites/base/05_ddl/05_0003_ddl_database_connection_validation.test new file mode 100644 index 0000000000000..c693e00cf8dbc --- /dev/null +++ b/tests/sqllogictests/suites/base/05_ddl/05_0003_ddl_database_connection_validation.test @@ -0,0 +1,85 @@ +# Test connection validation for database creation + +# Clean up any existing test data +statement ok +DROP DATABASE IF EXISTS test_db_nonexistent_conn + +statement ok +DROP DATABASE IF EXISTS test_db_valid_conn + +statement ok +DROP CONNECTION IF EXISTS test_conn_05_0003 + +# Create a test connection using local filesystem +statement ok +CREATE CONNECTION test_conn_05_0003 STORAGE_TYPE='fs' + +# Test creating database with valid connection (should succeed) +statement ok +CREATE DATABASE test_db_valid_conn OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'test_conn_05_0003', + DEFAULT_STORAGE_PATH = 'fs:///tmp/test-bucket/path/' +) + +# Test creating database with non-existent connection (should fail) +statement error 1006 +CREATE DATABASE test_db_nonexistent_conn OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'nonexistent_connection', + DEFAULT_STORAGE_PATH = 'fs:///tmp/test-bucket/path/' +) + +# Test protocol mismatch between connection and path (should fail) +statement ok +CREATE CONNECTION test_conn_05_0003_s3_05_0003 STORAGE_TYPE='s3' ENDPOINT_URL='http://127.0.0.1:9000' ACCESS_KEY_ID='test' SECRET_ACCESS_KEY='test' + +statement error 1006 +CREATE DATABASE test_db_protocol_mismatch OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'test_conn_05_0003_s3_05_0003', + DEFAULT_STORAGE_PATH = 'fs:///tmp/mismatch/' +) + +# Test ALTER DATABASE detects protocol mismatch when only path is changed +statement ok +DROP DATABASE IF EXISTS test_db_alter_protocol + +statement ok +DROP CONNECTION IF EXISTS test_conn_05_0003_alter + +statement ok +CREATE CONNECTION test_conn_05_0003_alter STORAGE_TYPE='fs' + +statement ok +CREATE DATABASE test_db_alter_protocol OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'test_conn_05_0003_alter', + DEFAULT_STORAGE_PATH = 'fs:///tmp/test-bucket/alter/' +) + +# ERROR HY000 (1105): BadArguments. Code: 1006, Text = DEFAULT_STORAGE_PATH protocol 's3' does not match connection 'test_conn_05_0003_alter' protocol 'fs'. +statement error 1006 +ALTER DATABASE test_db_alter_protocol SET OPTIONS ( + DEFAULT_STORAGE_PATH = 's3://test-bucket/mismatch/' +) + +# Cleanup database before continuing +statement ok +DROP DATABASE IF EXISTS test_db_alter_protocol + +statement ok +DROP CONNECTION IF EXISTS test_conn_05_0003_alter + +# Test ALTER DATABASE with non-existent connection (should fail) +statement error 1006 +ALTER DATABASE test_db_valid_conn SET OPTIONS ( + DEFAULT_STORAGE_CONNECTION = 'another_nonexistent_connection', + DEFAULT_STORAGE_PATH = 's3://test-bucket/path/' +) + +# Cleanup +statement ok +DROP DATABASE test_db_valid_conn + +statement ok +DROP CONNECTION test_conn_05_0003 + +statement ok +DROP CONNECTION test_conn_05_0003_s3_05_0003