ddl: support masking policy DDL#66035
Conversation
|
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. |
- Add nil guards in applyCreateMaskingPolicy and applyAlterMaskingPolicy to prevent panic when metadata is missing/stale - Add read lock in MaskingPolicyByID to prevent data race with setMaskingPolicy/deleteMaskingPolicy - Fix TestVersionedBootstrapSchemas to only check table IDs for duplicates, not database IDs (same database can be referenced in multiple versions) Co-Authored-By: Claude <noreply@anthropic.com>
Add version255 to upgrade the bootstrap version for the masking policy system table (mysql.tidb_masking_policy). Co-Authored-By: Claude <noreply@anthropic.com>
The In version 254, the tidb_masking_policy table should not exist yet.
// Skip the test if it table doesn't exist and verify upgrade creates it instead.
Co-Authored-By: Claude <noreply@anthropic.com>
Use = instead of := for exists variable which was already declared above.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughImplements end-to-end masking-policy DDL: model and sys-table schema changes, DDL executor dispatch and job submission, DDL worker handlers for create/alter/drop with sys-table persistence and validation, infoschema lazy caching and loader integration, builder/diff/init updates, and tests/BUILD changes. ChangesMasking Policy DDL Support
Sequence Diagram (high-level flow) sequenceDiagram
participant Client
participant Executor
participant DDLWorker
participant InfoSchema
participant MySQLMeta
Client->>Executor: CREATE MASKING POLICY ...
Executor->>DDLWorker: submit CreateMaskingPolicy job (policy args)
DDLWorker->>InfoSchema: validate target table/column & load masking policies
DDLWorker->>MySQLMeta: INSERT/UPDATE `mysql.tidb_masking_policy`
MySQLMeta-->>DDLWorker: write result (policy_id)
DDLWorker->>InfoSchema: reset/load masking-policy cache
DDLWorker-->>Executor: job finished (schema version)
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The struct and its initializer had declared twice due to a bad merge. This caused a compile error: vet: conflict_resolution_test.go:120:2: tbl redeclared
|
@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. |
|
/ok-to-test |
|
/retest |
# Conflicts: # pkg/infoschema/infoschema.go # pkg/infoschema/infoschema_nokit_test.go
|
/retest |
2 similar comments
|
/retest |
|
/retest |
When a table is renamed, update db_name and table_name in mysql.tidb_masking_policy. When a column is renamed, update column_name, column_id and rewrite the expression.
|
/retest |
…il DDL on policy cleanup error Review fixes: 1. HIGH: Add checkMaskingPolicyOnModifyColumn in executor layer (ModifyColumn/ChangeColumn) to reject type changes to unsupported types (e.g. JSON) for columns with masking policy. Also add safety-net check in syncMaskingPolicyForModifiedColumn (DDL worker layer). Test: TestMaskingPolicyModifyColumnRejectUnsupportedType. 2. MEDIUM: dropMaskingPoliciesOnTable/dropMaskingPoliciesOnColumn errors now return errors.Wrapf instead of logging Warn, so the DDL job will retry/rollback rather than silently leaving stale policy rows in mysql.tidb_masking_policy.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/test pull-integration-realcluster-test-next-gen |
|
@wuhuizuo: The specified target(s) for Use 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. |
|
/retest |
|
PTAL @bb7133 @wjhuang2016 |
|
/retest |
What problem does this PR solve?
Issue Number: ref #65744
Problem Summary:
What changed and how does it work?
Tests:
Stacked PRs
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
Database
Tests