jiangML commented on PR #39092:
URL: https://github.com/apache/shardingsphere/pull/39092#issuecomment-4966077964
### Summary
**Review Result: Mergeable**
**Reason:** The latest commit adds direct coverage for connection-attribute
extraction and propagation through the PostgreSQL and openGauss authentication
paths. No code-level blocker was found. This result applies to the reviewed
code scope only; CI not reviewed by request.
### Evidence
- `PostgreSQLComStartupPacket#getConnectionAttributes()` excludes `user`,
`database`, and `client_encoding` while preserving other startup parameters as
connection attributes.
- Both authentication engines store these attributes in the continued
`AuthenticationResult` and preserve them when producing the finished result.
- The shared frontend handler copies the finished authentication result into
`ConnectionSession`, completing this path:
`StartupPacket → AuthenticationResult.continued →
AuthenticationResult.finished → ConnectionSession`.
- `PostgreSQLComStartupPacketTest` verifies filtering of the core parameters
and retention of `application_name`.
- `PostgreSQLAuthenticationEngineTest` and
`OpenGaussAuthenticationEngineTest` verify that the connection attributes enter
the corresponding authentication results.
- No shared-layer dialect leakage, implicit mode switch, compatibility
regression, configuration migration, dependency, packaging, or distribution
concern was found.
### Review Details
- **Review Focus:** Code Correctness Review. CI not reviewed by request.
- **Reviewed Scope:** All six files reported by GitHub `/pulls/39092/files`
at head `1ce89555d8abc5c05d65c524680339946b0d6731`, including PostgreSQL
startup-packet parsing, the PostgreSQL/openGauss authentication lifecycle, the
three corresponding test classes, and the downstream shared
`FrontendChannelInboundHandler → ConnectionSession` path. The local merge-base
was `0b523bd47978445cd759aab596f8349d59646a06`. The local triple-dot file list
matched GitHub’s authoritative file list.
- **Not Reviewed Scope:** GitHub Actions, check-runs, workflow logs, and
native-client or E2E scenarios.
- **Verification:** Ran the three focused test classes against the latest PR
head. Maven exited with code 0 and `BUILD SUCCESS`; 36 tests ran with 0
failures, 0 errors, and 0 skipped.
- **Release Note / User Docs:** Not required. The PR adds internal
connection-attribute propagation without introducing configuration, migration,
compatibility-operation, or user-workflow changes.
--
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]
