strongduanmu commented on PR #38710:
URL: https://github.com/apache/shardingsphere/pull/38710#issuecomment-4507703957
Merge Verdict: Mergeable
Thanks for the update. I reviewed the latest PR head and the change is in
the right direction.
This PR replaces the Proxy-side JDBCQueryResultMetaData usage with
ShardingSphereResultSetMetaData when building query headers. With this change,
Proxy can reuse the same metadata decoration behavior as JDBC for table name
decoration,
derived projection handling, column name/label resolution, and column type
metadata. This makes Proxy behavior more consistent with JDBC, especially for
feature-modified metadata such as encryption columns.
Reviewed scope:
- infra/executor query result metadata abstraction and raw metadata
implementation
- JDBC ShardingSphereResultSetMetaData adjustment
- Proxy StandardDatabaseProxyConnector query header construction
- Proxy admin query handler metadata bridge
- QueryHeaderBuilderEngine and dialect QueryHeaderBuilder implementations
- MySQL prepared statement projection metadata resolver
- Related unit tests
- CI / CheckStyle / Spotless / E2E status
The main production paths are covered by the implementation:
- Normal Proxy query response headers now wrap the underlying
ResultSetMetaData with ShardingSphereResultSetMetaData.
- Federation query response headers follow the same metadata decoration
path.
- Admin/raw query results now provide a ResultSetMetaData bridge through
RawResultSetMetaData.
- MySQL COM_STMT_PREPARE projection metadata also uses the same
QueryHeaderBuilderEngine path.
I did not find unrelated changes in this PR. The changes are scoped to
metadata/header construction and the related tests. There are no parser
changes, configuration semantic changes, dependency manifest changes, or SQL
dialect syntax
changes.
Risk assessment:
- Design: acceptable for this PR. Proxy currently reuses JDBC metadata
behavior directly. A future refactor can move the reusable ResultSetMetaData
decoration logic into infra to make the dependency boundary cleaner.
- Compatibility: the change is behavior-compatible with the stated goal of
keeping Proxy metadata consistent with JDBC.
- Performance: no new remote calls, unbounded loops, blocking I/O, or
high-frequency ConcurrentHashMap#computeIfAbsent pattern were introduced on the
reviewed paths.
- Regression surface: affected dialect builders for MySQL, PostgreSQL,
openGauss, and Firebird were reviewed. The changes mainly replace the metadata
input type while preserving QueryHeader field construction semantics.
Validation:
- CI and required style checks are green.
- E2E has passed, including relevant Proxy/JDBC and encryption-related
scenarios.
- Added tests cover the new metadata bridge and adjusted header builder
behavior.
This PR is mergeable.
Follow-up suggestion:
Please consider a later refactor to extract the reusable
ShardingSphereResultSetMetaData decoration logic from jdbc into infra, so JDBC
and Proxy can share the same metadata decorator without Proxy depending on JDBC
driver internals.
--
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]
