You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "menghaoranss (via GitHub)" <gi...@apache.org> on 2026/08/26 08:38:48 UTC
[PR] Use database default schema in encrypt insert value rewriter [shardingsphere]
menghaoranss opened a new pull request, #39604:
URL: https://github.com/apache/shardingsphere/pull/39604
Fixes #ISSUSE_ID.
Changes proposed in this pull request:
-
---
Before committing this PR, I'm sure that I have checked the following options:
- [ ] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
- [ ] I have self-reviewed the commit code.
- [ ] I have (or in comment I request) added corresponding labels for the pull request.
- [ ] I have passed maven check locally : `./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e`.
- [ ] I have made corresponding changes to the documentation.
- [ ] I have added corresponding unit tests for my changes.
- [ ] I have updated the Release Notes of the current development version. For more details, see [Update Release Note](https://shardingsphere.apache.org/community/en/involved/contribute/contributor/)
- [ ] I have disclosed the tool and affected files or scope for any material AI assistance, as required by the [AI-assisted contribution policy].
[AI-assisted contribution policy]: https://github.com/apache/shardingsphere/blob/master/AI_POLICY.md
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use database default schema in encrypt insert value rewriter [shardingsphere]
Posted by "yx9o (via GitHub)" <gi...@apache.org>.
yx9o commented on PR #39604:
URL: https://github.com/apache/shardingsphere/pull/39604#issuecomment-5447183391
### Result
**Review Result: Mergeable**
No code-correctness blocker was found. The PR switches encrypt insert-value parameter rewriting to use the current `ShardingSphereDatabase` as the default-schema owner, aligning it with the insert-values SQL token generation path, and adds focused coverage for the schema-less insert case.
### Evidence
- Reviewed PR [#39604](https://github.com/apache/shardingsphere/pull/39604) latest head [`9eee65ef461ad83cd27a4c9e1a806018aac0a9b9`](https://github.com/apache/shardingsphere/commit/9eee65ef461ad83cd27a4c9e1a806018aac0a9b9), base `master` `d67a25dd98361c26d3f2526341eaa46e6e96d2d5`.
- `features/encrypt/core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/parameter/EncryptParameterRewritersRegistry.java:53` now passes `sqlRewriteContext.getDatabase()` to `EncryptInsertValueParameterRewriter`; the same PR-head registry passes the database object to the adjacent assignment and on-duplicate rewriters at `:50` and `:54`.
- `features/encrypt/core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/parameter/rewriter/EncryptInsertValueParameterRewriter.java:49` stores `ShardingSphereDatabase`, and `:66-67` resolves schema-less inserts with `database::getDefaultSchemaName`.
- `features/encrypt/core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/parameter/rewriter/EncryptInsertValueParameterRewriter.java:109`, `:112`, and `:115` still pass `database.getName()` into cipher, assisted-query, and like-query encryption, preserving the encrypt algorithm database-name argument.
- `infra/common/src/main/java/org/apache/shardingsphere/infra/metadata/database/ShardingSphereDatabase.java:308-315` owns protocol-aware default schema resolution; `database/connector/core/src/main/java/org/apache/shardingsphere/database/connector/core/type/DatabaseTypeRegistry.java:71-74` marks the old default-schema API as deprecated in favor of it.
- `features/encrypt/core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/insert/EncryptInsertValuesTokenGenerator.java:108-122` already uses `database::getDefaultSchemaName`, so the parameter rewriter now matches the token generator path.
- `features/encrypt/core/src/test/java/org/apache/shardingsphere/encrypt/rewrite/parameter/rewriter/EncryptInsertValueParameterRewriterTest.java:50-60` covers schema-less grouped insert parameter replacement; `:80` makes `TablesContext#getSchemaName()` empty, and the expected cipher call at `:55` verifies `foo_default_schema` is passed to the encrypt algorithm.
### Coverage
- Authoritative GitHub changed-file list accounted for 3 files: `EncryptParameterRewritersRegistry.java`, `EncryptInsertValueParameterRewriter.java`, and added `EncryptInsertValueParameterRewriterTest.java`.
- Public issue comments contained one previous mergeable review for older head `c03d17b0`; it was treated only as prior public context, and latest head `9eee65ef` was reviewed independently. Review comments and reviews endpoints returned empty.
- Behavior clusters reviewed: insert-value parameter default-schema resolution, registry construction compatibility, grouped parameter replacement, encrypt algorithm argument preservation, and focused test validity.
- Discovery lenses completed: Root Cause and Behavior; Blast Radius and Contracts; Tests, Runtime, and Operations. Triggered high-risk lenses checked: shared metadata ownership, test validity, high-frequency rewrite path, and user/runtime impact.
- Code-scope only; CI was not reviewed. Local Maven tests were not run because the PR head is not checked out locally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use database default schema in encrypt insert value rewriter [shardingsphere]
Posted by "yx9o (via GitHub)" <gi...@apache.org>.
yx9o merged PR #39604:
URL: https://github.com/apache/shardingsphere/pull/39604
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use database default schema in encrypt insert value rewriter [shardingsphere]
Posted by "menghaoranss (via GitHub)" <gi...@apache.org>.
menghaoranss commented on PR #39604:
URL: https://github.com/apache/shardingsphere/pull/39604#issuecomment-5435071703
```markdown
### Result
**Review Result: Mergeable**
No actionable correctness, compatibility, or regression findings.
### Evidence
- Reviewed exact head [`c03d17b0`](https://github.com/apache/shardingsphere/commit/c03d17b0cf60acac6fe80f31f292a8d125c6f607).
- Explicit SQL schemas retain precedence; schema-less insert values now use the selected database’s canonical default schema.
- All constructor consumers were checked, and cipher, assisted-query, and like-query encryption retain the same database argument.
- Focused insert-value rewriter and registry tests: 2 tests passed, 0 failures or errors.
- Final refresh confirmed the head, files, and empty discussion state were unchanged.
### Coverage
- All 3 authoritative changed files were reviewed.
- Completed rewrite dispatch, schema ownership, constructor compatibility, batch/grouped parameter behavior, test validity, non-regression, and convergence passes.
- No unresolved evidence or coverage gaps.
- Code-scope result only; CI was not reviewed.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org