From bada75ea30dfae8084c48d4aaa354170b39d5825 Mon Sep 17 00:00:00 2001 From: Li Yazhou Date: Mon, 28 Apr 2025 14:29:57 +0800 Subject: [PATCH 1/7] validate set settings name on alter taks set --- .../interpreters/interpreter_task_alter.rs | 19 +++++++++++++++++++ .../interpreters/interpreter_task_create.rs | 11 +++++++++++ src/query/settings/src/lib.rs | 1 + 3 files changed, 31 insertions(+) diff --git a/src/query/service/src/interpreters/interpreter_task_alter.rs b/src/query/service/src/interpreters/interpreter_task_alter.rs index 9ec9835df4a1b..c6942097da26d 100644 --- a/src/query/service/src/interpreters/interpreter_task_alter.rs +++ b/src/query/service/src/interpreters/interpreter_task_alter.rs @@ -26,6 +26,8 @@ use databend_common_cloud_control::pb::WarehouseOptions; use databend_common_config::GlobalConfig; use databend_common_exception::ErrorCode; use databend_common_exception::Result; +use databend_common_settings::DefaultSettings; +use databend_common_settings::SettingScope; use databend_common_sql::plans::AlterTaskPlan; use crate::interpreters::common::get_task_client_config; @@ -137,6 +139,22 @@ impl AlterTaskInterpreter { } req } + + fn validate_session_parameters(&self) -> Result<()> { + match &self.plan.alter_options { + AlterTaskOptions::Set { + session_parameters, .. + } => { + if let Some(session_parameters) = session_parameters { + for (key, _) in session_parameters.iter() { + DefaultSettings::check_setting_scope(key, SettingScope::Session)?; + } + } + } + _ => {} + } + Ok(()) + } } #[async_trait::async_trait] @@ -158,6 +176,7 @@ impl Interpreter for AlterTaskInterpreter { "cannot alter task without cloud control enabled, please set cloud_control_grpc_server_address in config", )); } + self.validate_session_parameters()?; let cloud_api = CloudControlApiProvider::instance(); let task_client = cloud_api.get_task_client(); let req = self.build_request(); diff --git a/src/query/service/src/interpreters/interpreter_task_create.rs b/src/query/service/src/interpreters/interpreter_task_create.rs index cbcd285a3b9a5..a0e4253648656 100644 --- a/src/query/service/src/interpreters/interpreter_task_create.rs +++ b/src/query/service/src/interpreters/interpreter_task_create.rs @@ -23,6 +23,8 @@ use databend_common_cloud_control::pb::CreateTaskRequest; use databend_common_config::GlobalConfig; use databend_common_exception::ErrorCode; use databend_common_exception::Result; +use databend_common_settings::DefaultSettings; +use databend_common_settings::SettingScope; use databend_common_sql::plans::CreateTaskPlan; use crate::interpreters::common::get_task_client_config; @@ -83,6 +85,14 @@ impl CreateTaskInterpreter { } req } + + fn validate_session_parameters(&self) -> Result<()> { + let session_parameters = self.plan.session_parameters.clone(); + for (key, _) in session_parameters.iter() { + DefaultSettings::check_setting_scope(key, SettingScope::Session)?; + } + Ok(()) + } } #[async_trait::async_trait] @@ -104,6 +114,7 @@ impl Interpreter for CreateTaskInterpreter { "cannot create task without cloud control enabled, please set cloud_control_grpc_server_address in config", )); } + self.validate_session_parameters()?; let cloud_api = CloudControlApiProvider::instance(); let task_client = cloud_api.get_task_client(); let req = self.build_request(); diff --git a/src/query/settings/src/lib.rs b/src/query/settings/src/lib.rs index 5ef08127ba28f..c7f284ce09e01 100644 --- a/src/query/settings/src/lib.rs +++ b/src/query/settings/src/lib.rs @@ -20,6 +20,7 @@ mod settings_global; pub use settings::ChangeValue; pub use settings::ScopeLevel; pub use settings::Settings; +pub use settings_default::DefaultSettings; pub use settings_default::ReplaceIntoShuffleStrategy; pub use settings_default::SettingMode; pub use settings_default::SettingRange; From 16c07fb36f7e6a2bb1066174c9cbd918da91b6aa Mon Sep 17 00:00:00 2001 From: Li Yazhou Date: Mon, 28 Apr 2025 14:35:17 +0800 Subject: [PATCH 2/7] add bad case --- tests/sqllogictests/suites/task/task_ddl_test.test | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/sqllogictests/suites/task/task_ddl_test.test b/tests/sqllogictests/suites/task/task_ddl_test.test index 24807506bba96..8da53b3f6ebb8 100644 --- a/tests/sqllogictests/suites/task/task_ddl_test.test +++ b/tests/sqllogictests/suites/task/task_ddl_test.test @@ -13,6 +13,13 @@ CREATE TASK mytask SCHEDULE = USING CRON '0 0 0 1 1 ? 2100' AS SELECT 1; +statement ok +CREATE TASK task_with_bad_set + WAREHOUSE = 'mywh' + SCHEDULE = USING CRON '0 0 0 1 1 ? 2100' + settings_not_found = '123' + AS SELECT 1; + query SSSS select name, warehouse, schedule, definition from system.tasks where name = 'mytask' ---- From b1ef4b93aa74bdd4ab8e2bb308a553b064fd4fb3 Mon Sep 17 00:00:00 2001 From: Li Yazhou Date: Mon, 28 Apr 2025 15:13:39 +0800 Subject: [PATCH 3/7] fix clippy --- .../src/interpreters/interpreter_task_alter.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/query/service/src/interpreters/interpreter_task_alter.rs b/src/query/service/src/interpreters/interpreter_task_alter.rs index c6942097da26d..2f45da642685e 100644 --- a/src/query/service/src/interpreters/interpreter_task_alter.rs +++ b/src/query/service/src/interpreters/interpreter_task_alter.rs @@ -141,17 +141,14 @@ impl AlterTaskInterpreter { } fn validate_session_parameters(&self) -> Result<()> { - match &self.plan.alter_options { - AlterTaskOptions::Set { - session_parameters, .. - } => { - if let Some(session_parameters) = session_parameters { - for (key, _) in session_parameters.iter() { - DefaultSettings::check_setting_scope(key, SettingScope::Session)?; - } - } + if let AlterTaskOptions::Set { + session_parameters: Some(session_parameters), + .. + } = &self.plan.alter_options + { + for (key, _) in session_parameters.iter() { + DefaultSettings::check_setting_scope(key, SettingScope::Session)?; } - _ => {} } Ok(()) } From 7f41fc25a90edcfdd3742fe1810c1b78269a7318 Mon Sep 17 00:00:00 2001 From: Li Yazhou Date: Mon, 28 Apr 2025 15:42:01 +0800 Subject: [PATCH 4/7] fix logic test --- tests/sqllogictests/suites/task/task_ddl_test.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sqllogictests/suites/task/task_ddl_test.test b/tests/sqllogictests/suites/task/task_ddl_test.test index 8da53b3f6ebb8..9f220a8aa10a5 100644 --- a/tests/sqllogictests/suites/task/task_ddl_test.test +++ b/tests/sqllogictests/suites/task/task_ddl_test.test @@ -13,7 +13,7 @@ CREATE TASK mytask SCHEDULE = USING CRON '0 0 0 1 1 ? 2100' AS SELECT 1; -statement ok +statement error 2801 CREATE TASK task_with_bad_set WAREHOUSE = 'mywh' SCHEDULE = USING CRON '0 0 0 1 1 ? 2100' From 3b3aa6b39944e950af5f48b34087140aaedad8cb Mon Sep 17 00:00:00 2001 From: Li Yazhou Date: Mon, 28 Apr 2025 16:10:52 +0800 Subject: [PATCH 5/7] fix --- tests/sqllogictests/suites/task/task_ddl_test.test | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/sqllogictests/suites/task/task_ddl_test.test b/tests/sqllogictests/suites/task/task_ddl_test.test index 9f220a8aa10a5..95f8327135d06 100644 --- a/tests/sqllogictests/suites/task/task_ddl_test.test +++ b/tests/sqllogictests/suites/task/task_ddl_test.test @@ -107,8 +107,7 @@ SUCCEEDED statement ok CREATE TASK sessionTask WAREHOUSE = 'mywh' - SCHEDULE = USING CRON '0 0 0 1 1 ? 2100' - DATABASE = 'mydb', TIMEZONE = 'America/Los_Angeles' + SCHEDULE = USING CRON '0 0 0 1 1 ? 2100' 'America/Los_Angeles' AS SELECT 1; query SSS From 571cf726b0f689d64b97cdf69faaa09ef986d114 Mon Sep 17 00:00:00 2001 From: Li Yazhou Date: Tue, 6 May 2025 13:28:02 +0800 Subject: [PATCH 6/7] fix test --- tests/sqllogictests/suites/task/task_ddl_test.test | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/sqllogictests/suites/task/task_ddl_test.test b/tests/sqllogictests/suites/task/task_ddl_test.test index 95f8327135d06..1eea1bc5d235d 100644 --- a/tests/sqllogictests/suites/task/task_ddl_test.test +++ b/tests/sqllogictests/suites/task/task_ddl_test.test @@ -113,7 +113,7 @@ CREATE TASK sessionTask query SSS select name, state, session_parameters from system.tasks where name = 'sessionTask' ---- -sessionTask Suspended {"database":"mydb","timezone":"America/Los_Angeles"} +sessionTask Suspended {} statement ok EXECUTE TASK sessionTask @@ -121,7 +121,7 @@ EXECUTE TASK sessionTask query SSSS select name, session_parameters from system.task_history where name = 'sessionTask' ---- -sessionTask {"database":"mydb","timezone":"America/Los_Angeles"} +sessionTask {} statement ok ALTER TASK sessionTask @@ -131,7 +131,7 @@ ALTER TASK sessionTask query SSS select name, state, session_parameters from system.tasks where name = 'sessionTask' ---- -sessionTask Suspended {"database":"db2","timezone":"Pacific/Honolulu"} +sessionTask Suspended {} query I select run_id from system.task_history where name = 'MockTask' limit 6; @@ -157,4 +157,4 @@ statement ok DROP TASK mytask statement ok -DROP TASK sessionTask \ No newline at end of file +DROP TASK sessionTask From d552af71788a4b07d1e35ff60e8634a5b5f5e3d3 Mon Sep 17 00:00:00 2001 From: Li Yazhou Date: Tue, 6 May 2025 17:36:55 +0800 Subject: [PATCH 7/7] fix err in sqlogictest --- tests/sqllogictests/suites/task/task_ddl_test.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sqllogictests/suites/task/task_ddl_test.test b/tests/sqllogictests/suites/task/task_ddl_test.test index 1eea1bc5d235d..763ab35cc76f5 100644 --- a/tests/sqllogictests/suites/task/task_ddl_test.test +++ b/tests/sqllogictests/suites/task/task_ddl_test.test @@ -123,7 +123,7 @@ select name, session_parameters from system.task_history where name = 'sessionTa ---- sessionTask {} -statement ok +statement error 2801 ALTER TASK sessionTask SET DATABASE = 'db2', TIMEZONE = 'Pacific/Honolulu'