abhinav-phi opened a new issue, #40230: URL: https://github.com/apache/shardingsphere/issues/40230
### Before reporting a bug - [x] I have searched open and closed [GitHub issues](https://github.com/apache/shardingsphere/issues). - [x] I have read the [ShardingSphere documentation](https://shardingsphere.apache.org/document/current/en/overview). ### ShardingSphere version master, commit `8dc4cdf6` (the base of #40013). The two files involved are byte identical between that base and the head of #40013, so this is not something that PR introduces or changes. ### ShardingSphere project ShardingSphere-Proxy ### Expected behavior `FirebirdBinaryColumnType` carries the on the wire octet count of each type, and `FirebirdBinaryProtocolValueFactory` routes each type to a writer of that width. A 128 bit decimal should be written as 16 octets, a 64 bit decimal as 8, and a quad as 8, the same width the server uses for the blob and slice identifiers that are quads. ### Actual behavior Three types are declared and encoded at the wrong width, and the three disagree with their own siblings in the same enum. `database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/packet/command/query/FirebirdBinaryColumnType.java`, the constructor fields are `value, length, subtype` at lines 73 to 77: | line | entry | declared length | | --- | --- | --- | | 49 | `BLOB(520, 8)` | 8 | | 51 | `QUAD(550, 4)` | 4 | | 59 | `INT128(32752, 16)` | 16 | | 62 | `DEC16(32760, 2)` | 2 | | 63 | `DEC34(32762, 4)` | 4 | `database/protocol/dialect/firebird/.../statement/execute/protocol/FirebirdBinaryProtocolValueFactory.java` then routes them by width: | line | mapping | writer width | | --- | --- | --- | | 70 | `INT128` to `FirebirdInt16BinaryProtocolValue` | 16, correct | | 75 to 77 | `INT64`, `NUMERIC`, `DECIMAL` to `FirebirdInt8BinaryProtocolValue` | 8, correct | | 78 | `DEC34` to `FirebirdInt8BinaryProtocolValue` | 8 | | 83 to 85 | `LONG`, `DEC16`, `QUAD` to `FirebirdInt4BinaryProtocolValue` | 4 | So a `DEC34`, which the enum's own BLR table calls `blr_dec128` at line 122, is written as 8 octets by the same writer used for the 64 bit `DECIMAL`. A `DEC16`, called `blr_dec64` at line 121, is written as 4 octets by the writer used for `LONG`. `QUAD` is written as 4. The two internal contradictions that make this checkable without a live server: 1. `DEC34` is `blr_dec128` and `INT128` is `blr_int128`, both 128 bit, declared 4 and 16 in the same enum and routed to writers of 8 and 16. One of the two is wrong, and it is not `INT128`. 2. `QUAD` is declared 4 while `BLOB` is declared 8. In the server's XDR layer the blob handle, the slice handle and the batch identifiers are all `xdr_quad`, see `MAP(xdr_quad, blob->p_blob_id)`, `MAP(xdr_quad, slice->p_slc_id)` and the size accounting comment `+ 8 // xdr_sizeof (xdr_quad, response->p_resp_blob_id)`. A quad is 8 octets, so `BLOB(520, 8)` is right and `QUAD(550, 4)` is wrong. This is not only metadata. `FirebirdReturnColumnPacket.java:86` does `int length = columnType.getLength();`, so the wrong declared width is emitted in the returned column descriptor, and the factory routing above is what writes the value itself on the statement execute path. ### Reason analysis The enum looks like it was filled from BLR in memory sizes rather than from the XDR wire form. In BLR a quad is two `ISC_SHORT` so 4, and `FB_TIME_TZ` is `ISC_TIME` plus `ISC_SHORT` so 6, but XDR pads every 16 and 32 bit field to 4 octets, which is why the wire quad is 8. `DEC16` at 2 and `DEC34` at 4 do not match either representation, they look like digit count placeholders, 16 and 34 digits, that were written as byte counts. ### Steps to reproduce I want to be straight that I do not have a runnable repro to paste here, I have no Firebird server and this environment cannot build the module. Everything above is from reading the two files at the commit named. To reproduce at runtime you need a result set with one of these columns, which Firebird 4 gives you for `DECIMAL(16)` and `DECIMAL(34)` declarations and for any `BLOB` id, and read it through the Firebird database protocol module with `FirebirdBinaryColumnType.getLength()` and the factory lookup instrumented. What the code says will happen is a `DEC34` value reaching the client as 8 octets instead of 16. A cheap static check instead: the two contradictions listed under actual behavior are decidable from the files alone and do not need a server. ### Example code No snippet, the line references above are the whole case. Related to #40013 but not part of it, that PR confines its own handling to the slice response path and deliberately leaves this shared factory untouched, which is why I am filing it separately instead of widening that diff. -- 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]
