yx9o commented on PR #39775: URL: https://github.com/apache/shardingsphere/pull/39775#issuecomment-5653557218
### Result **Review Result: Mergeable** PR #39775 is mergeable for code correctness. The current head fixes the Woodstox `XMLInputFactory` compatibility path without weakening the existing XML external-resource protections, updates native reachability metadata to the dependency versions used by the head, and migrates the native-test workflows to a supported GraalVM Community setup. ### Evidence - Effective candidate: public PR head `ff1e05649e2292ef9a6386e5cbc337b5b30d8221` against base `5cec93484b445d4fc64b02b15de874d83a2b6c09`, from [PR #39775](https://github.com/apache/shardingsphere/pull/39775). - [`JDBCRepositorySQLLoader.java:78-85`](https://github.com/apache/shardingsphere/blob/ff1e05649e2292ef9a6386e5cbc337b5b30d8221/mode/type/standalone/repository/provider/jdbc/src/main/java/org/apache/shardingsphere/mode/repository/standalone/jdbc/sql/JDBCRepositorySQLLoader.java#L78-L85) keeps `SUPPORT_DTD=false`, `IS_SUPPORTING_EXTERNAL_ENTITIES=false`, and the rejecting resolver, while only setting `ACCESS_EXTERNAL_DTD` when the selected StAX provider supports it. - [`setup-build-environment/action.yml:84-95`](https://github.com/apache/shardingsphere/blob/ff1e05649e2292ef9a6386e5cbc337b5b30d8221/.github/workflows/resources/actions/setup-build-environment/action.yml#L84-L95) routes `graalvm-community` to `graalvm/setup-graalvm@0426e2e...`; that pinned action lists `graalvm-community` as a supported distribution in its README. - [`graalvm.yml:62-65`](https://github.com/apache/shardingsphere/blob/ff1e05649e2292ef9a6386e5cbc337b5b30d8221/.github/workflows/graalvm.yml#L62-L65) and [`nightly-ci.yml:86-89`](https://github.com/apache/shardingsphere/blob/ff1e05649e2292ef9a6386e5cbc337b5b30d8221/.github/workflows/nightly-ci.yml#L86-L89) now call the shared action with `distribution: 'graalvm-community'`; a GitHub Contents API scan of all 18 PR-head workflow files found no remaining exact `distribution: graalvm` caller. - [`test/native/pom.xml:112-117`](https://github.com/apache/shardingsphere/blob/ff1e05649e2292ef9a6386e5cbc337b5b30d8221/test/native/pom.xml#L112-L117) and the HiveServer2 docs stop excluding `woodstox-core`, matching the PR's native-test failure path. - The PR head root POM uses `calcite.version=1.42.0`, `mariadb-java-client.version=2.7.14`, and `hive-server2-jdbc-driver-thin.version=1.8.2`, matching the moved/added reachability metadata version paths. The changed generated, Calcite, and MariaDB reachability metadata JSON files all parsed successfully. ### Coverage - Review basis: GitHub master `review-pr` skill, Formal Review Mode, Code Correctness Review. GitHub Actions/check runs were not queried or reviewed; local style verification below is not CI. - Authoritative changed files accounted for: 11 files from the PR Files API, covering GraalVM workflow setup, the shared setup action, HiveServer2 native-test dependency/docs, Calcite/MariaDB/generated reachability metadata, and JDBC repository XML loading. - Mandatory style verification: `JDBCRepositorySQLLoader.java` covered by `spotless:check checkstyle:check` with exact-file Spotless and Checkstyle includes; exit code 0, Spotless selected 1 Java file clean, Checkstyle reported 0 violations. - Mandatory style verification: `test/native/pom.xml` covered by `spotless:check` with exact-file Spotless include; exit code 0, Spotless.Pom selected 1 file clean. - Style not applicable: YAML, Markdown, and JSON PR-impact files are not governed by the repository Maven Checkstyle/Spotless gates found for this scope; JSON files were separately parsed for validity. - Completed discovery lenses: root cause and behavior; blast radius and shared contracts; tests, runtime, operations, dependencies, and generated artifacts. - Unresolved code-correctness gaps: none found. -- 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]
