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/07 13:54:12 UTC
[PR] Remove the useless props parameter for TableRefreshUtils [shardingsphere]
menghaoranss opened a new pull request, #39038:
URL: https://github.com/apache/shardingsphere/pull/39038
Changes proposed in this pull request:
- Remove the useless props parameter for TableRefreshUtils
---
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] Remove the useless props parameter for TableRefreshUtils [shardingsphere]
Posted by "menghaoranss (via GitHub)" <gi...@apache.org>.
menghaoranss merged PR #39038:
URL: https://github.com/apache/shardingsphere/pull/39038
--
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] Remove the useless props parameter for TableRefreshUtils [shardingsphere]
Posted by "menghaoranss (via GitHub)" <gi...@apache.org>.
menghaoranss commented on PR #39038:
URL: https://github.com/apache/shardingsphere/pull/39038#issuecomment-4904873665
```markdown
### Summary
**Review Result: Mergeable**
Reason: Code-scope only. CI not reviewed by request. The PR removes an unused `ConfigurationProperties` parameter from `TableRefreshUtils` and updates the affected metadata refresh callers consistently, while keeping configuration propagation in the loaders that still need it.
### Evidence
- `mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/refresher/util/TableRefreshUtils.java:76`, `:87`, `:99`, `:113`, `:133`, `:153`, `:168`, `:183`, and `:204` expose the simplified signatures. The implementation still uses the same `IdentifierCasePolicy`, schema lookup, and normalized fallback behavior; only the unused parameter was removed.
- `mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/refresher/pushdown/type/table/TableMetaDataRefresherLoader.java:80-87` and `mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/refresher/pushdown/type/view/ViewMetaDataRefresherLoader.java:57-64`, `:92-99` still pass `props` into `GenericSchemaBuilderMaterial`, so the configuration-dependent metadata loading path is preserved.
- The changed index/table/view refreshers now call the simplified utility methods, and no old `TableRefreshUtils` call with `props` remains in the reviewed PR scope.
- `mode/core/src/test/java/org/apache/shardingsphere/mode/metadata/refresher/util/TableRefreshUtilsIdentifierTest.java:47-91` covers actual table/view/index/column resolution and table/view load candidate normalization after the signature change.
### Review Details
- Review Focus: Code Correctness Review. CI not reviewed by request.
- Reviewed Scope: 14 files in `mode/core` from GitHub `/pulls/39038/files`, covering `TableRefreshUtils`, affected pushdown metadata refreshers/loaders for index/table/view, and related utility tests. Latest PR head SHA: `78d7998eb298b30e22d10a533054ec2785e990a1`. Base ref: `apache:master`, base SHA from PR metadata: `91a4e27823054d1050db1856c57e0dbdf5912736`. Local merge-base SHA: `ea9c85c26a3b6773b574f6f5e8ac8a44dceae0ce`. Local triple-dot file list matched GitHub `/pulls/39038/files`.
- Not Reviewed Scope: GitHub Actions/check-runs/logs, repository merge gate status, and modules outside the PR file list.
- Verification:
- `git fetch upstream pull/39038/head:refs/remotes/upstream/pr/39038`: exit 0.
- `git diff --name-only upstream/master...upstream/pr/39038`: exit 0; matched the 14 GitHub changed files.
- Bounded `git grep` over reviewed metadata refresher paths confirmed `ConfigurationProperties` remains only on refresher/loader contracts and schema-building paths, not on the removed `TableRefreshUtils` signatures.
- `git diff --check upstream/master...upstream/pr/39038`: exit 0.
- `./mvnw -pl mode/core -Dtest=TableRefreshUtilsIdentifierTest test -Pcheck -T1C`: exit 0, `BUILD SUCCESS`.
- A broader focused Maven run for the changed utility and affected refresher tests exited 1 because the local Oracle JDK 21 environment could not initialize Mockito inline Byte Buddy self-attach; the surefire report shows `Could not initialize inline Byte Buddy mock maker` before assertion execution. Several non-mocking affected refresher tests completed successfully in that run, and this local tooling failure was not used as PR-blocking evidence.
- Release Note / User Docs: Not required. This is an internal metadata refresh utility signature cleanup with no user-facing configuration, SQL behavior, diagnostics, migration, or release-awareness impact.
```
--
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