Skip to content

Commit 3e407f2

Browse files
TCeasonTCeason
committed
fix
1 parent c4f2c39 commit 3e407f2

File tree

14 files changed

+76
-56
lines changed

14 files changed

+76
-56
lines changed

‎Cargo.lock‎

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

‎src/meta/api/src/data_mask_api.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,26 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
use databend_common_meta_app::data_mask::data_mask_name_ident;
1516
use databend_common_meta_app::data_mask::CreateDatamaskReply;
1617
use databend_common_meta_app::data_mask::CreateDatamaskReq;
1718
use databend_common_meta_app::data_mask::DataMaskId;
1819
use databend_common_meta_app::data_mask::DataMaskNameIdent;
1920
use databend_common_meta_app::data_mask::DatamaskMeta;
2021
use databend_common_meta_app::tenant::Tenant;
22+
use databend_common_meta_app::tenant_key::errors::ExistError;
2123
use databend_common_meta_types::MetaError;
2224
use databend_common_meta_types::SeqV;
2325

2426
use crate::errors::MaskingPolicyError;
25-
use crate::kv_app_error::KVAppError;
2627
use crate::meta_txn_error::MetaTxnError;
2728

2829
#[async_trait::async_trait]
2930
pub trait DatamaskApi: Send + Sync {
3031
async fn create_data_mask(
3132
&self,
3233
req: CreateDatamaskReq,
33-
) -> Result<CreateDatamaskReply, KVAppError>;
34+
) -> Result<Result<CreateDatamaskReply, ExistError<data_mask_name_ident::Resource>>, MetaError>;
3435

