You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "susongyan (via GitHub)" <gi...@apache.org> on 2023/04/07 03:04:21 UTC
[GitHub] [shardingsphere] susongyan opened a new pull request, #25044: pg, set valueformat of row description in bind command
susongyan opened a new pull request, #25044:
URL: https://github.com/apache/shardingsphere/pull/25044
Fixes #ISSUSE_ID.
Changes proposed in this pull request:
-
---
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 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.
--
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
[GitHub] [shardingsphere] susongyan commented on a diff in pull request #25044: pg, set valueformat of row description in bind command
Posted by "susongyan (via GitHub)" <gi...@apache.org>.
susongyan commented on code in PR #25044:
URL: https://github.com/apache/shardingsphere/pull/25044#discussion_r1164930981
##########
db-protocol/postgresql/src/main/java/org/apache/shardingsphere/db/protocol/postgresql/packet/command/query/PostgreSQLRowDescriptionPacket.java:
##########
@@ -29,6 +30,7 @@
* Row description packet for PostgreSQL.
*/
@RequiredArgsConstructor
+@Getter
Review Comment:
only in test, i'l remove it
--
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
[GitHub] [shardingsphere] TeslaCN commented on a diff in pull request #25044: pg, set valueformat of row description in bind command
Posted by "TeslaCN (via GitHub)" <gi...@apache.org>.
TeslaCN commented on code in PR #25044:
URL: https://github.com/apache/shardingsphere/pull/25044#discussion_r1165030979
##########
proxy/frontend/type/postgresql/src/test/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/extended/PortalTest.java:
##########
@@ -145,7 +153,17 @@ void assertExecuteSelectStatementAndReturnAllRows() throws SQLException {
List<PostgreSQLValueFormat> resultFormats = new ArrayList<>(Arrays.asList(PostgreSQLValueFormat.TEXT, PostgreSQLValueFormat.BINARY));
Portal portal = new Portal("", preparedStatement, Collections.emptyList(), resultFormats, backendConnection);
portal.bind();
- assertThat(portal.describe(), instanceOf(PostgreSQLRowDescriptionPacket.class));
+
+ PostgreSQLPacket portalDescription = portal.describe();
+ assertThat(portalDescription, instanceOf(PostgreSQLRowDescriptionPacket.class));
+ Optional<Collection<PostgreSQLColumnDescription>> columnDescriptions = ReflectionUtils.getFieldValue(portalDescription, "columnDescriptions");
+ assertTrue(columnDescriptions.isPresent());
+ Iterator<PostgreSQLColumnDescription> columnDescriptionIterator = columnDescriptions.get().iterator();
+ PostgreSQLColumnDescription textColumnDescription = columnDescriptionIterator.next();
+ PostgreSQLColumnDescription intColumnDescription = columnDescriptionIterator.next();
+ assertEquals(textColumnDescription.getDataFormat(), PostgreSQLValueFormat.TEXT.getCode());
+ assertEquals(intColumnDescription.getDataFormat(), PostgreSQLValueFormat.BINARY.getCode());
Review Comment:
Please use `assertThat` here.
##########
proxy/frontend/type/postgresql/src/test/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/extended/PortalTest.java:
##########
@@ -145,7 +153,17 @@ void assertExecuteSelectStatementAndReturnAllRows() throws SQLException {
List<PostgreSQLValueFormat> resultFormats = new ArrayList<>(Arrays.asList(PostgreSQLValueFormat.TEXT, PostgreSQLValueFormat.BINARY));
Portal portal = new Portal("", preparedStatement, Collections.emptyList(), resultFormats, backendConnection);
portal.bind();
- assertThat(portal.describe(), instanceOf(PostgreSQLRowDescriptionPacket.class));
+
Review Comment:
Please remove blank line.
##########
db-protocol/postgresql/src/main/java/org/apache/shardingsphere/db/protocol/postgresql/packet/command/query/PostgreSQLColumnDescription.java:
##########
@@ -41,12 +42,20 @@ public final class PostgreSQLColumnDescription {
private final int typeModifier = -1;
- private final int dataFormat = 0;
+ private int dataFormat = PostgreSQLValueFormat.TEXT.getCode();
public PostgreSQLColumnDescription(final String columnName, final int columnIndex, final int columnType, final int columnLength, final String columnTypeName) {
this.columnName = columnName;
this.columnIndex = columnIndex;
this.columnLength = columnLength;
typeOID = Types.ARRAY == columnType ? PostgreSQLArrayColumnType.getTypeOid(columnTypeName) : PostgreSQLColumnType.valueOfJDBCType(columnType).getValue();
Review Comment:
Consider using `this(a, b, ...)` here.
##########
db-protocol/postgresql/src/main/java/org/apache/shardingsphere/db/protocol/postgresql/packet/command/query/PostgreSQLColumnDescription.java:
##########
@@ -41,12 +42,20 @@ public final class PostgreSQLColumnDescription {
private final int typeModifier = -1;
- private final int dataFormat = 0;
+ private int dataFormat = PostgreSQLValueFormat.TEXT.getCode();
Review Comment:
Please make this field final.
##########
proxy/frontend/type/postgresql/src/test/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/extended/PortalTest.java:
##########
@@ -132,8 +138,10 @@ void assertGetName() throws SQLException {
@Test
void assertExecuteSelectStatementAndReturnAllRows() throws SQLException {
QueryResponseHeader responseHeader = mock(QueryResponseHeader.class);
- QueryHeader queryHeader = new QueryHeader("schema", "table", "columnLabel", "columnName", Types.INTEGER, "columnTypeName", 0, 0, false, false, false, false);
- when(responseHeader.getQueryHeaders()).thenReturn(Collections.singletonList(queryHeader));
+ QueryHeader queryHeader = new QueryHeader("schema", "table", "columnLabel", "columnName", Types.VARCHAR, "columnTypeName", 0, 0, false, false, false, false);
+ QueryHeader intColumnQueryHeader = new QueryHeader("schema", "table", "columnLabel", "columnName", Types.INTEGER, "columnTypeName", 0, 0, false, false, false, false);
+ when(responseHeader.getQueryHeaders()).thenReturn(Arrays.asList(queryHeader, intColumnQueryHeader));
+
Review Comment:
Please remove blank line.
##########
proxy/frontend/type/postgresql/src/test/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/extended/PortalTest.java:
##########
@@ -145,7 +153,17 @@ void assertExecuteSelectStatementAndReturnAllRows() throws SQLException {
List<PostgreSQLValueFormat> resultFormats = new ArrayList<>(Arrays.asList(PostgreSQLValueFormat.TEXT, PostgreSQLValueFormat.BINARY));
Portal portal = new Portal("", preparedStatement, Collections.emptyList(), resultFormats, backendConnection);
portal.bind();
- assertThat(portal.describe(), instanceOf(PostgreSQLRowDescriptionPacket.class));
+
+ PostgreSQLPacket portalDescription = portal.describe();
+ assertThat(portalDescription, instanceOf(PostgreSQLRowDescriptionPacket.class));
+ Optional<Collection<PostgreSQLColumnDescription>> columnDescriptions = ReflectionUtils.getFieldValue(portalDescription, "columnDescriptions");
+ assertTrue(columnDescriptions.isPresent());
+ Iterator<PostgreSQLColumnDescription> columnDescriptionIterator = columnDescriptions.get().iterator();
+ PostgreSQLColumnDescription textColumnDescription = columnDescriptionIterator.next();
+ PostgreSQLColumnDescription intColumnDescription = columnDescriptionIterator.next();
+ assertEquals(textColumnDescription.getDataFormat(), PostgreSQLValueFormat.TEXT.getCode());
+ assertEquals(intColumnDescription.getDataFormat(), PostgreSQLValueFormat.BINARY.getCode());
+
Review Comment:
Please remove blank line.
--
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
[GitHub] [shardingsphere] RaigorJiang commented on pull request #25044: pg, set valueformat of row description in bind command
Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on PR #25044:
URL: https://github.com/apache/shardingsphere/pull/25044#issuecomment-1507859554
Merged, thank you! @susongyan @TeslaCN
--
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
[GitHub] [shardingsphere] TeslaCN commented on a diff in pull request #25044: pg, set valueformat of row description in bind command
Posted by "TeslaCN (via GitHub)" <gi...@apache.org>.
TeslaCN commented on code in PR #25044:
URL: https://github.com/apache/shardingsphere/pull/25044#discussion_r1163688208
##########
db-protocol/postgresql/src/main/java/org/apache/shardingsphere/db/protocol/postgresql/packet/command/query/PostgreSQLRowDescriptionPacket.java:
##########
@@ -29,6 +30,7 @@
* Row description packet for PostgreSQL.
*/
@RequiredArgsConstructor
+@Getter
Review Comment:
Do we need this getter in production code? If not, please access this field by Mockito Plugins or reflection.
--
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
[GitHub] [shardingsphere] RaigorJiang merged pull request #25044: pg, set valueformat of row description in bind command
Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang merged PR #25044:
URL: https://github.com/apache/shardingsphere/pull/25044
--
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
[GitHub] [shardingsphere] RaigorJiang commented on pull request #25044: pg, set valueformat of row description in bind command
Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on PR #25044:
URL: https://github.com/apache/shardingsphere/pull/25044#issuecomment-1501318583
Many CI actions failed, I try rerun 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
[GitHub] [shardingsphere] susongyan commented on pull request #25044: pg, set valueformat of row description in bind command
Posted by "susongyan (via GitHub)" <gi...@apache.org>.
susongyan commented on PR #25044:
URL: https://github.com/apache/shardingsphere/pull/25044#issuecomment-1500416209
> Hi @susongyan, Could you add some unit tests for this change?
ok, i'll add test for portal after bind
> Hi @susongyan, Could you add some unit tests for this change?
ok , i'll add data format test for port.describe
--
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