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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
