Copilot commented on code in PR #37239:
URL: https://github.com/apache/shardingsphere/pull/37239#discussion_r2581468334
##########
database/protocol/dialect/postgresql/src/main/java/org/apache/shardingsphere/database/protocol/postgresql/packet/command/query/PostgreSQLDataRowPacket.java:
##########
@@ -71,6 +71,10 @@ private void writeTextValue(final PostgreSQLPacketPayload
payload, final Object
payload.writeBytes((byte[]) each);
} else if (each instanceof SQLXML) {
writeSQLXMLData(payload, each);
+ } else if (each instanceof Boolean) {
+ byte[] columnData = ((Boolean) each ? "t" :
"f").getBytes(payload.getCharset());
+ payload.writeInt4(columnData.length);
+ payload.writeBytes(columnData);
Review Comment:
The Boolean to PostgreSQL text format conversion logic is duplicated. The
codebase already has `PostgreSQLTextBoolUtils.getTextValue()` which performs
the same conversion (Boolean to "t"/"f"). Consider refactoring to use:
```java
} else if (each instanceof Boolean) {
byte[] columnData =
PostgreSQLTextBoolUtils.getTextValue(each).getBytes(payload.getCharset());
payload.writeInt4(columnData.length);
payload.writeBytes(columnData);
```
This would ensure consistency with the conversion logic used in
`Portal.java` (line 201) and reduce code duplication.
##########
database/protocol/dialect/postgresql/src/main/java/org/apache/shardingsphere/database/protocol/postgresql/packet/command/query/PostgreSQLDataRowPacket.java:
##########
@@ -71,6 +71,10 @@ private void writeTextValue(final PostgreSQLPacketPayload
payload, final Object
payload.writeBytes((byte[]) each);
} else if (each instanceof SQLXML) {
writeSQLXMLData(payload, each);
+ } else if (each instanceof Boolean) {
+ byte[] columnData = ((Boolean) each ? "t" :
"f").getBytes(payload.getCharset());
+ payload.writeInt4(columnData.length);
+ payload.writeBytes(columnData);
Review Comment:
The new Boolean handling logic should have corresponding test coverage.
Consider adding tests for both `true` and `false` Boolean values to ensure they
are correctly serialized as 't' and 'f' respectively. The existing test file
`PostgreSQLDataRowPacketTest.java` has tests for other data types (null,
byte[], SQLXML, String), and should include a test for Boolean values as well.
--
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]
