You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "somiljain2006 (via GitHub)" <gi...@apache.org> on 2026/07/23 13:36:44 UTC
[PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
somiljain2006 opened a new pull request, #39225:
URL: https://github.com/apache/shardingsphere/pull/39225
Fixes #36978
Add a schema metadata fallback for PostgreSQL prepared statement parameter type resolution, populate unresolved PGobject types, support custom PostgreSQL types when JDBC metadata is unavailable, and extend parameter-to-column resolution for INSERT and UPDATE statements.
---
Before committing this PR, I'm sure that I have checked the following options:
- [ ] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
- [ ] I have self-reviewed the commit code.
- [ ] I have (or in the comment I request) added corresponding labels for the pull request.
- [ ] I have passed Maven check locally: `./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e`.
- [ ] I have made corresponding changes to the documentation.
- [ ] I have added corresponding unit tests for my changes.
- [ ] I have updated the Release Notes of the current development version. For more details, see [Update Release Note](https://shardingsphere.apache.org/community/en/involved/contribute/contributor/)
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
Posted by "somiljain2006 (via GitHub)" <gi...@apache.org>.
somiljain2006 commented on PR #39225:
URL: https://github.com/apache/shardingsphere/pull/39225#issuecomment-5100936446
@terrymanu Thanks for the detailed review.
I agree that we should first establish who should own PostgreSQL custom type resolution before continuing the implementation.
My understanding of the current discussion is:
* The root problem is that PostgreSQL custom types currently fall through the `Types.OTHER` path and can be classified as JSON.
* The resolver I modified is likely too late in the Parse/Describe/Bind flow to be the authoritative owner of PostgreSQL type identity.
* If `typeName` becomes part of the shared metadata model, it must also participate in persistence/reload rather than only exist in in-memory objects.
Rather than continuing with the current resolver-based approach, I'd like to revisit the design first.
My proposed plan is:
1. Trace the complete extended protocol flow (Parse → Describe → Bind) to identify where `Types.OTHER` is first mapped to `PostgreSQLBinaryColumnType`.
2. Move the custom-type resolution to that authoritative point instead of repairing it after decoding, and reuse existing Binder/Describe statement metadata where available rather than introducing a separate statement-to-column mapping in the resolver.
3. Only if metadata is still required there, make `typeName` a complete metadata contract (including persistence); otherwise keep PostgreSQL-specific type information local to the protocol layer.
4. Keep the existing fast path for already-resolved parameters so ordinary text binds don't incur additional metadata lookups.
Does that direction align better with the intended ownership, or is there another component in the PostgreSQL protocol pipeline where you'd prefer this logic to live?
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #39225:
URL: https://github.com/apache/shardingsphere/pull/39225#issuecomment-5266125530
@somiljain2006 Thanks. The proposed direction is aligned with the requested boundaries. Please proceed with the implementation.
Please keep the following contracts explicit:
1. `PostgreSQLServerPreparedStatement` should own the per-parameter state required across Describe, Bind, repeated Bind, and batch execution. A parameter should be considered resolved only when the state contains enough information to emit the correct `ParameterDescription` type OID and decode or bind its value correctly. A native type name is required where the type cannot be represented safely by the existing fixed protocol types.
2. Bind must resolve the parameter type before type-dependent decoding. JSON and JSONB must remain distinct, and an arbitrary `Types.OTHER` value must not be silently decoded as JSON or mapped to `VARCHAR`.
3. Semantic resolution must use each marker’s complete `ColumnSegmentBoundInfo`, including its database, schema, table, and column ownership. Missing or ambiguous ownership should remain unresolved rather than falling back to a statement-wide schema or a same-named object.
4. The metadata-only Describe path may be used only when all parameter and result types required by the response have been resolved unambiguously. Otherwise, use the existing JDBC metadata fallback. In particular, RETURNING projections must not be described from the statement’s first table when their actual ownership is different or unknown.
5. Preserve the existing PostgreSQL and openGauss composite-column OID behavior established by #39241 and #39253. This PR does not need a broader OID architecture change, but it must not replace actual composite result OIDs with the `VARCHAR` OID or remove the existing supported behavior.
6. Tests should exercise the real parser/Binder/Describe/Bind path and cover Describe-first, Bind-first, repeated Bind, batch execution, ordinary `VARCHAR`, JSON, JSONB, arbitrary custom types, schema-qualified and cross-schema statements, RETURNING expressions, composite result OIDs, and the already-resolved text fast path without additional connection or metadata access.
The exact shape and name of the parameter-state class can remain an implementation detail as long as these behavioral contracts are satisfied.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #39225:
URL: https://github.com/apache/shardingsphere/pull/39225#issuecomment-5261816985
### Result
**Review Result: Not Mergeable**
Feedback Mode: Needs Discussion
**Blocking Issues: 4**
The general ownership direction is reasonable: PostgreSQL extended-protocol state should own parameter-type resolution, schema metadata may supply native type names, and Binder should remain the semantic owner. However, the current implementation cannot represent arbitrary PostgreSQL types and treats incomplete metadata inference as complete. The replacement state model and fallback contract must be agreed before further implementation.
### Decisions Required Before Further Implementation
The PR needs agreement on:
1. How each prepared-statement parameter represents standard, JSON, JSONB, and dynamic custom types, including native type name, wire OID, decoding strategy, and unresolved state.
2. When raw values in Bind-first flows are converted, and how resolved state is reused by repeated Bind and batch execution.
3. What constitutes complete Binder/schema-metadata resolution, and when Describe must fall back to JDBC metadata.
4. How RETURNING projections and PostgreSQL/openGauss composite fields obtain their actual wire-level OIDs.
### Blocking Issues
#### 1. The parameter state model cannot preserve native custom type identity
**Evidence**
- [`PostgreSQLServerPreparedStatement`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/proxy/frontend/dialect/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/extended/PostgreSQLServerPreparedStatement.java#L49-L62) stores only `List<PostgreSQLBinaryColumnType>`.
- [`PostgreSQLBinaryColumnType.valueOfJDBCType`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/database/protocol/dialect/postgresql/src/main/java/org/apache/shardingsphere/database/protocol/postgresql/packet/command/query/extended/PostgreSQLBinaryColumnType.java#L225-L244) maps every unrecognized `Types.OTHER` value to `VARCHAR`.
- [`PostgreSQLPreparedStatementParameterTypeResolverTest`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/proxy/frontend/dialect/postgresql/src/test/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/extended/PostgreSQLPreparedStatementParameterTypeResolverTest.java#L300-L339) supplies `my_custom_type` and `my_enum` as native type names but expects both to become `VARCHAR`.
- `JSONB` uses [`PostgreSQLJsonValueParser`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/database/protocol/dialect/postgresql/src/main/java/org/apache/shardingsphere/database/protocol/postgresql/packet/command/query/extended/bind/protocol/text/impl/PostgreSQLJsonValueParser.java#L29-L36), which always creates a `PGobject` whose type is `json`.
- PostgreSQL requires `ParameterDescription` to contain each parameter’s data-type object ID. [PostgreSQL protocol specification](https://www.postgresql.org/docs/current/protocol-message-formats.html#PROTOCOL-MESSAGE-FORMATS-PARAMETERDESCRIPTION)
**Impact**
Enums, domains, and other native types reported as `Types.OTHER` are described and subsequently parsed as `VARCHAR`. JSONB can be bound as JSON. Describe-first, repeated Bind, and batch execution therefore cannot preserve the actual native type identity required by the PR.
**Discussion Needed**
Agree on a per-parameter state contract capable of representing both fixed protocol types and database-specific native types. The decision must also define raw Bind-first handling, wire-OID resolution, and reuse across repeated and batched execution. Extending the current enum with more special cases cannot represent arbitrary custom types.
#### 2. Per-marker schema ownership is ignored
**Evidence**
- [`ColumnSegmentBoundInfo`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/parser/sql/statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/generic/bound/ColumnSegmentBoundInfo.java#L32-L69) records the original database, schema, table, and column for each bound column.
- [`tryResolveFromBoundAST`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/proxy/frontend/dialect/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/extended/PostgreSQLPreparedStatementParameterTypeResolver.java#L120-L164) selects one statement-wide schema from `TablesContext` and uses only each marker’s table and column names.
- If the selected schema contains a same-named table and column, the resolver reports complete success and skips JDBC fallback even when the marker belongs to another schema.
**Impact**
Schema-qualified and cross-schema statements can resolve parameters against the wrong table. This becomes externally visible when same-named columns have different native types or OIDs.
**Discussion Needed**
Define whether marker resolution must always use the complete bound owner or whether some statement-level fallback is permitted. The completeness contract must prevent a lookup in a different same-named schema from being treated as a successful resolution.
#### 3. The DML Describe path treats partial inference as complete
**Evidence**
- [`tryDescribePreparedStatement`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/proxy/frontend/dialect/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/extended/describe/PostgreSQLComDescribeExecutor.java#L100-L111) ignores the result of `tryResolveFromBoundAST` and unconditionally bypasses JDBC metadata for every `INSERT`, `UPDATE`, and `DELETE`.
- Binder propagation currently covers only selected direct expression forms. For example, [`AssignmentSegmentBinder`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/infra/binder/core/src/main/java/org/apache/shardingsphere/infra/binder/engine/segment/dml/assign/AssignmentSegmentBinder.java#L80-L86) annotates the value only when it is directly a `ParameterMarkerExpressionSegment`.
- [`ExpressionSegmentBinder`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/infra/binder/core/src/main/java/org/apache/shardingsphere/infra/binder/engine/segment/dml/expression/ExpressionSegmentBinder.java#L82-L149) does not bind `TypeCastExpression`; unsupported expressions are returned unchanged.
- [`describeReturning`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/proxy/frontend/dialect/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/extended/describe/PostgreSQLComDescribeExecutor.java#L147-L170) selects the first table and uses it for every RETURNING projection.
- PostgreSQL permits RETURNING expressions to reference tables from `UPDATE ... FROM` and `DELETE ... USING`. [UPDATE documentation](https://www.postgresql.org/docs/current/sql-update.html), [DELETE documentation](https://www.postgresql.org/docs/current/sql-delete.html)
**Impact**
Nested parameters can remain `UNSPECIFIED` while Describe still returns without JDBC fallback. Multi-table RETURNING projections can receive the type of an unrelated same-named column or fall back to `VARCHAR`. Clients may consequently receive incorrect ParameterDescription and RowDescription packets.
**Discussion Needed**
Agree on the completeness boundary for metadata-only DML Describe. In particular, determine whether unsupported parameter or projection shapes must trigger whole-statement JDBC fallback, and establish which existing semantic owner supplies each RETURNING projection’s table and type. Continuing to add isolated AST cases would leave the same completeness problem.
#### 4. PostgreSQL/openGauss composite type OID resolution is removed
**Evidence**
- [`populateColumnTypes`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/proxy/frontend/dialect/postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/extended/describe/PostgreSQLComDescribeExecutor.java#L211-L232) no longer loads actual column OIDs from the database.
- [`PostgreSQLColumnDescription`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/database/protocol/dialect/postgresql/src/main/java/org/apache/shardingsphere/database/protocol/postgresql/packet/command/query/PostgreSQLColumnDescription.java#L49-L58) converts the JDBC type through `PostgreSQLBinaryColumnType`; `Types.STRUCT` consequently becomes `VARCHAR`.
- The PR removes the PostgreSQL and openGauss dialect-specific `ColumnTypeOIDResolver` wiring and `ColumnTypeOIDLoader` usage established by [#39241](https://github.com/apache/shardingsphere/pull/39241) and [#39253](https://github.com/apache/shardingsphere/pull/39253).
- PostgreSQL RowDescription requires the field’s actual data-type object ID. [PostgreSQL protocol specification](https://www.postgresql.org/docs/current/protocol-message-formats.html#PROTOCOL-MESSAGE-FORMATS-ROWDESCRIPTION)
**Impact**
Composite fields can be reported as `VARCHAR` OID 1043 instead of their actual database OID, causing clients to select an incorrect result decoder.
**Discussion Needed**
Confirm whether removing dynamic composite OID resolution is an intentional compatibility change. If no compatibility break is intended, the agreed Describe design must preserve equivalent PostgreSQL and openGauss behavior in both JDBC-backed and metadata-only result-description paths.
### Test Boundary
Several resolver and Describe tests manually inject `ColumnSegmentBoundInfo`, including [`PostgreSQLPreparedStatementParameterTypeResolverTest`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/proxy/frontend/dialect/postgresql/src/test/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/extended/PostgreSQLPreparedStatementParameterTypeResolverTest.java#L426-L435) and [`PostgreSQLComDescribeExecutorTest`](https://github.com/apache/shardingsphere/blob/1e6473ac5e8afce4619ead50d164273b8061f775/proxy/frontend/dialect/postgresql/src/test/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/extended/describe/PostgreSQLComDescribeExecutorTest.java#L601-L607). These tests bypass the changed Binder production path and therefore cannot establish end-to-end semantic resolution.
The agreed design should define expected behavior for:
- Describe-first, Bind-first, repeated Bind, and batch execution;
- ordinary `VARCHAR`, JSON, JSONB, and arbitrary custom `Types.OTHER`;
- schema-qualified and cross-schema multi-table statements;
- nested expressions, `UPDATE ... FROM`, `DELETE ... USING`, and RETURNING;
- PostgreSQL and openGauss composite type RowDescription OIDs;
- the already-resolved text fast path without additional routing, connection acquisition, or JDBC metadata access.
### Coverage
- Reviewed head: `1e6473ac5e8afce4619ead50d164273b8061f775`
- Base: `master` at `ec31978cbc7eeccd59c86c57a7f073595f7d0749`
- Scope: all 24 files in GitHub’s authoritative changed-file list across 11 commits.
- Requirements: [issue #36978](https://github.com/apache/shardingsphere/issues/36978), the PR description, and the public [extended-protocol ownership boundary](https://github.com/apache/shardingsphere/pull/39225#issuecomment-5242574708).
- Behavior clusters: shared native column metadata and YAML persistence; PostgreSQL protocol type mapping and parameter state; Binder semantic propagation; PostgreSQL/openGauss Describe, Bind, fallback, and tests.
- Review lenses completed: root cause and behavior; blast radius and contracts; tests, runtime, and operations.
- No unresolved evidence gap remains that could change the blocker set.
- This is a Code Correctness Review only. GitHub Actions and CI were not reviewed.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #39225:
URL: https://github.com/apache/shardingsphere/pull/39225#issuecomment-5324510370
Please resolve conflict first
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
Posted by "somiljain2006 (via GitHub)" <gi...@apache.org>.
somiljain2006 commented on PR #39225:
URL: https://github.com/apache/shardingsphere/pull/39225#issuecomment-5155444273
Thanks for the follow-up review. I understand that the remaining blockers are architectural rather than implementation-specific.
From the repeated feedback, my understanding is that the implementation should not move forward until two ownership decisions are agreed:
1. Which component should be the authoritative owner of PostgreSQL custom type identity across the extended protocol flow (Parse, Describe, Bind, repeated execution, and batch execution).
2. Whether `typeName` is intended to become part of the persisted shared metadata contract, or whether PostgreSQL custom type information should remain protocol-local.
Rather than continuing to iterate on the current resolver-based approach, I'd like to align on those design decisions first.
Could you clarify what the intended ownership is for these two areas? Once that direction is clear, I'll rework the implementation around that architecture instead of continuing to evolve the current approach.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
Posted by "somiljain2006 (via GitHub)" <gi...@apache.org>.
somiljain2006 commented on PR #39225:
URL: https://github.com/apache/shardingsphere/pull/39225#issuecomment-5301004173
@terrymanu Thanks for the confirmation. I’ve proceeded with the implementation and addressed all of the listed contracts, including persistent parameter state across Describe/Bind/batch flows, type resolution before decoding, distinct JSON/JSONB handling, safe handling of custom Types.OTHER types, strict semantic column ownership, and the existing composite OID behavior for PostgreSQL and openGauss.
I’ve also added/updated the relevant tests and verified the implementation and test suite. All previously identified concerns have been addressed.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
Posted by "somiljain2006 (via GitHub)" <gi...@apache.org>.
somiljain2006 commented on PR #39225:
URL: https://github.com/apache/shardingsphere/pull/39225#issuecomment-5259306267
@terrymanu Thanks for the detailed review. I’ve reworked the implementation around the requested architectural boundaries.
* PostgreSQL parameter type state is now retained by `PostgreSQLServerPreparedStatement` for Describe, Bind, repeated execution, and batch execution.
* Bind-first flows preserve `UNSPECIFIED` parameters until their types are resolved, without prematurely decoding them as JSON or treating arbitrary `Types.OTHER` values as JSON.
* Parameter resolution reuses the existing Binder/Describe semantics rather than maintaining a separate partial parameter-to-column traversal.
* `typeName` is handled as shared column metadata with the corresponding YAML swapper, persistence/reload support, and round-trip coverage.
* The existing fast path for already-resolved text parameters remains unchanged.
* Implicit INSERT columns, multi-row `VALUES`, UPDATE expressions, and multi-table statements are covered through the existing semantic resolution path.
* Added/updated coverage for Describe-first, Bind-first, repeated Bind, batch execution, custom types, ordinary `VARCHAR`, metadata persistence round trips, and the reused openGauss paths.
* All tests are passing locally, and I also fixed the OpenGauss test mock exposed by CI.
I believe the requested changes are now fully addressed. Please take another look when you have a chance.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
Posted by "somiljain2006 (via GitHub)" <gi...@apache.org>.
somiljain2006 commented on PR #39225:
URL: https://github.com/apache/shardingsphere/pull/39225#issuecomment-5135007752
@terrymanu Thanks for the review. They make the architectural concerns much clearer.
My understanding is that there are two ownership decisions we should resolve before continuing.
**1. Shared metadata contract**
One possible direction is to make `typeName` part of the shared metadata model. If that's the intended design, I'd complete the contract by persisting it through `YamlShardingSphereColumn` and `YamlColumnSwapper` so metadata survives the existing persistence and reload path.
Alternatively, if PostgreSQL type identity is intended to remain protocol-local, I'd remove `typeName` from the shared metadata model instead of leaving it half-integrated.
**2. Authoritative type resolution**
I agree that the resolver is too late in the extended protocol flow to become the authoritative owner.
My current thought is that type resolution should occur before parameter decoding, while reusing existing statement semantics (for example, Binder/Describe metadata) rather than introducing another statement-to-column mapping within the resolver. That would also allow the fast path to remain unchanged so ordinary text binds avoid additional metadata lookups.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #39225:
URL: https://github.com/apache/shardingsphere/pull/39225#issuecomment-5242574708
@somiljain2006 Your understanding is correct. Please use the following architectural boundaries:
1. PostgreSQL parameter type resolution and state should be owned by the PostgreSQL extended-protocol layer. `PostgreSQLServerPreparedStatement` should retain the parameter type information required by Describe, Bind, repeated execution, and batch execution. JDBC `ParameterMetaData` and schema metadata are information sources, not state owners.
Before Bind parses a value using a concrete type, the prepared statement must distinguish standard types, JSON, and custom types. If a Bind-first flow requires raw parameters for routing, preserve the raw `UNSPECIFIED` values until the types are known, and convert them afterward. Do not decode them as JSON prematurely or map arbitrary `Types.OTHER` values to JSON. Reuse the existing Binder or Describe statement semantics instead of maintaining another partial AST traversal in the resolver.
2. `typeName` may become part of the shared column metadata because it represents the database-native column type name. PostgreSQL may populate it first while other dialects leave it `null`. However, it must be a complete persistence contract, including `YamlShardingSphereColumn`, `YamlColumnSwapper`, persistence and reload paths, and round-trip tests.
The revised implementation should also ensure that:
- Already-resolved text parameters retain the existing fast path without additional routing, connection acquisition, or JDBC metadata queries.
- Implicit INSERT column lists, multi-row `VALUES`, UPDATE expressions, and multi-table statements are resolved through the existing semantic owner.
- Tests cover Describe-first, Bind-first, repeated Bind, batch execution, custom types, ordinary `VARCHAR`, metadata persistence round trips, and the reused openGauss paths.
Please rework the implementation around these boundaries. The current post-decoding fix and partial parameter-to-column mapping in the resolver should not be extended further.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
Posted by "somiljain2006 (via GitHub)" <gi...@apache.org>.
somiljain2006 commented on PR #39225:
URL: https://github.com/apache/shardingsphere/pull/39225#issuecomment-5301321854
The failure looks like a CI environment connection-limit problem, not a regression caused by my code
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
Posted by "somiljain2006 (via GitHub)" <gi...@apache.org>.
somiljain2006 commented on PR #39225:
URL: https://github.com/apache/shardingsphere/pull/39225#issuecomment-5262515553
@terrymanu Thanks for the detailed review.
My proposed design is:
### 1. Parameter state and wire OIDs
I will replace the current `List<PostgreSQLBinaryColumnType>`-only representation with a per-parameter state object, such as `PostgreSQLParameterState`.
The state will preserve the information required across Describe, Bind, repeated Bind, and batch execution, including the PostgreSQL wire OID, native type name where available, JDBC type where applicable, decoding strategy, and whether the type has been resolved.
Raw `UNSPECIFIED` Bind values will remain raw until the parameter type is resolved. Once resolved, that state will be retained by `PostgreSQLServerPreparedStatement` and reused for subsequent Bind and batch executions.
For JSON and JSONB, the state will preserve their distinct PostgreSQL identities rather than treating JSONB as JSON. For arbitrary `Types.OTHER`, the native type identity will be retained instead of mapping it to `VARCHAR`.
I will also restore the existing PostgreSQL/openGauss `ColumnTypeOIDResolver` and `ColumnTypeOIDLoader` path so composite fields continue to use their actual database wire OIDs.
### 2. Per-marker schema ownership
`ColumnSegmentBoundInfo` will be authoritative for parameter ownership.
Resolution will use the complete database/schema/table/column information associated with each marker rather than selecting a statement-wide schema. If the exact owner cannot be resolved unambiguously, that marker will remain unresolved rather than being resolved against a same-named object from another schema.
### 3. Describe completeness and JDBC fallback
I agree that partial inference must not be treated as complete.
The metadata-only Describe path will only be used when all parameter/projection types required for the response have been resolved unambiguously through the existing semantic/Binder and schema metadata paths.
If any required parameter or projection remains unresolved after those paths have been attempted—for example because of an unsupported expression, incomplete RETURNING ownership, or an ambiguous cross-schema reference—the statement will fall back to the existing JDBC metadata path.
This means `UNSPECIFIED` can remain an intermediate Bind-first state; it is the unresolved state after resolution has been exhausted that triggers the fallback.
### 4. Test boundaries
I will update the relevant tests so that Binder-dependent cases exercise the actual parsing/binding production path rather than manually injecting `ColumnSegmentBoundInfo`.
The tests will cover Describe-first, Bind-first, repeated Bind, batch execution, standard VARCHAR, JSON/JSONB, arbitrary custom types, schema-qualified/cross-schema statements, nested expressions and RETURNING, PostgreSQL/openGauss composite OIDs, and the already-resolved text fast path.
If this state model and completeness/fallback contract align with the intended architecture, I will proceed with `PostgreSQLParameterState` and the corresponding 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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
Posted by "somiljain2006 (via GitHub)" <gi...@apache.org>.
somiljain2006 commented on PR #39225:
URL: https://github.com/apache/shardingsphere/pull/39225#issuecomment-5063845858
@terrymanu Can you review this pr? The remaining 2 CI failures (MCP conformance test suite and the HiveConnectionPropertiesParser SPI issue) are unrelated to these 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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Resolve PostgreSQL custom parameter types using schema metadata [shardingsphere]
Posted by "somiljain2006 (via GitHub)" <gi...@apache.org>.
somiljain2006 closed pull request #39225: Resolve PostgreSQL custom parameter types using schema metadata
URL: https://github.com/apache/shardingsphere/pull/39225
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org