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/07/23 04:16:18 UTC
[PR] Replace StandardIdentifierCasePolicy with IdentifierCasePolicy class [shardingsphere]
menghaoranss opened a new pull request, #39219:
URL: https://github.com/apache/shardingsphere/pull/39219
Changes proposed in this pull request:
- Replace StandardIdentifierCasePolicy with IdentifierCasePolicy class
---
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/)
--
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] Replace StandardIdentifierCasePolicy with IdentifierCasePolicy class [shardingsphere]
Posted by "menghaoranss (via GitHub)" <gi...@apache.org>.
menghaoranss merged PR #39219:
URL: https://github.com/apache/shardingsphere/pull/39219
--
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] Replace StandardIdentifierCasePolicy with IdentifierCasePolicy class [shardingsphere]
Posted by "menghaoranss (via GitHub)" <gi...@apache.org>.
menghaoranss commented on PR #39219:
URL: https://github.com/apache/shardingsphere/pull/39219#issuecomment-5054879564
```markdown
### Correction
- **Previous Finding:** Removing null handling from `IdentifierCasePolicy` causes a behavioral regression.
- **Current Status:** Withdrawn
- **Reason:** Repository conventions require method parameters to be non-null, and no production path at the latest head passes null to `IdentifierCasePolicy.normalize` or `matches`. The fact that `IdentifierValue` can represent null does not extend the policy’s input contract.
### Summary
**Review Result: Mergeable**
**Reason:** No code-level blocker was found in the latest reviewed scope. This result is code-scope only; CI not reviewed by request.
### Evidence
- `StandardIdentifierCasePolicy` was the only repository implementation of `IdentifierCasePolicy`; merging the interface and its sole implementation appropriately simplifies the design.
- Constructor parameters, quoted and unquoted lookup modes, normalizers, and stored-name predicates retain their behavior for valid inputs.
- Changes to `IdentifierCasePolicyFactory` and the Firebird provider only replace the constructed type without altering database identifier policies.
- No references to `StandardIdentifierCasePolicy` remain at the latest head.
- Removing null-input tests is consistent with the repository’s non-null parameter contract and does not remove supported behavior.
- No unrelated production code, dependencies, configuration, distribution files, or documentation are changed.
### Review Details
- **Review Focus:** Code Correctness Review.
- CI not reviewed by request.
- **Reviewed Scope:** All 11 changed files across `database/connector/core`, `database/connector/dialect/firebird`, `infra/common`, and `kernel/single/core`. Latest head: `781a105702701857dc058f6922f919e758496ce9`; base and merge-base: `da096439aa116e5f15cebee32a01671ccb3e662c`.
- **File-list comparison:** The exact PR head and synthetic merge ref produced an 11-file triple-dot scope. GitHub `/pulls/39219/files` was inaccessible, so a separate endpoint comparison was unavailable.
- **Not Reviewed Scope:** GitHub Actions, CI, and GitHub review discussions.
- **Verification:**
- Focused identifier-policy tests: exit 0; 23 tests passed.
- Firebird and single-core consumer compilation: exit 0.
- Scoped Checkstyle: exit 0; zero violations.
- Infra tests could not complete because the sandbox prevented Mockito’s inline mock maker from attaching; this failure was unrelated to the PR.
- Spotless could not execute the modules because the sandbox denied a Maven resolver-status write; no formatting finding is based on that run.
- **Release Note / User Docs:** Not required. This is an internal implementation simplification with no user-visible behavior or configuration change.
```
--
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