Skip to content

Commit dc0d482

Browse files
authored
fix: CopyResponse did not return correct column format (#633)
CopyResponse for BINARY copy statements did not return the correct format code for the individual columns. The COPY protocol will always use either only TEXT or only BINARY for each column, and the overall format is given in a separate field. The format per column is a precaution in the protocol to support a mix of TEXT and BINARY in potential future versions of the protocol. Most clients therefore ignore the values for the separate columns in a COPY statement. psycopg3 however does inspect these values and will fail if they contain an invalid value. PGAdapter would return a byte array containing only 1's for the column formats if the BINARY protocol was used. As the individual column formats are interpreted as int2 values, that meant that the client saw a format code value of 257 instead of 1.
1 parent 54effed commit dc0d482

File tree

4 files changed

+82
-10
lines changed

4 files changed

+82
-10
lines changed

src/main/java/com/google/cloud/spanner/pgadapter/statements/CopyStatement.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,8 @@ public MutationWriter getMutationWriter() {
241241
}
242242

243243
/** @return 0 for text/csv formatting and 1 for binary */
244-
public int getFormatCode() {
245-
return (parsedCopyStatement.format == Format.BINARY) ? 1 : 0;
244+
public byte getFormatCode() {
245+
return (parsedCopyStatement.format == Format.BINARY) ? (byte) 1 : (byte) 0;
246246
}
247247

248248
private void verifyCopyColumns() {

src/main/java/com/google/cloud/spanner/pgadapter/utils/CopyDataReceiver.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ void handleCopy() throws Exception {
6565
this.connectionHandler.setActiveCopyStatement(copyStatement);
6666
new CopyInResponse(
6767
this.connectionHandler.getConnectionMetadata().getOutputStream(),
68-
copyStatement.getTableColumns().size(),
68+
(short) copyStatement.getTableColumns().size(),
6969
copyStatement.getFormatCode())
7070
.send();
7171
ConnectionStatus initialConnectionStatus = this.connectionHandler.getStatus();

src/main/java/com/google/cloud/spanner/pgadapter/wireoutput/CopyInResponse.java

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,23 +35,25 @@ public class CopyInResponse extends WireOutput {
3535

3636
protected static final char IDENTIFIER = 'G';
3737

38-
private final int numColumns;
39-
private final int formatCode;
40-
private final byte[] columnFormat;
38+
private final short numColumns;
39+
private final byte formatCode;
40+
private final short[] columnFormat;
4141

42-
public CopyInResponse(DataOutputStream output, int numColumns, int formatCode) {
42+
public CopyInResponse(DataOutputStream output, short numColumns, byte formatCode) {
4343
super(output, calculateLength(numColumns));
4444
this.numColumns = numColumns;
4545
this.formatCode = formatCode;
46-
columnFormat = new byte[COLUMN_NUM_LENGTH * this.numColumns];
47-
Arrays.fill(columnFormat, (byte) formatCode);
46+
this.columnFormat = new short[this.numColumns];
47+
Arrays.fill(this.columnFormat, formatCode);
4848
}
4949

5050
@Override
5151
protected void sendPayload() throws IOException {
5252
this.outputStream.writeByte(this.formatCode);
5353
this.outputStream.writeShort(this.numColumns);
54-
this.outputStream.write(this.columnFormat);
54+
for (int i = 0; i < this.numColumns; i++) {
55+
this.outputStream.writeShort(this.columnFormat[i]);
56+
}
5557
}
5658

5759
@Override
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Copyright 2023 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.cloud.spanner.pgadapter.wireoutput;
16+
17+
import static org.junit.Assert.assertArrayEquals;
18+
19+
import com.google.cloud.spanner.pgadapter.ProxyServer.DataFormat;
20+
import java.io.ByteArrayOutputStream;
21+
import java.io.DataOutputStream;
22+
import org.junit.Test;
23+
import org.junit.runner.RunWith;
24+
import org.junit.runners.JUnit4;
25+
26+
@RunWith(JUnit4.class)
27+
public class CopyInResponseTest {
28+
29+
@Test
30+
public void testSendPayloadText() throws Exception {
31+
ByteArrayOutputStream bytes = new ByteArrayOutputStream();
32+
DataOutputStream output = new DataOutputStream(bytes);
33+
short numColumns = 3;
34+
35+
CopyInResponse response =
36+
new CopyInResponse(output, numColumns, (byte) DataFormat.POSTGRESQL_TEXT.getCode());
37+
response.sendPayload();
38+
39+
assertArrayEquals(
40+
new byte[] {
41+
0, // format
42+
0, 3, // numColumns
43+
0, 0, // format column 1
44+
0, 0, // format column 2
45+
0, 0, // format column 3
46+
},
47+
bytes.toByteArray());
48+
}
49+
50+
@Test
51+
public void testSendPayloadBinary() throws Exception {
52+
ByteArrayOutputStream bytes = new ByteArrayOutputStream();
53+
DataOutputStream output = new DataOutputStream(bytes);
54+
short numColumns = 3;
55+
56+
CopyInResponse response =
57+
new CopyInResponse(output, numColumns, (byte) DataFormat.POSTGRESQL_BINARY.getCode());
58+
response.sendPayload();
59+
60+
assertArrayEquals(
61+
new byte[] {
62+
1, // format
63+
0, 3, // numColumns
64+
0, 1, // format column 1
65+
0, 1, // format column 2
66+
0, 1, // format column 3
67+
},
68+
bytes.toByteArray());
69+
}
70+
}

0 commit comments

Comments
 (0)