yx9o commented on PR #38918: URL: https://github.com/apache/shardingsphere/pull/38918#issuecomment-4805623922
### Summary - **Review Result: Mergeable** - Reason: The PR directly fixes the Firebird Proxy affected-rows propagation path for `isc_info_sql_records`, uses the correct Firebird record-count item codes, and adds focused tests for packet encoding and DML affected-row propagation. ### Evidence - `FirebirdExecuteStatementCommandExecutor` now stores `UpdateResponseHeader#getUpdateCount()` on the Firebird prepared statement after update execution, so later statement-info requests can read the real affected-row count. - `FirebirdSQLInfoExecutor` maps the saved count into insert/update/delete slots based on the prepared statement SQL type, while unknown or missing handles still return zero counts. - `FirebirdSQLInfoReturnPacket` encodes `isc_info_sql_records` with Firebird request-count item codes `13`, `14`, `15`, and `16`, rather than statement-type codes. - Tests cover the records packet layout, insert/update/delete slot mapping, and execute-path affected-row persistence. - CI on latest head `f18bd029091f714849c570f36f9065d1a86bcc4a` reports CheckStyle, Spotless, License, and CI success. ### Review Details - Reviewed Scope: latest PR head `f18bd029091f714849c570f36f9065d1a86bcc4a`; GitHub `/pulls/38918/files` scope matched the reviewed diff; reviewed Firebird protocol SQL info packet changes, Firebird Proxy SQL info executor, Firebird prepared statement state, Firebird execute statement executor, and related unit tests. - Not Reviewed Scope: live Firebird client/server integration beyond CI; full repository line-by-line review outside the 8 PR files. - Verification: Reviewed public PR metadata, changed-file list, diff, comments/reviews, commits, and check-runs. No local Maven test was run because the current PR source was reviewed from the public diff and CI evidence rather than a local checkout of the PR head. - Release Note / User Docs: Not required; this is a Firebird Proxy protocol correctness fix with no documented config, SQL syntax, migration, or user workflow change. -- 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]
