terrymanu commented on PR #38826: URL: https://github.com/apache/shardingsphere/pull/38826#issuecomment-4638085864
### Summary - **Merge Decision: Mergeable** - **Reason:** The latest PR fixes the producer-side row/header alignment root cause and has targeted regression coverage plus scoped local verification. ### Issues - No merge-blocking issues found in the reviewed scope. ### Evidence - `proxy/backend/core/src/main/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/DatabaseMetaDataExecutor.java:141` now derives metadata columns and row data from the same ordered `columnLabels`, and `:150` pads missing cells while preserving existing values. - `proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/admin/executor/DatabaseMetaDataExecutorTest.java:168` covers both filtered rows and mismatched row keys, including the former short-row symptom via `MergedResult#getValue`. - `proxy/backend/dialect/mysql/src/main/java/org/apache/shardingsphere/proxy/backend/mysql/connector/sane/MySQLDialectSaneQueryResultEngine.java:72` now rejects unsupported no-FROM projection shapes instead of silently producing partial sane results. - `proxy/backend/dialect/mysql/src/test/java/org/apache/shardingsphere/proxy/backend/mysql/connector/sane/MySQLDialectSaneQueryResultEngineTest.java:69` covers both supported expression projections and the unsupported shorthand counterexample. - Shared-path scan found `DatabaseMetaDataExecutor` is also used by MySQL, PostgreSQL, and openGauss admin/system-table query factories; the change is a generic memory-result consistency fix and does not introduce target-dialect names, protocol state, config changes, or API/SPI changes. ### Review Details - **Reviewed Scope:** [apache/shardingsphere#38826](https://github.com/apache/shardingsphere/pull/38826), latest head `8491ade4988af7ac363cbd4099b2d4c57e2a80ce`, base/merge-base `8162bdf1377764c7cf4a52edd32c35f4d0a2796c`. Local `git diff --name-status 8162bdf1377764c7cf4a52edd32c35f4d0a2796c..refs/remotes/apache/pr/38826` matched GitHub `/pulls/38826/files`. - **Reviewed Files:** `DatabaseMetaDataExecutor.java`, `StandardDatabaseProxyConnectorTest.java`, `DatabaseMetaDataExecutorTest.java`, `MySQLDialectSaneQueryResultEngine.java`, `MySQLDialectSaneQueryResultEngineTest.java`, `MySQLSelectWithoutFromAdminExecutorFactoryTest.java`. - **Not Reviewed Scope:** GitHub Actions/check-run status, full repository integration tests, and live Proxy client smoke tests were not reviewed. - **Verification:** `./mvnw -pl proxy/backend/core,proxy/backend/dialect/mysql -am -DskipITs -Dspotless.skip=true -Dcheckstyle.skip=true -Dtest=DatabaseMetaDataExecutorTest,StandardDatabaseProxyConnectorTest,MySQLDialectSaneQueryResultEngineTest,MySQLSelectWithoutFromAdminExecutorFactoryTest -Dsurefire.failIfNoSpecifiedTests=false test` exited 0. - **Verification:** `./mvnw -pl proxy/backend/core,proxy/backend/dialect/mysql -am -DskipTests -DskipITs spotless:check -Pcheck -T1C` exited 0. - **Verification:** `./mvnw -pl proxy/backend/core,proxy/backend/dialect/mysql -am -DskipTests -DskipITs checkstyle:check -Pcheck -T1C` exited 0. - **Release Note / User Docs:** Not required. The PR is scoped to internal result assembly/fallback correctness, does not add supported SQL syntax, config, API/SPI, migration behavior, or user action requirements. -- 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]
