pingcap / pingcap/tidb

column masking policy: enhance test coverage for CREATE OR REPLACE MASKING POLICY

Open
#67,219 0 comments 0 reactions 0 assignees View on GitHub
component/privilege sig/planner type/enhancement
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Enhancement Summary

The `CREATE OR REPLACE MASKING POLICY` statement has insufficient test coverage. Only one scenario is tested (replacing an existing policy), while many edge cases and boundary conditions are missing.

## Current Test Coverage

| Scenario | Covered? | Test Case |
|----------|----------|-----------|
| Replace existing policy (update expression) | ✅ Yes | `IT-MASK-P0-001` (line 22-23) |
| OR REPLACE when policy doesn't exist | ❌ No | - |
| OR REPLACE + IF NOT EXISTS (mutually exclusive) | ❌ No | - |
| OR REPLACE modifying only status | ❌ No | - |
| OR REPLACE modifying only restrict_on | ❌ No | - |
| OR REPLACE on different column with same name | ❌ No | - |
| OR REPLACE updates updated_at timestamp | ❌ No | - |
| OR REPLACE on DISABLED policy | ❌ No | - |

## Code Analysis

### Validation logic (pkg/ddl/executor.go:6347-6350)

\`\`\`go
func (e *executor) CreateMaskingPolicy(ctx sessionctx.Context, stmt *ast.CreateMaskingPolicyStmt) error {
if stmt.OrReplace && stmt.IfNotExists {
return dbterror.ErrWrongUsage.GenWithStackByArgs(\"OR REPLACE\", \"IF NOT EXISTS\")
}
...
}
\`\`\`

### Replace logic (pkg/ddl/masking_policy.go:63-82)

\`\`\`go
for _, p := range existPolicy {
if p.Name.L == policyInfo.Name.L {
if p.ColumnID != policyInfo.ColumnID {
// Same name on different column - error
return dbterror.ErrMaskingPolicyExists
}
// Same name on same column - allow replace
if !replaceOnExist {
return dbterror.ErrMaskingPolicyExists
}
// Replace: updates ALL attributes
replacePolicy.Expression = policyInfo.Expression
replacePolicy.Status = policyInfo.Status
replacePolicy.MaskingType = policyInfo.MaskingType
replacePolicy.RestrictOps = policyInfo.RestrictOps
replacePolicy.UpdatedAt = policyInfo.UpdatedAt
\`\`\`

## Missing Test Scenarios

### 1. OR REPLACE when policy doesn't exist

\`\`\`sql
CREATE TABLE t1(c VARCHAR(20));
-- Should succeed (creates new policy)
CREATE OR REPLACE MASKING POLICY p_new ON t1(c) AS MASK_FULL(c, '*') ENABLE;
\`\`\`

**Expected**: Should succeed and create the policy (same as plain CREATE).

### 2. OR REPLACE + IF NOT EXISTS (mutually exclusive)

\`\`\`sql
-- Should error: OR REPLACE and IF NOT EXISTS are mutually exclusive
CREATE OR REPLACE MASKING POLICY IF NOT EXISTS p_test ON t1(c) AS c;
\`\`\`

**Expected**: Error `ErrWrongUsage` with message about OR REPLACE and IF NOT EXISTS being mutually exclusive.

### 3. OR REPLACE modifying only status

\`\`\`sql
CREATE MASKING POLICY p_test ON t1(c) AS MASK_FULL(c, '*') ENABLE;
-- Only change status, keep expression
CREATE OR REPLACE MASKING POLICY p_test ON t1(c) AS MASK_FULL(c, '*') DISABLE;
SHOW MASKING POLICIES FOR t1;
\`\`\`

**Expected**: Expression should remain the same, status changed to DISABLED.

### 4. OR REPLACE adding RESTRICT ON

\`\`\`sql
CREATE MASKING POLICY p_test ON t1(c) AS c ENABLE;
-- Add restrict_on to existing policy
CREATE OR REPLACE MASKING POLICY p_test ON t1(c)
AS c RESTRICT ON (INSERT_INTO_SELECT) ENABLE;
\`\`\`

**Expected**: RestrictOn should be updated.

### 5. OR REPLACE on different column with same name

\`\`\`sql
CREATE TABLE t2(c1 VARCHAR(20), c2 VARCHAR(20));
CREATE MASKING POLICY p_test ON t2(c1) AS c ENABLE;
-- Should error: same policy name on different column
CREATE OR REPLACE MASKING POLICY p_test ON t2(c2) AS c ENABLE;
\`\`\`

**Expected**: Error `ErrMaskingPolicyExists` (8268).

### 6. Verify updated_at timestamp

\`\`\`sql
CREATE MASKING POLICY p_test ON t1(c) AS c ENABLE;
-- Check updated_at
SELECT created_at, updated_at FROM mysql.tidb_masking_policy WHERE policy_name = 'p_test';
-- Wait or use a different expression
CREATE OR REPLACE MASKING POLICY p_test ON t1(c) AS MASK_FULL(c, '*') ENABLE;
-- Verify updated_at changed
SELECT created_at, updated_at FROM mysql.tidb_masking_policy WHERE policy_name = 'p_test';
\`\`\`

**Expected**: `updated_at` should be newer than `created_at`, but `created_at` unchanged.

### 7. OR REPLACE on DISABLED policy

\`\`\`sql
CREATE MASKING POLICY p_test ON t1(c) AS MASK_FULL(c, '*') DISABLE;
-- Replace with ENABLED
CREATE OR REPLACE MASKING POLICY p_test ON t1(c) AS MASK_FULL(c, '*') ENABLE;
SELECT c FROM t1; -- Should return masked value
\`\`\`

**Expected**: Policy should be enabled and masking applied.

## Design Doc Reference

From `docs/design/2026-02-27-column-level-masking.md`:

> Rules:
> - `OR REPLACE` and `IF NOT EXISTS` are mutually exclusive.

## Proposed Test Addition

Add a new test case `IT-MASK-P0-013 TestColumnMaskPolicyCreateOrReplace` to cover all the missing scenarios above.

## Component

- `component/privilege`
- `sig/planner`

## Related Files

- Test file: `tests/integrationtest/t/privilege/column_masking_policy.test`
- Implementation: `pkg/ddl/executor.go` (CreateMaskingPolicy function)
- Implementation: `pkg/ddl/masking_policy.go` (onCreateMaskingPolicy function)
- Design doc: `docs/design/2026-02-27-column-level-masking.md`
- Related issues: #67217, #67218

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.