Skip to content

Commit 9d9d0d3

Browse files
TCeasonTCeason
committed
fix show task bug;
1 parent aef34a3 commit 9d9d0d3

File tree

5 files changed

+105
-78
lines changed

5 files changed

+105
-78
lines changed

‎src/query/ast/tests/it/testdata/statement-error.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -923,7 +923,7 @@ error:
923923
--> SQL:1:22
924924
|
925925
1 | GRANT CREATE TASK ON task mytask1 TO role role1
926-
| ----- ------ ^^^^ unexpected `task`, expecting `FALSE`, <QuotedString>, <LiteralInteger>, `TRUE`, `IDENTIFIER`, <PGLiteralHex>, <MySQLLiteralHex>, `*`, or <Ident>
926+
| ----- ------ ^^^^ unexpected `task`, expecting <LiteralString>, `FALSE`, <LiteralInteger>, `TRUE`, `IDENTIFIER`, <PGLiteralHex>, <MySQLLiteralHex>, `*`, or <Ident>
927927
| | |
928928
| | while parsing <privileges> ON <privileges_level>
929929
| while parsing `GRANT { ROLE <role_name> | schemaObjectPrivileges | ALL [ PRIVILEGES ] ON <privileges_level> } TO { [ROLE <role_name>] | [USER] <user> }`
@@ -948,7 +948,7 @@ error:
948948
--> SQL:1:17
949949
|
950950
1 | GRANT select ON task MyTask1 TO u2
951-
| ^^^^ unexpected `task`, expecting `FALSE`, `TABLE`, <QuotedString>, <LiteralInteger>, `TRUE`, `IDENTIFIER`, <PGLiteralHex>, <MySQLLiteralHex>, `DATABASE`, `*`, or <Ident>
951+
| ^^^^ unexpected `task`, expecting <LiteralString>, `FALSE`, `TABLE`, <LiteralInteger>, `TRUE`, `IDENTIFIER`, <PGLiteralHex>, <MySQLLiteralHex>, `DATABASE`, `*`, or <Ident>
952952

953953

954954
---------- Input ----------
@@ -958,6 +958,6 @@ error:
958958
--> SQL:1:16
959959
|
960960
1 | GRANT usage ON task MyTask1 TO u1
961-
| ^^^^ unexpected `task`, expecting `FALSE`, `TABLE`, <QuotedString>, <LiteralInteger>, `TRUE`, `IDENTIFIER`, <PGLiteralHex>, <MySQLLiteralHex>, `DATABASE`, `UDF`, `*`, or <Ident>
961+
| ^^^^ unexpected `task`, expecting <LiteralString>, `FALSE`, `TABLE`, <LiteralInteger>, `TRUE`, `IDENTIFIER`, <PGLiteralHex>, <MySQLLiteralHex>, `DATABASE`, `UDF`, `*`, or <Ident>
962962

963963

