Conversation
|
Skipping CI for Draft Pull Request. |
|
Hi @tiancaiamao. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
# Conflicts: # pkg/session/upgrade_def.go
|
/retest |
|
@tiancaiamao: PRs from untrusted users cannot be marked as trusted with DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fzzf678 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test pull-br-integration-test |
|
@tiancaiamao: PRs from untrusted users cannot be marked as trusted with DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
…WHERE/HAVING/GROUP BY This commit fixes a critical bug where masking policies in CTEs were incorrectly applied during CTE definition building, causing WHERE/HAVING/GROUP BY/ORDER BY clauses to see masked values instead of original values. Changes: - buildSelect/buildSetOpr: Skip masking when building CTE definitions by checking !b.isCTE && !b.buildingCTE - buildDataSourceFromCTEMerge: Set both b.isCTE and b.buildingCTE flags to ensure CTE context is maintained during inline merge - Preserve OrigTblName and OrigColName for masking policy lookup AT RESULT semantics: Masking is now correctly applied only at final output, not during intermediate operations. Issue: pingcap#67341 Co-Authored-By: Claude <noreply@anthropic.com>
This commit updates the column masking auto-validation skill to include the new CTE masking test case. Changes: - Add tests/integrationtest/t/privilege/column_masking_cte.test to test plan - Add P0-CORE-03 scenario for AT RESULT semantics with CTE - Add it_column_masking_cte step to validation script Co-Authored-By: Claude <noreply@anthropic.com>
The buildingCTE flag causes buildDistinct to access b.outerCTEs[len-1] which fails when b.outerCTEs has been truncated (as it is before calling buildDataSourceFromCTEMerge). Only setting b.isCTE is sufficient for the masking fix and avoids the index out of range panic. This fixes the collation_misc test failure with CTE UNION queries.
The original truncation of b.outerCTEs in tryBuildCTE caused buildDistinct to fail with "index out of range [-1]" when accessing b.outerCTEs[len-1]. The fix is to NOT truncate b.outerCTEs when calling buildDataSourceFromCTEMerge, which allows buildDistinct to access it safely when b.buildingCTE is true. This fixes both: 1. collation_misc test failure with CTE UNION queries 2. column_masking_cte test behavior with HAVING/ORDER BY using original values The masking fix (checking !b.isCTE && !b.buildingCTE) still works correctly because b.isCTE is set to true in buildDataSourceFromCTEMerge, which skips masking during CTE definition building.
…E masking policy improvements
… skip ID check for CTE columns, set buildingCTE in buildDataSourceFromCTEMerge
- Fix buildDataSourceFromCTEMerge to set both b.isCTE and b.buildingCTE to prevent masking during CTE definition building (buildTableRefs overwrites b.isCTE via buildResultSetNode(false), so b.buildingCTE serves as backup) - Fix adjustCTEPlanOutputName to preserve OrigTblName before overwriting with CTE name, so findMaskingPolicy can look up the original table - Fix findMaskingPolicy to skip column ID check for CTE-derived columns (OrigTblName != TblName) since getResultCTESchema reallocates IDs - Fix tryBuildCTE inline merge to truncate b.outerCTEs and set b.buildingCTE=false before calling buildDataSourceFromCTEMerge - Fix mask_partial function signature: change from (str, pad, start, length) to (str, preserveLeft, preserveRight, pad) to match policy definition MASK_PARTIAL(col, prefix_keep, suffix_keep, mask_char) - Clear masking policy expression cache on SET ROLE (current_role() is session-dependent but cache only invalidated on schema version)
|
[FORMAT CHECKER NOTIFICATION] Notice: To remove the For example:
📖 For more info, you can check the "Contribute Code" section in the development guide. |
|
@tiancaiamao: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What problem does this PR solve?
Issue Number: ref #65744
Problem Summary:
This is the implemention for server side column masking feature, I'll finish a demo fist in this PR.
And later, it will be split into small commits.
What changed and how does it work?
Currently most of the code are finished by AI, I just review the code and verify the result.
I keep a commit for each phase, so later it can be easier break into small PRs.
Phase 1: Parser and AST
CreateMaskingPolicyStmtAlterTablespecs forADD/ENABLE/DISABLE/DROP MASKING POLICYShowStmtTypeforSHOW MASKING POLICIESpkg/parser/parser.yfor new grammar.pkg/parser/ast/ddl.goandpkg/parser/ast/dml.go.Phase 2: Metadata Model and DDL Job Types
MaskingPolicyInfoinpkg/meta/model(new file) with IDs, names, expression, status, audit fields.pkg/meta/model/job.goandpkg/meta/model/job_args.go.meta.MutatorAPIs for create/update/drop/list policies with new meta keys (avoid colliding with placement policy keys).Phase 3: System Table
CreateTiDBMaskingPolicyTabletopkg/meta/metadef/system_tables_def.go.pkg/session/bootstrap.go,pkg/session/upgrade_def.go).policy_id BIGINT PRIMARY KEYpolicy_name VARCHAR(64) NOT NULLdb_name VARCHAR(64) NOT NULLtable_name VARCHAR(64) NOT NULLtable_id BIGINT NOT NULLcolumn_name VARCHAR(64) NOT NULLcolumn_id BIGINT NOT NULLexpression TEXT NOT NULLstatus ENUM('ENABLE','DISABLE') DEFAULT 'ENABLE'function_type VARCHAR(32)(FULL/PARTIAL/NULL/CUSTOM)created_at TIMESTAMP DEFAULT NOW()updated_at TIMESTAMP DEFAULT NOW()created_by VARCHAR(128)UNIQUE KEY (db_name, policy_name)INDEX (table_id, column_id)Phase 4: InfoSchema Integration
infoSchemaMiscwith amaskingPolicyMapkeyed by table_id/column_id and by policy name.InfoSchemaandinfoschema/context:MaskingPolicyByName/MaskingPolicyByTableColumn/AllMaskingPolicies.infoschema.Builderapply-diff paths for create/alter/drop policy jobs.Phase 5: DDL Execution and Validation
pkg/ddl/masking_policy.go:pkg/ddl/job_worker.goand DDL executor.IF NOT EXISTS/OR REPLACEerror rules.GetModifiableColumnJob.TRUNCATE TABLEupdatestable_idin policy entries.RENAME TABLE/RENAME COLUMNupdates names in policy entries.mysql.tidb_masking_policyin job handlers.Phase 6: SHOW Statements (moved forward)
SHOW MASKING POLICIES FOR t [WHERE column='c']inpkg/executor/show.go.SHOW CREATE TABLEoutput to append:/* MASKING POLICYnameENABLE */per column (policy name + status only).Phase 7: Expression Parsing and Caching
expression.ParseSimpleExprwith schema containing only the target column.Phase 8: Planner Rewrite (Apply Masking)
buildProjection, substitute column references with the policy expression usingexpression.ColumnSubstitute.Phase 9: Masking Builtins
MASK_PARTIAL,MASK_FULL,MASK_NULL,MASK_DATEinpkg/expressionand register inbuiltin.go.Phase 10: Privileges and Errors
CREATE MASKING POLICYALTER MASKING POLICYDROP MASKING POLICYpkg/errnoandpkg/util/dbterror(patterned after placement policy errors).Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
Summary by CodeRabbit
New Features
Documentation
Tests