schizophrenicmaniac commented on PR #40212: URL: https://github.com/apache/shardingsphere/pull/40212#issuecomment-6228675046
@terrymanu Thanks for the detailed review. I pushed a follow-up commit for points 1 to 4. 1. The bool text parser now follows PostgreSQL's input rules: t/true/y/yes/on/1 and the matching false forms, unique prefixes, any case, surrounding spaces. Anything else fails with SQLSTATE 22P02 instead of quietly becoming false. 2. TIMESTAMPTZ text now keeps its offset and decodes to the instant it represents, while TIMESTAMP still ignores the offset like PostgreSQL does. When JDBC metadata reports `Types.TIMESTAMP` with type name `timestamptz`, the parameter is resolved as TIMESTAMPTZ. One gap is left: for INSERT the types come from `ShardingSphereColumn`, which only has the JDBC type, so a timestamptz column still resolves to TIMESTAMP on that path. Closing it would mean carrying the column type name in the table metadata. Would you rather have that in this PR, or should OID 0 timestamp parameters of INSERT stay undecoded for now? 3. NUMERIC text accepts NaN, Infinity, -Infinity, inf and the signed forms. BigDecimal can't represent those, so they are decoded as Double. 4. Both the INSERT path and the JDBC metadata path now map each marker to its PostgreSQL parameter index through `actualParameterMarkerIndexes`, so something like `VALUES ($2, $1)` gets the right types. `PostgreSQLComBindPacketTest` now covers bool, the NUMERIC special values and timestamptz with different offsets through the actual Bind text decoding. -- 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]
