Skip to content

Commit 5220ad9

Browse files
TCeasonTCeason
authored
chore(query): delete replace masking/row access policy (#18972)
* chore(query): delete replace masking/row access policy * remove useless get_pb
1 parent de8e392 commit 5220ad9

File tree

17 files changed

+79
-334
lines changed

17 files changed

+79
-334
lines changed

‎src/meta/api/src/data_mask_api_impl.rs‎

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ use databend_common_meta_app::data_mask::MaskPolicyTableIdListIdent;
2525
use databend_common_meta_app::data_mask::MaskpolicyTableIdList;
2626
use databend_common_meta_app::id_generator::IdGenerator;
2727
use databend_common_meta_app::row_access_policy::RowAccessPolicyNameIdent;
28-
use databend_common_meta_app::schema::CreateOption;
2928
use databend_common_meta_app::tenant::Tenant;
3029
use databend_common_meta_app::tenant_key::errors::ExistError;
3130
use databend_common_meta_app::KeyWithTenant;
@@ -71,31 +70,6 @@ impl<KV: kvapi::KVApi<Error = MetaError>> DatamaskApi for KV {
7170

7271
let mut txn = TxnRequest::default();
7372

74-
let res = self.get_id_and_value(name_ident).await?;
75-
debug!(res :? = res, name_key :? =(name_ident); "create_data_mask");
76-
77-
let mut curr_seq = 0;
78-
79-
if let Some((seq_id, seq_meta)) = res {
80-
match req.create_option {
81-
CreateOption::Create => {
82-
return Ok(Err(
83-
name_ident.exist_error(format!("{} already exists", req.name))
84-
));
85-
}
86-
CreateOption::CreateIfNotExists => {
87-
return Ok(Ok(CreateDatamaskReply { id: *seq_id.data }));
88-
}
89-
CreateOption::CreateOrReplace => {
90-
let id_ident = seq_id.data.into_t_ident(name_ident.tenant());
91-
92-
txn_delete_exact(&mut txn, &id_ident, seq_meta.seq);
93-
94-
curr_seq = seq_id.seq;
95-
}
96-
};
97-
}
98-
9973
// Create data mask by inserting these record:
10074
// name -> id
10175
// id -> policy
@@ -114,7 +88,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> DatamaskApi for KV {
11488
{
11589
let meta: DatamaskMeta = req.data_mask_meta.clone();
11690
let id_list = MaskpolicyTableIdList::default();
117-
txn.condition.push(txn_cond_eq_seq(name_ident, curr_seq));
91+
txn.condition.push(txn_cond_eq_seq(name_ident, 0));
11892
txn.condition
11993
.push(txn_cond_eq_seq(&row_access_name_ident, 0));
12094
txn.if_then.extend(vec![

‎src/meta/api/src/row_access_policy_api_impl.rs‎

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -70,25 +70,6 @@ impl<KV: kvapi::KVApi<Error = MetaError>> RowAccessPolicyApi for KV {
7070
let policy_id = RowAccessPolicyId::new(row_access_id);
7171
let mut txn = TxnRequest::default();
7272

73-
let res = self.get_id_and_value(name_ident).await?;
74-
debug!(res :? = res, name_key :? =(name_ident); "create_row_access");
75-
76-
let mut curr_seq = 0;
77-
78-
if let Some((seq_id, seq_meta)) = res {
79-
if req.can_replace {
80-
let id_ident = seq_id.data.into_t_ident(name_ident.tenant());
81-
82-
txn_delete_exact(&mut txn, &id_ident, seq_meta.seq);
83-
84-
// TODO(eason): need to remove row policy from table meta
85-
86-
curr_seq = seq_id.seq;
87-
} else {
88-
return Ok(Err(name_ident.exist_error(func_name!())));
89-
}
90-
}
91-
9273
// Create row policy by inserting these record:
9374
// name -> id
9475
// id -> policy
@@ -103,7 +84,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> RowAccessPolicyApi for KV {
10384

10485
{
10586
let meta: RowAccessPolicyMeta = req.row_access_policy_meta.clone();
106-
txn.condition.push(txn_cond_eq_seq(name_ident, curr_seq));
87+
txn.condition.push(txn_cond_eq_seq(name_ident, 0));
10788
txn.condition.push(txn_cond_eq_seq(&mask_name_ident, 0));
10889
txn.if_then.extend(vec![
10990
txn_op_put_pb(name_ident, &policy_id, None)?, // name -> policy_id

‎src/meta/api/src/schema_api_test_suite.rs‎

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ use databend_common_expression::VIRTUAL_COLUMNS_LIMIT;
3737
use databend_common_expression::VIRTUAL_COLUMN_ID_START;
3838
use databend_common_meta_app::app_error::AppError;
3939
use databend_common_meta_app::data_mask::CreateDatamaskReq;
40-
use databend_common_meta_app::data_mask::DataMaskIdIdent;
4140
use databend_common_meta_app::data_mask::DataMaskNameIdent;
4241
use databend_common_meta_app::data_mask::DatamaskMeta;
4342
use databend_common_meta_app::data_mask::MaskPolicyIdTableId;
@@ -3184,7 +3183,6 @@ impl SchemaApiTestSuite {
31843183
let mask2_id;
31853184
{
31863185
let req = CreateDatamaskReq {
3187-
create_option: CreateOption::CreateIfNotExists,
31883186
name: DataMaskNameIdent::new(tenant.clone(), mask_name_1.to_string()),
31893187
data_mask_meta: DatamaskMeta {
31903188
args: vec![],
@@ -3201,7 +3199,6 @@ impl SchemaApiTestSuite {
32013199
mask1_id = get_kv_u64_data(mt, &mask1_name_ident).await?;
32023200

32033201
let req = CreateDatamaskReq {
3204-
create_option: CreateOption::CreateIfNotExists,
32053202
name: DataMaskNameIdent::new(tenant.clone(), mask_name_2.to_string()),
32063203
data_mask_meta: DatamaskMeta {
32073204
args: vec![],
@@ -3457,57 +3454,6 @@ impl SchemaApiTestSuite {
34573454
assert!(res.is_none());
34583455
}
34593456

3460-
info!("--- create or replace mask policy");
3461-
{
3462-
let mask_name = "replace_mask";
3463-
let name = DataMaskNameIdent::new(tenant.clone(), mask_name);
3464-
let req = CreateDatamaskReq {
3465-
create_option: CreateOption::CreateIfNotExists,
3466-
name: name.clone(),
3467-
data_mask_meta: DatamaskMeta {
3468-
args: vec![],
3469-
return_type: "".to_string(),
3470-
body: "".to_string(),
3471-
comment: Some("before".to_string()),
3472-
create_on: created_on,
3473-
update_on: None,
3474-
},
3475-
};
3476-
mt.create_data_mask(req).await??;
3477-
let old_id: u64 = get_kv_u64_data(mt, &name).await?;
3478-
3479-
let id_key = DataMaskIdIdent::new(&tenant, old_id);
3480-
3481-
let meta: DatamaskMeta = get_kv_data(mt, &id_key).await?;
3482-
assert_eq!(meta.comment, Some("before".to_string()));
3483-
3484-
let req = CreateDatamaskReq {
3485-
create_option: CreateOption::CreateOrReplace,
3486-
name: name.clone(),
3487-
data_mask_meta: DatamaskMeta {
3488-
args: vec![],
3489-
return_type: "".to_string(),
3490-
body: "".to_string(),
3491-
comment: Some("after".to_string()),
3492-
create_on: created_on,
3493-
update_on: None,
3494-
},
3495-
};
3496-
mt.create_data_mask(req).await??;
3497-
3498-
// assert old id key has been deleted
3499-
let meta: Result<DatamaskMeta, KVAppError> = get_kv_data(mt, &id_key).await;
3500-
assert!(meta.is_err());
3501-
3502-
let id: u64 = get_kv_u64_data(mt, &name).await?;
3503-
assert_ne!(old_id, id);
3504-
3505-
let id_key = DataMaskIdIdent::new(&tenant, id);
3506-
3507-
let meta: DatamaskMeta = get_kv_data(mt, &id_key).await?;
3508-
assert_eq!(meta.comment, Some("after".to_string()));
3509-
}
3510-
35113457
Ok(())
35123458
}
35133459

@@ -3549,7 +3495,6 @@ impl SchemaApiTestSuite {
35493495
info!("--- create row access policy");
35503496

35513497
let req = CreateRowAccessPolicyReq {
3552-
can_replace: false,
35533498
name: RowAccessPolicyNameIdent::new(tenant.clone(), policy1.to_string()),
35543499
row_access_policy_meta: RowAccessPolicyMeta {
35553500
args: vec![("number".to_string(), "UInt64".to_string())],
@@ -3566,7 +3511,6 @@ impl SchemaApiTestSuite {
35663511
let policy1_id = res.0.data;
35673512

35683513
let req = CreateRowAccessPolicyReq {
3569-
can_replace: false,
35703514
name: RowAccessPolicyNameIdent::new(tenant.clone(), policy2.to_string()),
35713515
row_access_policy_meta: RowAccessPolicyMeta {
35723516
args: vec![("number".to_string(), "UInt64".to_string())],
@@ -3887,7 +3831,6 @@ impl SchemaApiTestSuite {
38873831
let mask_cleanup_ident =
38883832
DataMaskNameIdent::new(tenant.clone(), mask_cleanup_name.to_string());
38893833
mt.create_data_mask(CreateDatamaskReq {
3890-
create_option: CreateOption::Create,
38913834
name: mask_cleanup_ident.clone(),
38923835
data_mask_meta: DatamaskMeta {
38933836
args: vec![],
@@ -3950,7 +3893,6 @@ impl SchemaApiTestSuite {
39503893
// Create another masking policy and bind it.
39513894
let mask_guard_ident = DataMaskNameIdent::new(tenant.clone(), mask_guard_name.to_string());
39523895
mt.create_data_mask(CreateDatamaskReq {
3953-
create_option: CreateOption::Create,
39543896
name: mask_guard_ident.clone(),
39553897
data_mask_meta: DatamaskMeta {
39563898
args: vec![],
@@ -4043,7 +3985,6 @@ impl SchemaApiTestSuite {
40433985
let policy_cleanup_ident =
40443986
RowAccessPolicyNameIdent::new(tenant.clone(), policy_cleanup_name.to_string());
40453987
mt.create_row_access_policy(CreateRowAccessPolicyReq {
4046-
can_replace: false,
40473988
name: policy_cleanup_ident.clone(),
40483989
row_access_policy_meta: RowAccessPolicyMeta {
40493990
args: vec![("number".to_string(), "UInt64".to_string())],
@@ -4117,7 +4058,6 @@ impl SchemaApiTestSuite {
41174058
let policy_guard_ident =
41184059
RowAccessPolicyNameIdent::new(tenant.clone(), policy_guard_name.to_string());
41194060
mt.create_row_access_policy(CreateRowAccessPolicyReq {
4120-
can_replace: false,
41214061
name: policy_guard_ident.clone(),
41224062
row_access_policy_meta: RowAccessPolicyMeta {
41234063
args: vec![("number".to_string(), "UInt64".to_string())],

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ pub use mask_policy_policy_table_id_ident::MaskPolicyIdTableId;
2828
pub use mask_policy_policy_table_id_ident::MaskPolicyTableIdIdent;
2929
pub use mask_policy_table_id_list_ident::MaskPolicyTableIdListIdent;
3030

31-
use crate::schema::CreateOption;
32-
3331
#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, PartialEq, Eq)]
3432
pub struct DatamaskMeta {
3533
// Vec<(arg_name, arg_type)>
@@ -43,7 +41,6 @@ pub struct DatamaskMeta {
4341

4442
#[derive(Clone, Debug, PartialEq, Eq)]
4543
pub struct CreateDatamaskReq {
46-
pub create_option: CreateOption,
4744
pub name: DataMaskNameIdent,
4845
pub data_mask_meta: DatamaskMeta,
4946
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ pub struct RowAccessPolicyMeta {
3535

3636
#[derive(Clone, Debug, PartialEq, Eq)]
3737
pub struct CreateRowAccessPolicyReq {
38-
pub can_replace: bool,
3938
pub name: RowAccessPolicyNameIdent,
4039
pub row_access_policy_meta: RowAccessPolicyMeta,
4140
}

‎src/query/ast/src/ast/statements/data_mask.rs‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ use derive_visitor::Drive;
1919
use derive_visitor::DriveMut;
2020

2121
use crate::ast::quote::QuotedString;
22-
use crate::ast::CreateOption;
2322
use crate::ast::Expr;
2423
use crate::ast::TypeName;
2524

@@ -39,19 +38,15 @@ pub struct DataMaskPolicy {
3938

4039
#[derive(Debug, Clone, PartialEq, Drive, DriveMut)]
4140
pub struct CreateDatamaskPolicyStmt {
42-
pub create_option: CreateOption,
41+
pub if_not_exists: bool,
4342
pub name: String,
4443
pub policy: DataMaskPolicy,
4544
}
4645

4746
impl Display for CreateDatamaskPolicyStmt {
4847
fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
49-
write!(f, "CREATE ")?;
50-
if let CreateOption::CreateOrReplace = self.create_option {
51-
write!(f, "OR REPLACE ")?;
52-
}
53-
write!(f, "MASKING POLICY ")?;
54-
if let CreateOption::CreateIfNotExists = self.create_option {
48+
write!(f, "CREATE MASKING POLICY ")?;
49+
if self.if_not_exists {
5550
write!(f, "IF NOT EXISTS ")?;
5651
}
5752
write!(f, "{} AS (", self.name)?;

‎src/query/ast/src/ast/statements/row_access_policy.rs‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ use derive_visitor::Drive;
1919
use derive_visitor::DriveMut;
2020

2121
use crate::ast::write_comma_separated_list;
22-
use crate::ast::CreateOption;
2322
use crate::ast::Expr;
2423
use crate::ast::Identifier;
2524
use crate::ast::TypeName;
@@ -54,23 +53,19 @@ impl Display for RowAccessPolicyDefinition {
5453

5554
#[derive(Debug, Clone, PartialEq, Drive, DriveMut)]
5655
pub struct CreateRowAccessPolicyStmt {
57-
pub create_option: CreateOption,
56+
pub if_not_exists: bool,
5857
pub name: Identifier,
5958
pub description: Option<String>,
6059
pub definition: RowAccessPolicyDefinition,
6160
}
6261

63-
// CREATE [ OR REPLACE ] ROW ACCESS POLICY [ IF NOT EXISTS ] <name> AS
62+
// CREATE ROW ACCESS POLICY [ IF NOT EXISTS ] <name> AS
6463
// ( <arg_name> <arg_type> [ , ... ] ) RETURNS BOOLEAN -> <body>
6564
// [ COMMENT = '<string_literal>' ]
6665
impl Display for CreateRowAccessPolicyStmt {
6766
fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
68-
write!(f, "CREATE")?;
69-
if let CreateOption::CreateOrReplace = self.create_option {
70-
write!(f, " OR REPLACE")?;
71-
}
72-
write!(f, " ROW ACCESS POLICY")?;
73-
if let CreateOption::CreateIfNotExists = self.create_option {
67+
write!(f, "CREATE ROW ACCESS POLICY")?;
68+
if self.if_not_exists {
7469
write!(f, " IF NOT EXISTS")?;
7570
}
7671
write!(f, " {} AS {}", self.name, self.definition)?;

‎src/query/ast/src/parser/statement.rs‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1826,26 +1826,22 @@ pub fn statement_body(i: Input) -> IResult<Statement> {
18261826
);
18271827

18281828
// row policy
1829-
// CREATE [ OR REPLACE ] ROW ACCESS POLICY [ IF NOT EXISTS ] <name> AS
1829+
// CREATE ROW ACCESS POLICY [ IF NOT EXISTS ] <name> AS
18301830
// ( <arg_name> <arg_type> [ , ... ] ) RETURNS BOOLEAN -> <body>
18311831
// [ COMMENT = '<string_literal>' ]
1832-
let create_row_access = map_res(
1832+
let create_row_access = map(
18331833
rule! {
1834-
CREATE ~ ( OR ~ ^REPLACE )? ~ ROW ~ ACCESS ~ POLICY ~ ( IF ~ ^NOT ~ ^EXISTS )?
1834+
CREATE ~ ROW ~ ACCESS ~ POLICY ~ ( IF ~ ^NOT ~ ^EXISTS )?
18351835
~ #ident ~ #row_access_definition
18361836
~ ( COMMENT ~ ^"=" ~ ^#literal_string )?
18371837
},
1838-
|(_, opt_or_replace, _, _, _, opt_if_not_exists, name, definition, opt_comment)| {
1839-
let create_option =
1840-
parse_create_option(opt_or_replace.is_some(), opt_if_not_exists.is_some())?;
1841-
Ok(Statement::CreateRowAccessPolicy(
1842-
CreateRowAccessPolicyStmt {
1843-
create_option,
1844-
name,
1845-
description: opt_comment.map(|(_, _, description)| description),
1846-
definition,
1847-
},
1848-
))
1838+
|(_, _, _, _, opt_if_not_exists, name, definition, opt_comment)| {
1839+
Statement::CreateRowAccessPolicy(CreateRowAccessPolicyStmt {
1840+
if_not_exists: opt_if_not_exists.is_some(),
1841+
name,
1842+
description: opt_comment.map(|(_, _, description)| description),
1843+
definition,
1844+
})
18491845
},
18501846
);
18511847
let drop_row_access = map(
@@ -2066,19 +2062,17 @@ pub fn statement_body(i: Input) -> IResult<Statement> {
20662062
let show_file_formats = value(Statement::ShowFileFormats, rule! { SHOW ~ FILE ~ FORMATS });
20672063

20682064
// data mark policy
2069-
let create_data_mask_policy = map_res(
2065+
let create_data_mask_policy = map(
20702066
rule! {
2071-
CREATE ~ ( OR ~ ^REPLACE )? ~ MASKING ~ POLICY ~ ( IF ~ ^NOT ~ ^EXISTS )? ~ #ident ~ #data_mask_policy
2067+
CREATE ~ MASKING ~ POLICY ~ ( IF ~ ^NOT ~ ^EXISTS )? ~ #ident ~ #data_mask_policy
20722068
},
2073-
|(_, opt_or_replace, _, _, opt_if_not_exists, name, policy)| {
2074-
let create_option =
2075-
parse_create_option(opt_or_replace.is_some(), opt_if_not_exists.is_some())?;
2069+
|(_, _, _, opt_if_not_exists, name, policy)| {
20762070
let stmt = CreateDatamaskPolicyStmt {
2077-
create_option,
2071+
if_not_exists: opt_if_not_exists.is_some(),
20782072
name: name.to_string(),
20792073
policy,
20802074
};
2081-
Ok(Statement::CreateDatamaskPolicy(stmt))
2075+
Statement::CreateDatamaskPolicy(stmt)
20822076
},
20832077
);
20842078
let drop_data_mask_policy = map(
@@ -2787,7 +2781,7 @@ AS
27872781
[ WEBHOOK = ( url = <string_literal>, method = <string_literal>, authorization_header = <string_literal> ) ]
27882782
[ COMMENT = '<string_literal>' ]`"
27892783
| #create_connection: "`CREATE [OR REPLACE] CONNECTION [IF NOT EXISTS] <connection_name> STORAGE_TYPE = <type> <storage_configs>`"
2790-
| #create_row_access : "`CREATE [ OR REPLACE ] ROW ACCESS POLICY [ IF NOT EXISTS ] <name> AS ( <arg_name> <arg_type> [ , ... ] ) RETURNS BOOLEAN -> <body> [ COMMENT = '<string_literal>' ]`"
2784+
| #create_row_access : "`CREATE ROW ACCESS POLICY [ IF NOT EXISTS ] <name> AS ( <arg_name> <arg_type> [ , ... ] ) RETURNS BOOLEAN -> <body> [ COMMENT = '<string_literal>' ]`"
27912785
| #create_user : "`CREATE [OR REPLACE] USER [IF NOT EXISTS] '<username>' IDENTIFIED [WITH <auth_type>] [BY <password>] [WITH <user_option>, ...]`"
27922786
| #create_role : "`CREATE ROLE [IF NOT EXISTS] <role_name> [COMMENT ='<string_literal>']`"
27932787
| #create_udf : "`CREATE [OR REPLACE] FUNCTION [IF NOT EXISTS] <udf_name> <udf_definition> [DESC = <description>]`"

‎src/query/ast/tests/it/parser.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,6 @@ SELECT * from s;"#,
676676
r#"SELECT * FROM t GROUP BY a, ROLLUP (b, c)"#,
677677
r#"SELECT * FROM t GROUP BY GROUPING SETS ((a, b)), a, ROLLUP (b, c)"#,
678678
r#"CREATE MASKING POLICY email_mask AS (val STRING) RETURNS STRING -> CASE WHEN current_role() IN ('ANALYST') THEN VAL ELSE '*********'END comment = 'this is a masking policy'"#,
679-
r#"CREATE OR REPLACE MASKING POLICY email_mask AS (val STRING) RETURNS STRING -> CASE WHEN current_role() IN ('ANALYST') THEN VAL ELSE '*********'END comment = 'this is a masking policy'"#,
680679
r#"DESC MASKING POLICY email_mask"#,
681680
r#"DROP MASKING POLICY IF EXISTS email_mask"#,
682681
r#"REFRESH VIRTUAL COLUMN FOR t"#,
@@ -974,12 +973,12 @@ SELECT * from s;"#,
974973
r#"ALTER TABLE p1 CONNECTION=(CONNECTION_NAME='test')"#,
975974
r#"ALTER table t connection=(access_key_id ='x' secret_access_key ='y' endpoint_url='http://127.0.0.1:9900')"#,
976975
// row policy
977-
r#"create or replace row access policy rap_it as (empl_id varchar) returns boolean ->
976+
r#"create row access policy rap_it as (empl_id varchar) returns boolean ->
978977
case
979978
when 'it_admin' = current_role() then true
980979
else false
981980
end"#,
982-
r#"create or replace row access policy rap_sales_manager_regions_1 as (sales_region varchar) returns boolean ->
981+
r#"create row access policy if not exists rap_sales_manager_regions_1 as (sales_region varchar) returns boolean ->
983982
'sales_executive_role' = current_role()
984983
or exists (
985984
select 1 from salesmanagerregions

0 commit comments

Comments
 (0)