diff --git a/src/query/ast/src/ast/statements/table.rs b/src/query/ast/src/ast/statements/table.rs index 7efc87fa6b8f..34a89d4a83d0 100644 --- a/src/query/ast/src/ast/statements/table.rs +++ b/src/query/ast/src/ast/statements/table.rs @@ -745,6 +745,21 @@ impl Display for Engine { } } +impl From<&str> for Engine { + fn from(s: &str) -> Self { + match s.to_lowercase().as_str() { + "null" => Engine::Null, + "memory" => Engine::Memory, + "fuse" => Engine::Fuse, + "view" => Engine::View, + "random" => Engine::Random, + "iceberg" => Engine::Iceberg, + "delta" => Engine::Delta, + _ => unreachable!("invalid engine: {}", s), + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, Drive, DriveMut)] pub enum CompactTarget { Block, diff --git a/src/query/service/src/interpreters/common/table_option_validation.rs b/src/query/service/src/interpreters/common/table_option_validation.rs index a36a217ac8b0..b16132635d8c 100644 --- a/src/query/service/src/interpreters/common/table_option_validation.rs +++ b/src/query/service/src/interpreters/common/table_option_validation.rs @@ -17,6 +17,7 @@ use std::collections::HashSet; use std::sync::LazyLock; use chrono::Duration; +use databend_common_ast::ast::Engine; use databend_common_exception::ErrorCode; use databend_common_expression::TableSchemaRef; use databend_common_io::constants::DEFAULT_BLOCK_MAX_ROWS; @@ -45,7 +46,7 @@ use databend_storages_common_table_meta::table::OPT_KEY_TEMP_PREFIX; use log::error; /// Table option keys that can occur in 'create table statement'. -pub static CREATE_TABLE_OPTIONS: LazyLock> = LazyLock::new(|| { +pub static CREATE_FUSE_OPTIONS: LazyLock> = LazyLock::new(|| { let mut r = HashSet::new(); r.insert(FUSE_OPT_KEY_ROW_PER_PAGE); r.insert(FUSE_OPT_KEY_BLOCK_PER_SEGMENT); @@ -64,12 +65,32 @@ pub static CREATE_TABLE_OPTIONS: LazyLock> = LazyLock::new r.insert(OPT_KEY_ENGINE); + r.insert(OPT_KEY_CONNECTION_NAME); + + r.insert("transient"); + r.insert(OPT_KEY_TEMP_PREFIX); + r +}); + +pub static CREATE_LAKE_OPTIONS: LazyLock> = LazyLock::new(|| { + let mut r = HashSet::new(); + r.insert(OPT_KEY_ENGINE); r.insert(OPT_KEY_LOCATION); r.insert(OPT_KEY_CONNECTION_NAME); + r +}); +pub static CREATE_RANDOM_OPTIONS: LazyLock> = LazyLock::new(|| { + let mut r = HashSet::new(); + r.insert(OPT_KEY_ENGINE); r.insert(OPT_KEY_RANDOM_SEED); + r +}); - r.insert("transient"); +pub static CREATE_MEMORY_OPTIONS: LazyLock> = LazyLock::new(|| { + let mut r = HashSet::new(); + r.insert(OPT_KEY_ENGINE); + r.insert(OPT_KEY_DATABASE_ID); r.insert(OPT_KEY_TEMP_PREFIX); r }); @@ -85,8 +106,16 @@ pub static UNSET_TABLE_OPTIONS_WHITE_LIST: LazyLock> = Laz r }); -pub fn is_valid_create_opt>(opt_key: S) -> bool { - CREATE_TABLE_OPTIONS.contains(opt_key.as_ref().to_lowercase().as_str()) +pub fn is_valid_create_opt>(opt_key: S, engine: &Engine) -> bool { + let opt_key = opt_key.as_ref().to_lowercase(); + let opt_key = opt_key.as_str(); + match engine { + Engine::Fuse => CREATE_FUSE_OPTIONS.contains(opt_key), + Engine::Iceberg | Engine::Delta => CREATE_LAKE_OPTIONS.contains(&opt_key), + Engine::Random => CREATE_RANDOM_OPTIONS.contains(&opt_key), + Engine::Memory => CREATE_MEMORY_OPTIONS.contains(&opt_key), + Engine::Null | Engine::View => opt_key == OPT_KEY_ENGINE, + } } pub fn is_valid_block_per_segment( diff --git a/src/query/service/src/interpreters/interpreter_table_create.rs b/src/query/service/src/interpreters/interpreter_table_create.rs index f15937f892cc..173d891b5d0c 100644 --- a/src/query/service/src/interpreters/interpreter_table_create.rs +++ b/src/query/service/src/interpreters/interpreter_table_create.rs @@ -402,7 +402,7 @@ impl CreateTableInterpreter { for table_option in table_meta.options.iter() { let key = table_option.0.to_lowercase(); - if !is_valid_create_opt(&key) { + if !is_valid_create_opt(&key, &self.plan.engine) { error!("invalid opt for fuse table in create table statement"); return Err(ErrorCode::TableOptionInvalid(format!( "table option {key} is invalid for create table statement", diff --git a/src/query/service/src/interpreters/interpreter_table_set_options.rs b/src/query/service/src/interpreters/interpreter_table_set_options.rs index 9aa7e26449fc..2147a607ae16 100644 --- a/src/query/service/src/interpreters/interpreter_table_set_options.rs +++ b/src/query/service/src/interpreters/interpreter_table_set_options.rs @@ -15,6 +15,7 @@ use std::collections::HashMap; use std::sync::Arc; +use databend_common_ast::ast::Engine; use databend_common_catalog::table::TableExt; use databend_common_exception::ErrorCode; use databend_common_exception::Result; @@ -101,9 +102,17 @@ impl Interpreter for SetOptionsInterpreter { OPT_KEY_CLUSTER_TYPE ))); } + let catalog = self.ctx.get_catalog(self.plan.catalog.as_str()).await?; + let database = self.plan.database.as_str(); + let table_name = self.plan.table.as_str(); + let table = catalog + .get_table(&self.ctx.get_tenant(), database, table_name) + .await?; + for table_option in self.plan.set_options.iter() { let key = table_option.0.to_lowercase(); - if !is_valid_create_opt(&key) { + let engine = Engine::from(table.engine()); + if !is_valid_create_opt(&key, &engine) { error!("{}", &error_str); return Err(ErrorCode::TableOptionInvalid(format!( "table option {key} is invalid for alter table statement", @@ -111,12 +120,6 @@ impl Interpreter for SetOptionsInterpreter { } options_map.insert(key, Some(table_option.1.clone())); } - let catalog = self.ctx.get_catalog(self.plan.catalog.as_str()).await?; - let database = self.plan.database.as_str(); - let table_name = self.plan.table.as_str(); - let table = catalog - .get_table(&self.ctx.get_tenant(), database, table_name) - .await?; let table_version = table.get_table_info().ident.seq; if let Some(value) = self.plan.set_options.get(OPT_KEY_CHANGE_TRACKING) { diff --git a/src/query/service/src/interpreters/interpreter_table_show_create.rs b/src/query/service/src/interpreters/interpreter_table_show_create.rs index 8021e1d6f2ac..1b69d3ef4f81 100644 --- a/src/query/service/src/interpreters/interpreter_table_show_create.rs +++ b/src/query/service/src/interpreters/interpreter_table_show_create.rs @@ -281,7 +281,7 @@ impl ShowCreateTableInterpreter { if engine != "ICEBERG" && engine != "DELTA" { if let Some(sp) = &table_info.meta.storage_params { - table_create_sql.push_str(format!(" LOCATION = '{}'", sp).as_str()); + table_create_sql.push_str(format!(" '{}' ", sp).as_str()); } } diff --git a/tests/sqllogictests/suites/base/05_ddl/05_0000_ddl_create_tables.test b/tests/sqllogictests/suites/base/05_ddl/05_0000_ddl_create_tables.test index c3d9cd3649ed..49b3ff94ca67 100644 --- a/tests/sqllogictests/suites/base/05_ddl/05_0000_ddl_create_tables.test +++ b/tests/sqllogictests/suites/base/05_ddl/05_0000_ddl_create_tables.test @@ -303,6 +303,15 @@ create table t(a int) x=x statement error 1301 create table t(a int) external_location='xxx' +statement error 1301 +create table t(a int) location='xxx' + +statement error 1301 +create table t(a int) seed='123' + +statement ok +create table t_random(a int) engine=RANDOM seed='123' + statement error 1301 create table t(a int) snapshot_loc='xxx' diff --git a/tests/sqllogictests/suites/base/06_show/06_0001_show_create_table.test b/tests/sqllogictests/suites/base/06_show/06_0001_show_create_table.test index 2020b3db6d84..558d5f33b593 100644 --- a/tests/sqllogictests/suites/base/06_show/06_0001_show_create_table.test +++ b/tests/sqllogictests/suites/base/06_show/06_0001_show_create_table.test @@ -53,7 +53,7 @@ CREATE TABLE test.d (a int not null) ENGINE=FUSE 'fs:///tmp/load/files/' CONNECT query TT SHOW CREATE TABLE `test`.`d` ---- -d CREATE TABLE d ( a INT NOT NULL ) ENGINE=FUSE CLUSTER BY linear(a, a % 3) CLUSTER_TYPE='linear' COMPRESSION='lz4' STORAGE_FORMAT='parquet' LOCATION = 'fs | root=/tmp/load/files/' +d CREATE TABLE d ( a INT NOT NULL ) ENGINE=FUSE CLUSTER BY linear(a, a % 3) CLUSTER_TYPE='linear' COMPRESSION='lz4' STORAGE_FORMAT='parquet' 'fs | root=/tmp/load/files/' query TT SHOW CREATE TABLE `test`.`c`