zxs1633079383 commented on issue #36978: URL: https://github.com/apache/shardingsphere/issues/36978#issuecomment-3580567164
Hi @terrymanu Thank you again for your detailed guidance. I have now completed the PostgreSQL part of the solution and would like to provide an update and ask for confirmation regarding the PR scope. Current Status — PostgreSQL Part Fully Implemented Following your Phase 1–3 design, I have completed the implementation for PostgreSQL. Metadata Layer - ColumnMetaData now includes typeName - PostgreSQLMetaDataLoader correctly reads udt_name - ShardingSphereColumn updated to store typeName - YAML Swapper updated to support typeName in both directions (YAML to Column, Column to YAML) Protocol Layer - Extended PostgreSQLColumnType.valueOfJDBCType(int, String) - Added UDT_GENERIC fallback logic - Implemented PostgreSQLUDTValueParser - BIND executor now correctly injects PGobject(typeName, value) Validation Using Real PostgreSQL I tested with tables that include: - ENUM UDT (post_type) - bit varying - jsonb - varchar[] - bytea - boolean - complex update expressions such as bit operations and JSONB casting All behaviors are now correct when passing through ShardingSphere-Proxy: - Insert works correctly - Update works correctly - Select works correctly - Complex expressions such as read_bits | CAST(...) work correctly - PGobject consistently receives the correct PostgreSQL typeName This fully resolves the original issue: custom PostgreSQL UDT types no longer break under ShardingSphere-Proxy. Question About Multi-Dialect Implementation Scope Your proposed solution is naturally PostgreSQL-first. For other dialects (MySQL, SQLServer, Firebird, OpenGauss): - They currently do not extract typeName - They do not expose UDT information - Their metadata models differ significantly - Extracting correct typeName would require dialect maintainers to design their own logic Therefore, I would like to confirm the following: ### Question 1 Is it acceptable for my PR to implement the feature for PostgreSQL only, while keeping typeName as null (placeholder) for other dialects? Dialect | typeName after PR PostgreSQL | fully implemented MySQL | null (unchanged) SQLServer | null (unchanged) Firebird | null (unchanged) OpenGauss | null (unchanged) ### Question 2 For unit tests of other dialects: Is it acceptable that tests simply verify typeName == null, without requiring full typeName extraction for those databases? This approach ensures: - PostgreSQL improvements are merged - No regressions for other dialects - Dialect maintainers can independently extend typeName support later - This seems consistent with your earlier suggestion that PostgreSQL implementation can be contributed first. ### Summary I have completed: - Full PostgreSQL UDT, bit varying, and JSONB support - Updated metadata, parser, binding, and execution paths - Ensured no regressions for other dialects - All PostgreSQL-related tests pass I would like to proceed with submitting a PR that covers only the PostgreSQL part, with placeholder support for other dialects. Could you confirm that this PR scope is appropriate? Thanks again for your time and guidance — it has been extremely helpful for contributing to ShardingSphere. -- 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]
