Skip to content

Commit 4af19cb

Browse files
TCeasonTCeason
authored
refactor: optimize query system.tables when query single table (#16869)
optimize: optimize query system.tables when query single table
1 parent b40336b commit 4af19cb

File tree

22 files changed

+327
-221
lines changed

22 files changed

+327
-221
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/meta/app/src/principal/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ pub use user_defined_function::UserDefinedFunction;
102102
pub use user_grant::GrantEntry;
103103
pub use user_grant::GrantObject;
104104
pub use user_grant::UserGrantSet;
105+
pub use user_grant::SYSTEM_TABLES_ALLOW_LIST;
105106
pub use user_identity::UserIdentity;
106107
pub use user_info::UserInfo;
107108
pub use user_info::UserOption;

‎src/meta/app/src/principal/user_grant.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,33 @@ use enumflags2::BitFlags;
2121
use crate::principal::UserPrivilegeSet;
2222
use crate::principal::UserPrivilegeType;
2323

24+
// some statements like `SELECT 1`, `SHOW USERS`, `SHOW ROLES`, `SHOW TABLES` will be
25+
// rewritten to the queries on the system tables, we need to skip the privilege check on
26+
// these tables.
27+
pub const SYSTEM_TABLES_ALLOW_LIST: [&str; 21] = [
28+
"catalogs",
29+
"columns",
30+
"databases",
31+
"databases_with_history",
32+
"dictionaries",
33+
"tables",
34+
"views",
35+
"tables_with_history",
36+
"views_with_history",
37+
"password_policies",
38+
"streams",
39+
"streams_terse",
40+
"virtual_columns",
41+
"users",
42+
"roles",
43+
"stages",
44+
"one",
45+
"processes",
46+
"user_functions",
47+
"functions",
48+
"indexes",
49+
];
50+
2451
#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, Eq, PartialEq, Hash)]
2552
pub enum GrantObject {
2653
Global,

‎src/query/catalog/src/table_context.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,10 @@ pub trait TableContext: Send + Sync {
224224
check_current_role_only: bool,
225225
) -> Result<()>;
226226
async fn get_available_roles(&self) -> Result<Vec<RoleInfo>>;
227-
async fn get_visibility_checker(&self) -> Result<GrantObjectVisibilityChecker>;
227+
async fn get_visibility_checker(
228+
&self,
229+
ignore_ownership: bool,
230+
) -> Result<GrantObjectVisibilityChecker>;
228231
fn get_fuse_version(&self) -> String;
229232
fn get_format_settings(&self) -> Result<FormatSettings>;
230233
fn get_tenant(&self) -> Tenant;

‎src/query/service/src/interpreters/access/privilege_access.rs‎

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use databend_common_meta_app::principal::StageType;
2929
use databend_common_meta_app::principal::UserGrantSet;
3030
use databend_common_meta_app::principal::UserPrivilegeSet;
3131
use databend_common_meta_app::principal::UserPrivilegeType;
32+
use databend_common_meta_app::principal::SYSTEM_TABLES_ALLOW_LIST;
3233
use databend_common_meta_app::tenant::Tenant;
3334
use databend_common_meta_types::seq_value::SeqV;
3435
use databend_common_sql::binder::MutationType;
@@ -58,33 +59,6 @@ enum ObjectId {
5859
Table(u64, u64),
5960
}
6061

61-
// some statements like `SELECT 1`, `SHOW USERS`, `SHOW ROLES`, `SHOW TABLES` will be
62-
// rewritten to the queries on the system tables, we need to skip the privilege check on
63-
// these tables.
64-
const SYSTEM_TABLES_ALLOW_LIST: [&str; 21] = [
65-
"catalogs",
66-
"columns",
67-
"databases",
68-
"databases_with_history",
69-
"dictionaries",
70-
"tables",
71-
"views",
72-
"tables_with_history",
73-
"views_with_history",
74-
"password_policies",
75-
"streams",
76-
"streams_terse",
77-
"virtual_columns",
78-
"users",
79-
"roles",
80-
"stages",
81-
"one",
82-
"processes",
83-
"user_functions",
84-
"functions",
85-
"indexes",
86-
];
87-
8862
// table functions that need `Super` privilege
8963
const SYSTEM_TABLE_FUNCTIONS: [&str; 1] = ["fuse_amend"];
9064

‎src/query/service/src/sessions/query_ctx.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -692,8 +692,14 @@ impl TableContext for QueryContext {
692692
self.get_current_session().get_id()
693693
}
694694

695-
async fn get_visibility_checker(&self) -> Result<GrantObjectVisibilityChecker> {
696-
self.shared.session.get_visibility_checker().await
695+
async fn get_visibility_checker(
696+
&self,
697+
ignore_ownership: bool,
698+
) -> Result<GrantObjectVisibilityChecker> {
699+
self.shared
700+
.session
701+
.get_visibility_checker(ignore_ownership)
702+
.await
697703
}
698704

699705
fn get_fuse_version(&self) -> String {

‎src/query/service/src/sessions/session.rs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,13 @@ impl Session {
310310
}
311311

312312
#[async_backtrace::framed]
313-
pub async fn get_visibility_checker(&self) -> Result<GrantObjectVisibilityChecker> {
314-
self.privilege_mgr().get_visibility_checker().await
313+
pub async fn get_visibility_checker(
314+
&self,
315+
ignore_ownership: bool,
316+
) -> Result<GrantObjectVisibilityChecker> {
317+
self.privilege_mgr()
318+
.get_visibility_checker(ignore_ownership)
319+
.await
315320
}
316321

317322
pub fn get_settings(&self) -> Arc<Settings> {

‎src/query/service/src/sessions/session_privilege_mgr.rs‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,10 @@ pub trait SessionPrivilegeManager {
7575

7676
async fn validate_available_role(&self, role_name: &str) -> Result<RoleInfo>;
7777

78-
async fn get_visibility_checker(&self) -> Result<GrantObjectVisibilityChecker>;
78+
async fn get_visibility_checker(
79+
&self,
80+
ignore_ownership: bool,
81+
) -> Result<GrantObjectVisibilityChecker>;
7982

8083
// fn show_grants(&self);
8184
}
@@ -336,27 +339,31 @@ impl<'a> SessionPrivilegeManager for SessionPrivilegeManagerImpl<'a> {
336339
}
337340

338341
#[async_backtrace::framed]
339-
async fn get_visibility_checker(&self) -> Result<GrantObjectVisibilityChecker> {
342+
async fn get_visibility_checker(
343+
&self,
344+
ignore_ownership: bool,
345+
) -> Result<GrantObjectVisibilityChecker> {
340346
// TODO(liyz): is it check the visibility according onwerships?
341-
let user_api = UserApiProvider::instance();
342-
let ownerships = user_api
343-
.role_api(&self.session_ctx.get_current_tenant())
344-
.get_ownerships()
345-
.await?;
346347
let roles = self.get_all_effective_roles().await?;
347348
let roles_name: Vec<String> = roles.iter().map(|role| role.name.to_string()).collect();
348349

349-
let ownership_objects = if roles_name.contains(&"account_admin".to_string()) {
350-
vec![]
351-
} else {
352-
let mut ownership_objects = vec![];
353-
for ownership in ownerships {
354-
if roles_name.contains(&ownership.data.role) {
355-
ownership_objects.push(ownership.data.object);
350+
let ownership_objects =
351+
if roles_name.contains(&"account_admin".to_string()) || ignore_ownership {
352+
vec![]
353+
} else {
354+
let user_api = UserApiProvider::instance();
355+
let ownerships = user_api
356+
.role_api(&self.session_ctx.get_current_tenant())
357+
.get_ownerships()
358+
.await?;
359+
let mut ownership_objects = vec![];
360+
for ownership in ownerships {
361+
if roles_name.contains(&ownership.data.role) {
362+
ownership_objects.push(ownership.data.object);
363+
}
356364
}
357-
}
358-
ownership_objects
359-
};
365+
ownership_objects
366+
};
360367

361368
Ok(GrantObjectVisibilityChecker::new(
362369
&self.get_current_user()?,

‎src/query/service/src/table_functions/infer_schema/parquet.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ impl AsyncSource for ParquetInferSchemaSource {
9999
.get_settings()
100100
.get_enable_experimental_rbac_check()?;
101101
if enable_experimental_rbac_check {
102-
let visibility_checker = self.ctx.get_visibility_checker().await?;
102+
let visibility_checker = self.ctx.get_visibility_checker(false).await?;
103103
if !(stage_info.is_temporary
104104
|| visibility_checker.check_stage_read_visibility(&stage_info.stage_name)
105105
|| stage_info.stage_type == StageType::User

‎src/query/service/src/table_functions/inspect_parquet/inspect_parquet_table.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ impl AsyncSource for InspectParquetSource {
213213
.get_settings()
214214
.get_enable_experimental_rbac_check()?;
215215
if enable_experimental_rbac_check {
216-
let visibility_checker = self.ctx.get_visibility_checker().await?;
216+
let visibility_checker = self.ctx.get_visibility_checker(false).await?;
217217
if !(stage_info.is_temporary
218218
|| visibility_checker.check_stage_read_visibility(&stage_info.stage_name)
219219
|| stage_info.stage_type == StageType::User

0 commit comments

Comments
 (0)