3536
/// On success, returns the dropped id and data mask.
3637
/// Returning None, means nothing is removed.

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

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
use databend_common_meta_app::app_error::AppError;
16-
use databend_common_meta_app::app_error::TxnRetryMaxTimes;
15+
use databend_common_meta_app::data_mask::data_mask_name_ident;
1716
use databend_common_meta_app::data_mask::CreateDatamaskReply;
1817
use databend_common_meta_app::data_mask::CreateDatamaskReq;
1918
use databend_common_meta_app::data_mask::DataMaskId;
@@ -28,6 +27,7 @@ use databend_common_meta_app::id_generator::IdGenerator;
2827
use databend_common_meta_app::row_access_policy::RowAccessPolicyNameIdent;
2928
use databend_common_meta_app::schema::CreateOption;
3029
use databend_common_meta_app::tenant::Tenant;
30+
use databend_common_meta_app::tenant_key::errors::ExistError;
3131
use databend_common_meta_app::KeyWithTenant;
3232
use databend_common_meta_kvapi::kvapi;
3333
use databend_common_meta_kvapi::kvapi::DirName;
@@ -40,7 +40,6 @@ use log::debug;
4040
use crate::data_mask_api::DatamaskApi;
4141
use crate::errors::MaskingPolicyError;
4242
use crate::fetch_id;
43-
use crate::kv_app_error::KVAppError;
4443
use crate::kv_pb_api::KVPbApi;
4544
use crate::meta_txn_error::MetaTxnError;
4645
use crate::txn_backoff::txn_backoff;
@@ -57,7 +56,8 @@ impl<KV: kvapi::KVApi<Error = MetaError>> DatamaskApi for KV {
5756
async fn create_data_mask(
5857
&self,
5958
req: CreateDatamaskReq,
60-
) -> Result<CreateDatamaskReply, KVAppError> {
59+
) -> Result<Result<CreateDatamaskReply, ExistError<data_mask_name_ident::Resource>>, MetaError>
60+
{
6161
debug!(req :? =(&req); "DatamaskApi: {}", func_name!());
6262

6363
let name_ident = &req.name;
@@ -67,10 +67,9 @@ impl<KV: kvapi::KVApi<Error = MetaError>> DatamaskApi for KV {
6767
name_ident.data_mask_name().to_string(),
6868
);
6969
if self.get_pb(&row_access_name_ident).await?.is_some() {
70-
return Err(AppError::DatamaskAlreadyExists(
71-
name_ident.exist_error("name conflicts with an existing row access policy"),
72-
)
73-
.into());
70+
return Ok(Err(
71+
name_ident.exist_error("name conflicts with an existing masking policy")
72+
));
7473
}
7574

7675
let masking_policy_id = fetch_id(self, IdGenerator::data_mask_id()).await?;
@@ -85,13 +84,12 @@ impl<KV: kvapi::KVApi<Error = MetaError>> DatamaskApi for KV {
8584
if let Some((seq_id, seq_meta)) = res {
8685
match req.create_option {
8786
CreateOption::Create => {
88-
return Err(AppError::DatamaskAlreadyExists(
89-
name_ident.exist_error(func_name!()),
90-
)
91-
.into());
87+
return Ok(Err(
88+
name_ident.exist_error(format!("{} already exists", req.name))
89+
));
9290
}
9391
CreateOption::CreateIfNotExists => {
94-
return Ok(CreateDatamaskReply { id: *seq_id.data });
92+
return Ok(Ok(CreateDatamaskReply { id: *seq_id.data }));
9593
}
9694
CreateOption::CreateOrReplace => {
9795
let id_ident = seq_id.data.into_t_ident(name_ident.tenant());
@@ -142,11 +140,13 @@ impl<KV: kvapi::KVApi<Error = MetaError>> DatamaskApi for KV {
142140
);
143141

144142
if succ {
145-
Ok(CreateDatamaskReply {
143+
Ok(Ok(CreateDatamaskReply {
146144
id: masking_policy_id,
147-
})
145+
}))
148146
} else {
149-
Err(KVAppError::from(TxnRetryMaxTimes::new(func_name!(), 1)))
147+
Ok(Err(
148+
name_ident.exist_error(format!("{} already exists", req.name))
149+
))
150150
}
151151
}
152152

@@ -185,7 +185,6 @@ impl<KV: kvapi::KVApi<Error = MetaError>> DatamaskApi for KV {
185185
// Policy is in use - cannot drop
186186
if !table_policy_refs.is_empty() {
187187
return Ok(Err(MaskingPolicyError::policy_in_use(
188-
tenant.tenant_name().to_string(),
189188
name_ident.data_mask_name().to_string(),
190189
)));
191190
}

‎src/meta/api/src/errors.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ pub enum MaskingPolicyError {
4747
}
4848

4949
impl MaskingPolicyError {
50-
pub fn policy_in_use(_tenant: impl Into<String>, policy_name: impl Into<String>) -> Self {
50+
pub fn policy_in_use(policy_name: impl Into<String>) -> Self {
5151
Self::PolicyInUse {
5252
policy_name: policy_name.into(),
5353
}
@@ -72,7 +72,7 @@ pub enum RowAccessPolicyError {
7272
}
7373

7474
impl RowAccessPolicyError {
75-
pub fn policy_in_use(_tenant: impl Into<String>, policy_name: impl Into<String>) -> Self {
75+
pub fn policy_in_use(policy_name: impl Into<String>) -> Self {
7676
Self::PolicyInUse {
7777
policy_name: policy_name.into(),
7878
}

‎src/meta/api/src/row_access_policy_api.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ pub trait RowAccessPolicyApi: Send + Sync {
3333
req: CreateRowAccessPolicyReq,
3434
) -> Result<
3535
Result<CreateRowAccessPolicyReply, ExistError<row_access_policy_name_ident::Resource>>,
36-
MetaTxnError,
36+
MetaError,
3737
>;
3838

3939
/// On success, returns the dropped id and row policy.

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
use databend_common_meta_app::app_error::TxnRetryMaxTimes;
1615
use databend_common_meta_app::data_mask::DataMaskNameIdent;
1716
use databend_common_meta_app::id_generator::IdGenerator;
1817
use databend_common_meta_app::row_access_policy::row_access_policy_name_ident;
@@ -56,7 +55,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> RowAccessPolicyApi for KV {
5655
req: CreateRowAccessPolicyReq,
5756
) -> Result<
5857
Result<CreateRowAccessPolicyReply, ExistError<row_access_policy_name_ident::Resource>>,
59-
MetaTxnError,
58+
MetaError,
6059
> {
6160
debug!(req :? =(&req); "RowAccessPolicyApi: {}", func_name!());
6261

@@ -129,10 +128,9 @@ impl<KV: kvapi::KVApi<Error = MetaError>> RowAccessPolicyApi for KV {
129128
if succ {
130129
Ok(Ok(CreateRowAccessPolicyReply { id: row_access_id }))
131130
} else {
132-
Err(MetaTxnError::TxnRetryMaxTimes(TxnRetryMaxTimes::new(
133-
func_name!(),
134-
1,
135-
)))
131+
Ok(Err(
132+
name_ident.exist_error(format!("{} already exists", req.name))
133+
))
136134
}
137135
}
138136

@@ -171,7 +169,6 @@ impl<KV: kvapi::KVApi<Error = MetaError>> RowAccessPolicyApi for KV {
171169
// Policy is in use - cannot drop
172170
if !table_policy_refs.is_empty() {
173171
return Ok(Err(RowAccessPolicyError::policy_in_use(
174-
tenant.tenant_name().to_string(),
175172
name_ident.row_access_name().to_string(),
176173
)));
177174
}

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3195,7 +3195,7 @@ impl SchemaApiTestSuite {
31953195
update_on: None,
31963196
},
31973197
};
3198-
mt.create_data_mask(req).await?;
3198+
mt.create_data_mask(req).await.unwrap().unwrap();
31993199

32003200
let mask1_name_ident = DataMaskNameIdent::new(tenant.clone(), mask_name_1.to_string());
32013201
mask1_id = get_kv_u64_data(mt, &mask1_name_ident).await?;
@@ -3212,7 +3212,7 @@ impl SchemaApiTestSuite {
32123212
update_on: None,
32133213
},
32143214
};
3215-
mt.create_data_mask(req).await?;
3215+
mt.create_data_mask(req).await.unwrap().unwrap();
32163216

32173217
let mask2_name_ident = DataMaskNameIdent::new(tenant.clone(), mask_name_2.to_string());
32183218
mask2_id = get_kv_u64_data(mt, &mask2_name_ident).await?;
@@ -3473,7 +3473,7 @@ impl SchemaApiTestSuite {
34733473
update_on: None,
34743474
},
34753475
};
3476-
mt.create_data_mask(req).await?;
3476+
mt.create_data_mask(req).await.unwrap().unwrap();
34773477
let old_id: u64 = get_kv_u64_data(mt, &name).await?;
34783478

34793479
let id_key = DataMaskIdIdent::new(&tenant, old_id);
@@ -3493,7 +3493,7 @@ impl SchemaApiTestSuite {
34933493
update_on: None,
34943494
},
34953495
};
3496-
mt.create_data_mask(req).await?;
3496+
mt.create_data_mask(req).await.unwrap().unwrap();
34973497

34983498
// assert old id key has been deleted
34993499
let meta: Result<DatamaskMeta, KVAppError> = get_kv_data(mt, &id_key).await;
@@ -3898,7 +3898,9 @@ impl SchemaApiTestSuite {
38983898
update_on: None,
38993899
},
39003900
})
3901-
.await?;
3901+
.await
3902+
.unwrap()
3903+
.unwrap();
39023904
let mask_cleanup_id = get_kv_u64_data(mt, &mask_cleanup_ident).await?;
39033905

39043906
let set_req = SetTableColumnMaskPolicyReq {
@@ -3961,7 +3963,9 @@ impl SchemaApiTestSuite {
39613963
update_on: None,
39623964
},
39633965
})
3964-
.await?;
3966+
.await
3967+
.unwrap()
3968+
.unwrap();
39653969
let mask_guard_id = get_kv_u64_data(mt, &mask_guard_ident).await?;
39663970

39673971
table_info = util.get_table().await?;

‎src/query/ee/src/data_mask/data_mask_handler.rs‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,16 @@ use databend_common_base::base::GlobalInstance;
1818
use databend_common_exception::Result;
1919
use databend_common_meta_api::DatamaskApi;
2020
use databend_common_meta_app::app_error::AppError;
21+
use databend_common_meta_app::data_mask::data_mask_name_ident::Resource;
22+
use databend_common_meta_app::data_mask::CreateDatamaskReply;
2123
use databend_common_meta_app::data_mask::CreateDatamaskReq;
2224
use databend_common_meta_app::data_mask::DataMaskNameIdent;
2325
use databend_common_meta_app::data_mask::DatamaskMeta;
2426
use databend_common_meta_app::data_mask::DropDatamaskReq;
2527
use databend_common_meta_app::tenant::Tenant;
28+
use databend_common_meta_app::tenant_key::errors::ExistError;
2629
use databend_common_meta_store::MetaStore;
30+
use databend_common_meta_types::MetaError;
2731
use databend_common_meta_types::SeqV;
2832
use databend_enterprise_data_mask_feature::data_mask_handler::DatamaskHandler;
2933
use databend_enterprise_data_mask_feature::data_mask_handler::DatamaskHandlerWrapper;
@@ -36,10 +40,11 @@ impl DatamaskHandler for RealDatamaskHandler {
3640
&self,
3741
meta_api: Arc<MetaStore>,
3842
req: CreateDatamaskReq,
39-
) -> Result<()> {
40-
let _ = meta_api.create_data_mask(req).await?;
41-
42-
Ok(())
43+
) -> std::result::Result<
44+
std::result::Result<CreateDatamaskReply, ExistError<Resource>>,
45+
MetaError,
46+
> {
47+
meta_api.create_data_mask(req).await
4348
}
4449

4550
async fn drop_data_mask(&self, meta_api: Arc<MetaStore>, req: DropDatamaskReq) -> Result<()> {

‎src/query/ee/src/row_access_policy/row_access_policy_handler.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ use std::sync::Arc;
1616

1717
use databend_common_base::base::GlobalInstance;
1818
use databend_common_exception::Result;
19-
use databend_common_meta_api::meta_txn_error::MetaTxnError;
2019
use databend_common_meta_api::RowAccessPolicyApi;
2120
use databend_common_meta_app::app_error::AppError;
2221
use databend_common_meta_app::row_access_policy::row_access_policy_name_ident::Resource;
@@ -29,6 +28,7 @@ use databend_common_meta_app::row_access_policy::RowAccessPolicyNameIdent;
2928
use databend_common_meta_app::tenant::Tenant;
3029
use databend_common_meta_app::tenant_key::errors::ExistError;
3130
use databend_common_meta_store::MetaStore;
31+
use databend_common_meta_types::MetaError;
3232
use databend_common_meta_types::SeqV;
3333
use databend_enterprise_row_access_policy_feature::row_access_policy_handler::RowAccessPolicyHandler;
3434
use databend_enterprise_row_access_policy_feature::row_access_policy_handler::RowAccessPolicyHandlerWrapper;
@@ -43,7 +43,7 @@ impl RowAccessPolicyHandler for RealRowAccessPolicyHandler {
4343
req: CreateRowAccessPolicyReq,
4444
) -> std::result::Result<
4545
std::result::Result<CreateRowAccessPolicyReply, ExistError<Resource>>,
46-
MetaTxnError,
46+
MetaError,
4747
> {
4848
meta_api.create_row_access_policy(req).await
4949
}

‎src/query/ee_features/data_mask/src/data_mask_handler.rs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,15 @@ use std::sync::Arc;
3030

3131
use databend_common_base::base::GlobalInstance;
3232
use databend_common_exception::Result;
33+
use databend_common_meta_app::data_mask::data_mask_name_ident::Resource;
34+
use databend_common_meta_app::data_mask::CreateDatamaskReply;
3335
use databend_common_meta_app::data_mask::CreateDatamaskReq;
3436
use databend_common_meta_app::data_mask::DatamaskMeta;
3537
use databend_common_meta_app::data_mask::DropDatamaskReq;
3638
use databend_common_meta_app::tenant::Tenant;
39+
use databend_common_meta_app::tenant_key::errors::ExistError;
3740
use databend_common_meta_store::MetaStore;
41+
use databend_common_meta_types::MetaError;
3842
use databend_common_meta_types::SeqV;
3943

4044
#[async_trait::async_trait]
@@ -43,7 +47,10 @@ pub trait DatamaskHandler: Sync + Send {
4347
&self,
4448
meta_api: Arc<MetaStore>,
4549
req: CreateDatamaskReq,
46-
) -> Result<()>;
50+
) -> std::result::Result<
51+
std::result::Result<CreateDatamaskReply, ExistError<Resource>>,
52+
MetaError,
53+
>;
4754

4855
async fn drop_data_mask(&self, meta_api: Arc<MetaStore>, req: DropDatamaskReq) -> Result<()>;
4956

@@ -75,7 +82,10 @@ impl DatamaskHandlerWrapper {
7582
&self,
7683
meta_api: Arc<MetaStore>,
7784
req: CreateDatamaskReq,
78-
) -> Result<()> {
85+
) -> std::result::Result<
86+
std::result::Result<CreateDatamaskReply, ExistError<Resource>>,
87+
MetaError,
88+
> {
7989
self.handler.create_data_mask(meta_api, req).await
8090
}
8191

0 commit comments

Comments
 (0)