menghaoranss commented on PR #39053: URL: https://github.com/apache/shardingsphere/pull/39053#issuecomment-4912794791
```markdown ### Summary **Review Result:** Mergeable **Reason:** In Code Correctness Review scope, the latest PR head now consistently applies the view identifier policy to federation create-view, alter-view, and rename-view refresh paths. CI not reviewed by request, so this result is code-scope only. ### Evidence - `CreateViewFederationMetaDataRefresher` now resolves the created view name with `TableRefreshUtils.getViewLoadCandidateName(...)` before persisting the `ShardingSphereView`, so federation create-view refresh uses the database view identifier policy rather than the old database-type table identifier formatting. - `AlterViewFederationMetaDataRefresher` now resolves both the existing view name and the rename target with `TableRefreshUtils.getViewLoadCandidateName(...)`. This fixes the previous asymmetry where the old view name used the view policy but the rename target could still be persisted from the raw identifier value. - `TableRefreshUtils.getViewLoadCandidateName(...)` delegates to the existing identifier-policy based load-candidate logic, preserving quoted identifiers while normalizing unquoted identifiers according to the view lookup policy. - The removed `TableRefreshUtils.getTableName(...)` helper has no remaining callers in the reviewed `mode/core` source scope, and `mode/core` compiles at the latest PR head. ### Review Details - **Review Focus:** Code Correctness Review. CI not reviewed by request. - **Reviewed Scope:** PR #39053 latest head `f646426c803719e83160ddfd1659e5fa23365ebd`; base `upstream/master` at `2e4bc504ed792cee62ae56fbb511c11c5bbbf470`; local merge-base `2e4bc504ed792cee62ae56fbb511c11c5bbbf470`. Reviewed all six GitHub-listed files under `mode/core`, and the local triple-dot file list matched GitHub `/pulls/39053/files`. - **Not Reviewed Scope:** GitHub Actions/check-runs, full Maven install, and end-to-end database/proxy behavior. - **Verification:** `./mvnw -pl mode/core -DskipTests compile -Pcheck -T1C` exited 0. `./mvnw -pl mode/core -Dtest=TableRefreshUtilsIdentifierTest test -Pcheck -T1C` exited 0. A broader focused test command including Mockito-based federation refresher tests exited 1 because the local Mockito inline mock maker could not initialize the Byte Buddy self-attach agent before executing those tests; this local tooling failure is not treated as PR evidence. - **Release Note / User Docs:** Not required for this internal metadata refresh correctness change; no user-facing configuration, syntax, or migration behavior is introduced. ``` -- 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]