‎src/query/ast/tests/it/testdata/statement.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18353,13 +18353,13 @@ CreateDynamicTable(
1835318353
---------- Input ----------
1835418354
GRANT CREATE TASK ON *.* TO role role1
1835518355
---------- Output ---------
18356-
GRANT CREATE DATABASE ON *.* TO ROLE 'role1'
18356+
GRANT CREATE TASK ON *.* TO ROLE 'role1'
1835718357
---------- AST ------------
1835818358
Grant(
1835918359
GrantStmt {
1836018360
source: Privs {
1836118361
privileges: [
18362-
CreateDatabase,
18362+
CreateTask,
1836318363
],
1836418364
level: Global,
1836518365
},

‎src/query/storages/system/src/task_history_table.rs

Lines changed: 53 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -186,45 +186,60 @@ impl AsyncSystemTable for TaskHistoryTable {
186186
}
187187
}
188188

189-
let owned_tasks_names = get_owned_task_names(user_api, &tenant, &all_effective_roles).await;
190-
if let Some(task_name) = &task_name {
191-
// The user does not have admin role and not own the task_name
192-
// Need directly return empty block
193-
if !all_effective_roles
194-
.iter()
195-
.any(|role| role.to_lowercase() == BUILTIN_ROLE_ACCOUNT_ADMIN)
196-
&& !owned_tasks_names.contains(task_name)
197-
{
198-
info!(
199-
"--task_history:198 all_effective_roles is {:?}, owned_tasks_names is {:?}, task_name is {:?}",
200-
all_effective_roles.clone(),
201-
owned_tasks_names.clone(),
202-
task_name.clone()
203-
);
204-
return parse_task_runs_to_datablock(vec![]);
189+
let has_admin_role = all_effective_roles
190+
.iter()
191+
.any(|role| role.to_lowercase() == BUILTIN_ROLE_ACCOUNT_ADMIN);
192+
let req = if has_admin_role {
193+
ShowTaskRunsRequest {
194+
tenant_id: tenant.tenant_name().to_string(),
195+
scheduled_time_start: scheduled_time_start.unwrap_or("".to_string()),
196+
scheduled_time_end: scheduled_time_end.unwrap_or("".to_string()),
197+
task_name: task_name.unwrap_or("".to_string()),
198+
result_limit: result_limit.unwrap_or(0), // 0 means default
199+
error_only: false,
200+
owners: all_effective_roles.clone(),
201+
next_page_token: None,
202+
page_size: None,
203+
previous_page_token: None,
204+
task_ids: vec![],
205+
task_names: vec![],
206+
}
207+
} else {
208+
let owned_tasks_names =
209+
get_owned_task_names(user_api, &tenant, &all_effective_roles, has_admin_role).await;
210+
if let Some(task_name) = &task_name {
211+
// The user does not have admin role and not own the task_name
212+
// Need directly return empty block
213+
if !owned_tasks_names.contains(task_name) {
214+
info!(
215+
"--task_history:215 all_effective_roles is {:?}, owned_tasks_names is {:?}, task_name is {:?}",
216+
all_effective_roles.clone(),
217+
owned_tasks_names.clone(),
218+
task_name.clone()
219+
);
220+
return parse_task_runs_to_datablock(vec![]);
221+
}
222+
}
223+
info!(
224+
"--task_history:224 all_effective_roles is {:?}, owned_tasks_names is {:?}, task_name is {:?}",
225+
all_effective_roles.clone(),
226+
owned_tasks_names.clone(),
227+
task_name.clone()
228+
);
229+
ShowTaskRunsRequest {
230+
tenant_id: tenant.tenant_name().to_string(),
231+
scheduled_time_start: scheduled_time_start.unwrap_or("".to_string()),
232+
scheduled_time_end: scheduled_time_end.unwrap_or("".to_string()),
233+
task_name: task_name.unwrap_or("".to_string()),
234+
result_limit: result_limit.unwrap_or(0), // 0 means default
235+
error_only: false,
236+
owners: all_effective_roles.clone(),
237+
next_page_token: None,
238+
page_size: None,
239+
previous_page_token: None,
240+
task_ids: vec![],
241+
task_names: owned_tasks_names.clone(),
205242
}
206-
}
207-
208-
info!(
209-
"--task_history:203 all_effective_roles is {:?}, owned_tasks_names is {:?}, task_name is {:?}",
210-
all_effective_roles.clone(),
211-
owned_tasks_names.clone(),
212-
task_name.clone()
213-
);
214-
215-
let req = ShowTaskRunsRequest {
216-
tenant_id: tenant.tenant_name().to_string(),
217-
scheduled_time_start: scheduled_time_start.unwrap_or("".to_string()),
218-
scheduled_time_end: scheduled_time_end.unwrap_or("".to_string()),
219-
task_name: task_name.unwrap_or("".to_string()),
220-
result_limit: result_limit.unwrap_or(0), // 0 means default
221-
error_only: false,
222-
owners: all_effective_roles.clone(),
223-
next_page_token: None,
224-
page_size: None,
225-
previous_page_token: None,
226-
task_ids: vec![],
227-
task_names: owned_tasks_names.clone(),
228243
};
229244

230245
let cloud_api = CloudControlApiProvider::instance();

‎src/query/storages/system/src/tasks_table.rs

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -159,38 +159,53 @@ impl AsyncSystemTable for TasksTable {
159159
});
160160
}
161161
}
162-
let owned_tasks_names = get_owned_task_names(user_api, &tenant, &all_effective_roles).await;
163-
if let Some(task_name) = &task_name {
164-
// The user does not have admin role and not own the task_name
165-
// Need directly return empty block
166-
if !all_effective_roles
167-
.iter()
168-
.any(|role| role.to_lowercase() == BUILTIN_ROLE_ACCOUNT_ADMIN)
169-
&& !owned_tasks_names.contains(task_name)
170-
{
171-
info!(
172-
"--tasks:171 all_effective_roles is {:?}, owned_tasks_names is {:?}, task_name is {:?}",
173-
all_effective_roles.clone(),
174-
owned_tasks_names.clone(),
175-
task_name.clone()
176-
);
162+
163+
let has_admin_role = all_effective_roles
164+
.iter()
165+
.any(|role| role.to_lowercase() == BUILTIN_ROLE_ACCOUNT_ADMIN);
166+
167+
let req = if has_admin_role {
168+
ShowTasksRequest {
169+
tenant_id: tenant.tenant_name().to_string(),
170+
name_like: "".to_string(),
171+
result_limit: 10000, // TODO: use plan.limit pushdown
172+
owners: all_effective_roles.clone(),
173+
task_ids: vec![],
174+
task_names: vec![],
175+
}
176+
} else {
177+
let owned_tasks_names =
178+
get_owned_task_names(user_api, &tenant, &all_effective_roles, has_admin_role).await;
179+
if let Some(task_name) = &task_name {
180+
// The user does not have admin role and not own the task_name
181+
// Need directly return empty block
182+
if !owned_tasks_names.contains(task_name) {
183+
info!(
184+
"--tasks:184 all_effective_roles is {:?}, owned_tasks_names is {:?}, task_name is {:?}",
185+
all_effective_roles.clone(),
186+
owned_tasks_names.clone(),
187+
task_name.clone()
188+
);
189+
return parse_task_runs_to_datablock(vec![]);
190+
}
191+
}
192+
info!(
193+
"--tasks:193 all_effective_roles is {:?}, owned_tasks_names is {:?}, task_name is {:?}",
194+
all_effective_roles.clone(),
195+
owned_tasks_names.clone(),
196+
task_name.clone()
197+
);
198+
if owned_tasks_names.is_empty() {
177199
return parse_task_runs_to_datablock(vec![]);
178200
}
179-
}
180-
info!(
181-
"--tasks:175 all_effective_roles is {:?}, owned_tasks_names is {:?}, task_name is {:?}",
182-
all_effective_roles.clone(),
183-
owned_tasks_names.clone(),
184-
task_name.clone()
185-
);
186-
187-
let req = ShowTasksRequest {
188-
tenant_id: tenant.tenant_name().to_string(),
189-
name_like: "".to_string(),
190-
result_limit: 10000, // TODO: use plan.limit pushdown
191-
owners: all_effective_roles.clone(),
192-
task_ids: vec![],
193-
task_names: owned_tasks_names.clone(),
201+
ShowTasksRequest {
202+
tenant_id: tenant.tenant_name().to_string(),
203+
name_like: "".to_string(),
204+
result_limit: 10000, // TODO: use plan.limit pushdown
205+
owners: all_effective_roles.clone(),
206+
task_ids: vec![],
207+
task_names: owned_tasks_names.clone(),
208+
}
194209
};
195210

196211
let cloud_api = CloudControlApiProvider::instance();

‎src/query/storages/system/src/util.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ use databend_common_expression::Scalar;
1919
use databend_common_meta_app::principal::OwnershipObject;
2020
use databend_common_meta_app::tenant::Tenant;
2121
use databend_common_users::UserApiProvider;
22-
use databend_common_users::BUILTIN_ROLE_ACCOUNT_ADMIN;
2322

2423
pub fn find_eq_filter(expr: &Expr<String>, visitor: &mut impl FnMut(&str, &Scalar)) {
2524
match expr {
@@ -118,12 +117,10 @@ pub(crate) async fn get_owned_task_names(
118117
user_api: Arc<UserApiProvider>,
119118
tenant: &Tenant,
120119
all_effective_roles: &[String],
120+
has_admin_role: bool,
121121
) -> Vec<String> {
122122
let mut owned_tasks_names = vec![];
123-
let has_admin_role = all_effective_roles
124-
.iter()
125-
.any(|role| role.to_lowercase() == BUILTIN_ROLE_ACCOUNT_ADMIN);
126-
if has_admin_role {
123+
if !has_admin_role {
127124
// Note: In old version databend-query the hashmap maybe empty
128125
let task_ownerships = user_api
129126
.list_tasks_ownerships(tenant)

0 commit comments

Comments
 (0)