iamhucong commented on PR #38898:
URL: https://github.com/apache/shardingsphere/pull/38898#issuecomment-4778515093

   ### Summary
   
      **Review Result: Mergeable**
   
      Reason: The PR fixes the resource-leak trigger directly and keeps the 
existing table-existence semantics unchanged. `DatabaseMetaData#getTables(...)` 
returns a `ResultSet`, and the updated `containsTable(...)` now closes
    it with try-with-resources before returning the existing `next()` result.
   
      ### Evidence
   
      - 
`test/e2e/sql/src/test/java/org/apache/shardingsphere/test/e2e/sql/it/sql/ddl/DDLE2EIT.java:322-326`
 now closes the `ResultSet` returned by `DatabaseMetaData#getTables(...)`.
      - This aligns `containsTable(...)` with the existing metadata access 
patterns in the same class:
        - `getActualColumns(...)` closes the `getColumns(...)` result set.
        - `getActualIndexes(...)` closes the `getIndexInfo(...)` result set.
      - The change is limited to one E2E test class, does not change production 
behavior, and does not introduce configuration, API, dependency, or 
distribution impact.
      - I checked the PR checks at review time. There were no completed failing 
checks; several checks were still queued.
   
      ### Review Details
   
      - Reviewed Scope:
        - Latest PR head SHA: `6a5a00050e3710671988ef8c49ddc9395c4f0faa`
        - Base ref/SHA: `apache/master` at 
`7ca70b559c6e211ab043e57ca1d6f9cb487cf625`
        - Local merge-base SHA: `7ca70b559c6e211ab043e57ca1d6f9cb487cf625`
        - GitHub `/pulls/38898/files` matched the local triple-dot file list.
        - Reviewed file:
          - 
`test/e2e/sql/src/test/java/org/apache/shardingsphere/test/e2e/sql/it/sql/ddl/DDLE2EIT.java`
   
      - Not Reviewed Scope:
        - Full Oracle E2E execution was not run locally because this review 
only needs to verify the resource-management fix and local compile/style gates. 
CI was inspected for failures and had no completed failing checks at
    review time.
   
      - Verification:
        - `./mvnw -pl test/e2e/sql -am -DskipITs -Dspotless.skip=true 
-DskipTests test-compile` — exit code 0
        - `./mvnw spotless:apply -Pcheck -T1C` — exit code 0
        - `./mvnw checkstyle:check -Pcheck -T1C` — exit code 0
   
      - Release Note / User Docs:
        - Not required. This is an internal E2E test resource-management fix 
and does not change user-facing behavior, configuration, APIs, migration steps, 
or release artifacts.


-- 
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]

Reply via email to