menghaoranss commented on PR #39033:
URL: https://github.com/apache/shardingsphere/pull/39033#issuecomment-4902969930
```markdown
### Summary
**Review Result: Mergeable**
Reason: Code-scope only. CI not reviewed by request. The PR removes an
unused `ConfigurationProperties` parameter from `SchemaRefreshUtils` and
updates all PR-head callers consistently, without changing the schema matching
logic or metadata refresh behavior.
### Evidence
-
`mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/refresher/util/SchemaRefreshUtils.java:60`
and `:71` keep the same actual-schema resolution path: raw schema extraction
delegates to identifier-based matching, and identifier matching still uses
`IdentifierCasePolicy` plus the same fallback normalization.
-
`mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/refresher/pushdown/PushDownMetaDataRefreshEngine.java:91`
still resolves the schema name before passing the original `props` through to
the selected `PushDownMetaDataRefresher` at `:94-95`, so downstream
table/view/index refresh logic still receives configuration properties where
they are actually used.
-
`mode/core/src/main/java/org/apache/shardingsphere/mode/metadata/refresher/util/TableRefreshUtils.java:217`,
`:302`, and `:320` now call the simplified schema utility, while object-level
table/view/index matching still keeps `ConfigurationProperties` in its own
public methods and downstream calls.
-
`mode/core/src/test/java/org/apache/shardingsphere/mode/metadata/refresher/util/SchemaRefreshUtilsTest.java:61-69`
covers case-insensitive schema matching and multi-schema resolution after the
signature change. The affected schema/table/view/index refreshers also have
focused tests for actual-name resolution.
### Review Details
- Review Focus: Code Correctness Review. CI not reviewed by request.
- Reviewed Scope: 13 files in `mode/core` from GitHub `/pulls/39033/files`,
covering `SchemaRefreshUtils`, affected pushdown refreshers for
schema/table/view/index metadata, `TableRefreshUtils`, and related tests.
Latest PR head SHA: `0ec89ef7810ffe8f74099986622241f855a2e0f7`. Base ref:
`apache:master`, base SHA from PR metadata:
`32aa9fae360b1d1659a787d44b0082f49319057e`. Local merge-base SHA:
`c766f879a08513d51f1d87b160d09b015c8ecf64`. Local triple-dot file list matched
GitHub `/pulls/39033/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/39033/head:refs/remotes/upstream/pr/39033`:
exit 0.
- `git diff --name-only upstream/master...upstream/pr/39033`: exit 0;
matched the 13 GitHub changed files.
- `git grep` on `upstream/pr/39033` for old `SchemaRefreshUtils`
signatures with `ConfigurationProperties`: exit 1, no remaining references
found.
- `./mvnw -pl mode/core
-Dtest=SchemaRefreshUtilsTest,AlterIndexPushDownMetaDataRefresherTest,DropIndexPushDownMetaDataRefresherTest,AlterSchemaPushDownMetaDataRefresherTest,CreateSchemaPushDownMetaDataRefresherTest,DropSchemaPushDownMetaDataRefresherTest,DropTablePushDownMetaDataRefresherTest,RenameTablePushDownMetaDataRefresherTest,DropViewPushDownMetaDataRefresherTest
test -Pcheck -T1C`: exit 0, `BUILD SUCCESS`.
- A broader focused run including `PushDownMetaDataRefreshEngineTest`
exited 1 because Mockito inline static mocking could not initialize Byte Buddy
self-attach on the local Oracle JDK 21 VM; this is an environment/tooling
failure before test body execution, not evidence of a PR behavior regression.
- Release Note / User Docs: Not required. This is an internal refactor of an
unused utility parameter with no user-facing configuration, SQL behavior,
migration, diagnostics, 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]
