pingcap / pingcap/tidb

column masking policy: MASK_FULL has incorrect signature - should not require mask_char parameter

Open
#67,224 2 comments 0 reactions 0 assignees View on GitHub
component/ddl component/expression severity/moderate type/bug
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Bug Report

The \`MASK_FULL\` function has an incorrect signature. It currently requires a \`mask_char\` parameter, but according to the design specification, it should use type-specific defaults and accept only one parameter (the column to mask).

## Design Specification

According to the design document and expected behavior:

| Function | Signature | Description |
|----------|-----------|-------------|
| \`MASK_FULL\` | \`MASK_FULL(col)\` | Replaces the entire value with a type-specific default |

**Expected behavior by type:**
- **String** → Returns \`'X'\` repeated to match the original length
- **Number/Integer** → Returns \`0\`
- **Date/DateTime** → Returns \`'1970-01-01'\` or \`'1970-01-01 00:00:00'\`
- **Duration** → Returns \`'00:00:00'\`

**Example usage:**
\`\`\`sql
-- CORRECT (one parameter)
SELECT MASK_FULL(customer_id);
-- Result: XXXXXXXXXXXX (for a 12-character ID)

-- CORRECT
SELECT MASK_FULL(salary_amount);
-- Result: 0
\`\`\`

## Current (Incorrect) Implementation

In \`pkg/expression/builtin_masking.go\`, line 35:

\`\`\`go
// Current implementation - WRONG (requires 2 parameters)
bf, err := newBaseBuiltinFuncWithTp(ctx, c.funcName, args, evalTp, evalTp, types.ETString)
\`\`\`

And line 84:
\`\`\`go
// Current implementation uses args[1] for mask character
mask, isNull, err := b.args[1].EvalString(ctx, row)
\`\`\`

**Current behavior (WRONG):**
\`\`\`sql
-- Current INCORRECT syntax (requires 2 parameters)
SELECT MASK_FULL(customer_id, '*');
\`\`\`

## Impact

1. **API mismatch**: The API doesn't match the design specification
2. **Poor UX**: Users must specify a mask character when it should be type-specific
3. **Inconsistency**: Different types should have different default masks, not require user input

## Fix Required

### 1. Change the function signature to accept only one parameter

In \`pkg/expression/builtin_masking.go\`, line 35:

**Current (incorrect):**
\`\`\`go
bf, err := newBaseBuiltinFuncWithTp(ctx, c.funcName, args, evalTp, evalTp, types.ETString)
\`\`\`

**Should be:**
\`\`\`go
bf, err := newBaseBuiltinFuncWithTp(ctx, c.funcName, args, evalTp)
\`\`\`

### 2. Update implementation to use type-specific defaults

#### For strings (builtinMaskFullStringSig)

**Current (line 79-92):**
\`\`\`go
func (b *builtinMaskFullStringSig) evalString(...) (string, bool, error) {
str, isNull, err := b.args[0].EvalString(ctx, row)
// ...
mask, isNull, err := b.args[1].EvalString(ctx, row) // ← Wrong: uses args[1]
// ...
return strings.Repeat(string(maskRunes[0]), len([]rune(str))), false, nil
}
\`\`\`

**Should be:**
\`\`\`go
func (b *builtinMaskFullStringSig) evalString(...) (string, bool, error) {
str, isNull, err := b.args[0].EvalString(ctx, row)
if isNull || err != nil {
return "", true, err
}
// Use fixed 'X' character for string types
return strings.Repeat("X", len([]rune(str))), false, nil
}
\`\`\`

#### For integers (builtinMaskFullIntSig)

**Current (line 187-192):**
\`\`\`go
func (b *builtinMaskFullIntSig) evalInt(...) (int64, bool, error) {
_, isNull, err := b.args[0].EvalInt(ctx, row)
// ...
return 1970, false, nil // ← Wrong: returns 1970 (for YEAR type)
}
\`\`\`

**Should be:**
\`\`\`go
func (b *builtinMaskFullIntSig) evalInt(...) (int64, bool, error) {
_, isNull, err := b.args[0].EvalInt(ctx, row)
if isNull || err != nil {
return 0, true, err
}
return 0, false, nil // Return 0 for numeric masking
}
\`\`\`

#### For datetime (builtinMaskFullTimeSig)

**Current implementation is already correct:**
Returns \`1970-01-01\` for dates and \`1970-01-01 00:00:00\` for datetime (line 144-146)

## Test Cases Needed

\`\`\`sql
-- String masking (should use 'X' by default)
CREATE TABLE t1(id INT, ssn VARCHAR(20));
INSERT INTO t1 VALUES (1, '123456789');
CREATE MASKING POLICY p_ssn ON t1(ssn) AS MASK_FULL(ssn) ENABLE;
SELECT ssn FROM t1;
-- Expected: XXXXXXXXX (all X's)

-- Number masking (should return 0)
CREATE TABLE t2(id INT, salary BIGINT);
INSERT INTO t2 VALUES (1, 50000);
CREATE MASKING POLICY p_salary ON t2(salary) AS MASK_FULL(salary) ENABLE;
SELECT salary FROM t2;
-- Expected: 0

-- Date masking (should return 1970-01-01)
CREATE TABLE t3(id INT, birth_date DATE);
INSERT INTO t3 VALUES (1, '1990-05-15');
CREATE MASKING POLICY p_date ON t3(birth_date) AS MASK_FULL(birth_date) ENABLE;
SELECT birth_date FROM t3;
-- Expected: 1970-01-01
\`\`\`

## Component

- \`component/expression\`
- \`component/ddl\`

## Related Files

- Implementation: \`pkg/expression/builtin_masking.go\` (lines 25-193)
- Test: \`pkg/expression/builtin_masking_test.go\`
- Design: \`docs/design/2026-02-27-column-level-masking.md\`

## Related Issues

- #67222: MASK_PARTIAL has incorrect type signature (similar issue)
- #67221: Dynamic privileges not implemented
- #67219: CREATE OR REPLACE test coverage
- #67218: CTE test coverage
- #67217: Policy name uniqueness test coverage

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.