strongduanmu commented on PR #39276:
URL: https://github.com/apache/shardingsphere/pull/39276#issuecomment-5128430848
• ### Result
**Review Result: Mergeable**
Code-scope review found no blocking correctness issue in PR #39276. The
change models PostgreSQL-family whole-row projection as a dialect SQL
capability, keeps default behavior disabled for other dialects, and applies the
binder
fallback only after ordinary column binding fails.
### Evidence
- PR head reviewed: `7e9913ac37b50aad53018f7a983164582826b3e1`
- GitHub authoritative changed-file list: 12 files; local
`upstream/master...HEAD` inventory matched GitHub paths 12/12.
- `DialectDatabaseMetaData#getSQLOption()` defaults to `DefaultSQLOption`,
so existing dialects keep whole-row projection disabled by default:
`database/connector/core/src/main/java/org/apache/shardingsphere/database/connector/core/
metadata/database/metadata/DialectDatabaseMetaData.java:196`.
- PostgreSQL opts in through `PostgreSQLSQLOption`:
`database/connector/dialect/postgresql/src/main/java/org/apache/shardingsphere/database/connector/postgresql/metadata/database/option/PostgreSQLSQLOption.java:25`.
- openGauss explicitly reuses the PostgreSQL SQL option in its own
metadata:
`database/connector/dialect/opengauss/src/main/java/org/apache/shardingsphere/database/connector/opengauss/metadata/database/
OpenGaussDatabaseMetaData.java:101`.
- Binder fallback is scoped to `ColumnNotFoundException`; ordinary
real-column binding remains first:
`infra/binder/core/src/main/java/org/apache/shardingsphere/infra/binder/engine/segment/dml/projection/
ProjectionsSegmentBinder.java:91`.
- Whole-row detection is restricted to unqualified, non-nested column
projection text that matches a table alias/name and a dialect option that
supports it:
`infra/binder/core/src/main/java/org/apache/shardingsphere/infra/binder/engine/
segment/dml/projection/ProjectionsSegmentBinder.java:156`.
- Tests cover the new default option, PostgreSQL/openGauss opt-in, and
binder conversion to `ExpressionProjectionSegment`:
`infra/binder/core/src/test/java/org/apache/shardingsphere/infra/binder/engine/statement/dml/
SelectStatementBinderTest.java:131`.
### Coverage
Reviewed files accounted for:
`RELEASE-NOTES.md`; core metadata option SPI and tests; PostgreSQL
metadata option and tests; openGauss metadata reuse and tests;
`ProjectionsSegmentBinder`; `SelectStatementBinderTest`.
Behavior clusters covered:
release note, core SQL option SPI compatibility, PostgreSQL/openGauss
dialect opt-in, binder whole-row fallback behavior, focused regression tests.
Discovery lenses completed:
root cause and behavior, blast radius and contracts,
tests/runtime/operations, convergence pass.
Unresolved gaps:
none for code correctness.
CI scope:
Code Correctness Review only. CI, Actions, and E2E were not queried or
rerun per request.
--
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]
