Skip to content

chore(task): validate set settings name on alter taks set #17860

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/query/service/src/interpreters/interpreter_task_alter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -137,6 +139,19 @@ impl AlterTaskInterpreter {
}
req
}

fn validate_session_parameters(&self) -> Result<()> {
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(())
}
}

#[async_trait::async_trait]
Expand All @@ -158,6 +173,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();
Expand Down
11 changes: 11 additions & 0 deletions src/query/service/src/interpreters/interpreter_task_create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]
Expand All @@ -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();
Expand Down
1 change: 1 addition & 0 deletions src/query/settings/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
20 changes: 13 additions & 7 deletions tests/sqllogictests/suites/task/task_ddl_test.test
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ CREATE TASK mytask
SCHEDULE = USING CRON '0 0 0 1 1 ? 2100'
AS SELECT 1;

statement error 2801
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'
----
Expand Down Expand Up @@ -100,32 +107,31 @@ 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
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

query SSSS
select name, session_parameters from system.task_history where name = 'sessionTask'
----
sessionTask {"database":"mydb","timezone":"America/Los_Angeles"}
sessionTask {}

statement ok
statement error 2801
ALTER TASK sessionTask
SET
DATABASE = 'db2', TIMEZONE = 'Pacific/Honolulu'

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;
Expand All @@ -151,4 +157,4 @@ statement ok
DROP TASK mytask

statement ok
DROP TASK sessionTask
DROP TASK sessionTask
Loading