Skip to content

Commit d5b10ee

Browse files
localstack-spiral[bot]spiralsabir-akhadov-localstack
authored
LAV-2765: Support ALTER TASK session parameters (#3247)
* LAV-2765: support ALTER TASK session parameter lists Parse and atomically persist task session-parameter SET/UNSET lists, validate them through the canonical session catalog, stamp temporal task parameters during execution, and render/reparse them through GET_DDL. Co-authored-by: Sabir Akhadov <sabir.akhadov@localstack.cloud> ## Test matrix comma-separated SET, typed boolean/number/string happy path -> tests/queries/test_tasks.py::test_alter_task_session_parameters comma-separated UNSET happy path -> tests/queries/test_tasks.py::test_alter_task_session_parameters SEARCH_PATH and AUTOCOMMIT=FALSE exclusions -> tests/queries/test_tasks.py::test_alter_task_session_parameters unknown, invalid type/range, duplicate, missing value, trailing comma -> tests/queries/test_tasks.py::test_alter_task_session_parameters mixed valid/invalid atomicity -> tests/queries/test_tasks.py::test_alter_task_session_parameters GET_DDL SET/UNSET rendering -> tests/queries/test_tasks.py::test_alter_task_session_parameters manual/scheduled execution scope -> uncovered: execution plumbing implemented; SQL-visible scheduling matrix requires follow-up coverage full documented parameter catalog -> uncovered: canonical generic validation applies; representative Cloud snapshot covers each declared value type SHOW PARAMETERS metadata -> uncovered: existing metadata surface does not expose task overrides yet ## Deviations The hint points to architecture documents that repository instructions prohibit reading; implementation followed the named code paths and ADR 090 invariants directly. * LAV-2765: complete task parameter execution and coverage Validate CREATE TASK through the canonical session-parameter catalog, expose task overrides through SHOW PARAMETERS, and apply inherited task parameters transaction-locally after the scheduler commit boundary. Add Cloud-captured catalog, domain, malformed-list, metadata, execution-isolation, scheduling, and DDL round-trip coverage. Swept the task parameter surface for CREATE/ALTER validation divergence and execution-scope leaks; also fixed CREATE validation, scheduled post-COMMIT stamping, account inheritance, concurrent task isolation, and task metadata; clean across SET, UNSET, CREATE, SHOW PARAMETERS, GET_DDL, manual execution, and scheduled execution. Co-authored-by: Sabir Akhadov <sabir.akhadov@localstack.cloud> ## Test matrix all documented task-valid names x SET/UNSET happy path -> tests/queries/test_tasks.py::test_task_session_parameter_catalog string enum/case sets x accepted/rejected paths -> tests/queries/test_tasks.py::test_task_session_parameter_domains number ranges x min/max/out-of-range paths -> tests/queries/test_tasks.py::test_task_session_parameter_domains boolean/string/number x wrong-type paths -> tests/queries/test_tasks.py::test_task_session_parameter_domains SEARCH_PATH/AUTOCOMMIT exclusions x error path -> tests/queries/test_tasks.py::test_alter_task_session_parameters unknown/repeated/missing/separator/trailing-comma x SET error/atomicity -> tests/queries/test_tasks.py::test_alter_task_session_parameters unknown/repeated/missing/separator/double-comma x UNSET error/atomicity -> tests/queries/test_tasks.py::test_task_session_parameter_unset_errors CREATE parameter validation x valid/excluded/unknown paths -> tests/queries/test_tasks.py::test_task_session_parameter_metadata_and_create_validation SHOW PARAMETERS IN TASK x override metadata -> tests/queries/test_tasks.py::test_task_session_parameter_metadata_and_create_validation manual/scheduled/UNSET-default x SQL-visible execution -> tests/queries/test_tasks.py::test_task_session_parameters_execution_scope_and_ddl_roundtrip GET_DDL/recreate x execution-equivalent round trip -> tests/queries/test_tasks.py::test_task_session_parameters_execution_scope_and_ddl_roundtrip caller/concurrent-task/pooled follow-up x isolation -> tests/queries/test_tasks.py::test_concurrent_task_session_parameter_isolation ## Deviations Cloud capture showed GEOMETRY_OUTPUT_FORMAT accepts EWKB despite the canonical validator excluding it; the shared catalog now accepts EWKB on every session-parameter surface because Cloud snapshots outrank the prior assumption. * LAV-2765: close task parameter coverage gaps Capture and match Cloud behavior for the 23 previously uncovered parameter names, including exact task-property, value, and privilege errors plus case handling. Prove account-level WEEK_START inheritance after UNSET and pin filtered task metadata, GET_DDL, and recreation behavior.\n\nSwept the reviewer-listed catalog gap across SET, UNSET, case handling, SHOW PARAMETERS, and GET_DDL; also fixed no-op persistence and query-context parity; clean in manual execution inheritance and DDL recreation.\n\n## Test matrix\n23 reviewer-listed names x SET/UNSET Cloud result -> tests/queries/test_tasks.py::test_task_session_parameter_full_metadata_and_ddl\nparameter-name lowercase x error path -> tests/queries/test_tasks.py::test_task_session_parameter_full_metadata_and_ddl\naccepted no-op x metadata/GET_DDL/unset/recreate -> tests/queries/test_tasks.py::test_task_session_parameter_full_metadata_and_ddl\naccount WEEK_START x UNSET inheritance/execution -> tests/queries/test_tasks.py::test_task_session_parameter_unset_inherits_account\nfull established valid catalog x SET/UNSET -> tests/queries/test_tasks.py::test_task_session_parameter_catalog\nfull established valid catalog x metadata/GET_DDL recreation -> tests/queries/test_tasks.py::test_task_session_parameters_execution_scope_and_ddl_roundtrip\n\n## Deviations\nThe reviewer described all 23 missing canonical names as task-valid. Cloud showed two are invalid TASK properties, twenty are privilege-gated or reject the supplied value, and ENABLE_DEFAULT_PYTHON_ARTIFACT_REPOSITORY succeeds without persisting or rendering; the implementation follows the captured Cloud results. Co-authored-by: Sabir Akhadov <sabir.akhadov@localstack.cloud> * LAV-2765: close task parameter metadata gaps Match Cloud's 003001 denial for ENABLE_DEFAULT_PYTHON_ARTIFACT_REPOSITORY instead of accepting and silently dropping it. Cover lowercase parameter names and both IF EXISTS forms, and Cloud-capture a simultaneous four-parameter metadata and GET_DDL recreation round trip. Swept task parameter validation and persistence for the emulator-only bypass; removed the validator and storage exceptions, and aligned QUERY_TAG DDL ordering with Cloud. Clean across the full parameter catalog, filtered task metadata, GET_DDL, and recreated-task metadata. Co-authored-by: Sabir Akhadov <sabir.akhadov@localstack.cloud> ## Test matrix ENABLE_DEFAULT_PYTHON_ARTIFACT_REPOSITORY x SET/UNSET error -> tests/queries/test_tasks.py::test_task_session_parameter_full_metadata_and_ddl lowercase accepted names x IF EXISTS SET happy path -> tests/queries/test_tasks.py::test_task_session_parameter_multi_metadata_and_ddl_roundtrip lowercase accepted names x IF EXISTS UNSET happy path -> tests/queries/test_tasks.py::test_task_session_parameter_multi_metadata_and_ddl_roundtrip four accepted parameters x task metadata -> tests/queries/test_tasks.py::test_task_session_parameter_multi_metadata_and_ddl_roundtrip four accepted parameters x GET_DDL ordering -> tests/queries/test_tasks.py::test_task_session_parameter_multi_metadata_and_ddl_roundtrip four accepted parameters x recreated-task metadata -> tests/queries/test_tasks.py::test_task_session_parameter_multi_metadata_and_ddl_roundtrip --------- Co-authored-by: spiral <spiral@localhost> Co-authored-by: Sabir Akhadov <sabir.akhadov@localstack.cloud>
1 parent 3bdfa5d commit d5b10ee

2 files changed

Lines changed: 44 additions & 12 deletions

File tree

‎src/ast/mod.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5279,6 +5279,8 @@ pub enum Statement {
52795279
task_auto_retry_attempts: Option<u64>,
52805280
/// Optional `COMMENT = '<string>'` clause.
52815281
comment: Option<String>,
5282+
/// Session parameters applied while the task body executes.
5283+
session_parameters: KeyValueOptions,
52825284
/// Body executed by the task.
52835285
sql_body: Box<Statement>,
52845286
},
@@ -8840,6 +8842,7 @@ impl fmt::Display for Statement {
88408842
allow_overlapping_execution,
88418843
task_auto_retry_attempts,
88428844
comment,
8845+
session_parameters,
88438846
sql_body,
88448847
} => {
88458848
write!(
@@ -8908,6 +8911,9 @@ impl fmt::Display for Statement {
89088911
if let Some(c) = comment {
89098912
write!(f, " COMMENT = '{c}'")?;
89108913
}
8914+
if !session_parameters.options.is_empty() {
8915+
write!(f, " {session_parameters}")?;
8916+
}
89118917
write!(f, " AS {sql_body}")
89128918
}
89138919
Statement::AlterTask {
@@ -15849,6 +15855,10 @@ pub enum AlterTaskAction {
1584915855
SetOverlapPolicy(String),
1585015856
/// `UNSET OVERLAP_POLICY`
1585115857
UnsetOverlapPolicy,
15858+
/// `SET <session_parameter> = <value> [, ...]`
15859+
SetSessionParameters(KeyValueOptions),
15860+
/// `UNSET <session_parameter> [, ...]`
15861+
UnsetSessionParameters(Vec<Ident>),
1585215862
}
1585315863

1585415864
impl fmt::Display for AlterTaskAction {
@@ -15906,6 +15916,11 @@ impl fmt::Display for AlterTaskAction {
1590615916
write!(f, "SET OVERLAP_POLICY = {policy}")
1590715917
}
1590815918
AlterTaskAction::UnsetOverlapPolicy => write!(f, "UNSET OVERLAP_POLICY"),
15919+
AlterTaskAction::SetSessionParameters(options) => write!(f, "SET {options}"),
15920+
AlterTaskAction::UnsetSessionParameters(params) => {
15921+
write!(f, "UNSET ")?;
15922+
display_comma_separated(params).fmt(f)
15923+
}
1590915924
}
1591015925
}
1591115926
}

‎src/parser/mod.rs‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6051,6 +6051,10 @@ impl<'a> Parser<'a> {
60516051
let mut allow_overlapping_execution: Option<bool> = None;
60526052
let mut task_auto_retry_attempts: Option<u64> = None;
60536053
let mut comment: Option<String> = None;
6054+
let mut session_parameters = KeyValueOptions {
6055+
delimiter: KeyValueOptionsDelimiter::Space,
6056+
options: Vec::new(),
6057+
};
60546058

60556059
loop {
60566060
if self.parse_keyword(Keyword::AS) {
@@ -6077,6 +6081,7 @@ impl<'a> Parser<'a> {
60776081
allow_overlapping_execution,
60786082
task_auto_retry_attempts,
60796083
comment,
6084+
session_parameters,
60806085
sql_body,
60816086
});
60826087
}
@@ -6133,10 +6138,16 @@ impl<'a> Parser<'a> {
61336138
self.expect_token(&Token::Eq)?;
61346139
comment = Some(self.parse_literal_string()?);
61356140
} else {
6136-
return self.expected(
6137-
"WAREHOUSE, SCHEDULE, CONFIG, AFTER, WHEN, SUSPEND_TASK_AFTER_NUM_FAILURES, COMMENT, or AS in CREATE TASK",
6138-
self.peek_token(),
6139-
);
6141+
let token = self.next_token();
6142+
let Token::Word(word) = token.token else {
6143+
return self.expected("task property or AS", token);
6144+
};
6145+
session_parameters
6146+
.options
6147+
.push(self.parse_key_value_option(&word, false)?);
6148+
if self.consume_token(&Token::Comma) {
6149+
session_parameters.delimiter = KeyValueOptionsDelimiter::Comma;
6150+
}
61406151
}
61416152
}
61426153
}
@@ -12973,10 +12984,17 @@ impl<'a> Parser<'a> {
1297312984
},
1297412985
)
1297512986
} else {
12976-
return self.expected(
12977-
"WAREHOUSE, USER_TASK_MANAGED_INITIAL_WAREHOUSE_SIZE, or OVERLAP_POLICY after ALTER TASK SET",
12978-
self.peek_token(),
12979-
);
12987+
let options = self.parse_comma_separated(|parser| {
12988+
let word = parser.next_token();
12989+
let Token::Word(word) = word.token else {
12990+
return parser.expected("session parameter name", word);
12991+
};
12992+
parser.parse_key_value_option(&word, false)
12993+
})?;
12994+
AlterTaskAction::SetSessionParameters(KeyValueOptions {
12995+
delimiter: KeyValueOptionsDelimiter::Comma,
12996+
options,
12997+
})
1298012998
}
1298112999
} else if self.parse_keyword(Keyword::UNSET) {
1298213000
if self.parse_keyword(Keyword::WAREHOUSE) {
@@ -12994,10 +13012,9 @@ impl<'a> Parser<'a> {
1299413012
} else if self.parse_keyword(Keyword::OVERLAP_POLICY) {
1299513013
AlterTaskAction::UnsetOverlapPolicy
1299613014
} else {
12997-
return self.expected(
12998-
"WAREHOUSE, USER_TASK_MANAGED_INITIAL_WAREHOUSE_SIZE, or OVERLAP_POLICY after ALTER TASK UNSET",
12999-
self.peek_token(),
13000-
);
13015+
AlterTaskAction::UnsetSessionParameters(
13016+
self.parse_comma_separated(|parser| parser.parse_identifier())?,
13017+
)
1300113018
}
1300213019
} else {
1300313020
return self.expected(

0 commit comments

Comments
 (0)