Skip to content

Commit

Permalink
Make SqlVarbinary map serialization consistent with other types
Browse files Browse the repository at this point in the history
Other Sql* classes are serialized to JSON on-the-wire format,
using @JsonValue annotated toString methods, except for SqlVarbinary
which was serialized using its' getBytes() method that was Base64-encoded
to a map key.
  • Loading branch information
wendigo committed Nov 22, 2024
1 parent bbe3021 commit 709ff9e
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@

import java.io.IOException;
import java.math.BigDecimal;
import java.util.Base64;
import java.util.List;
import java.util.function.Consumer;

Expand Down Expand Up @@ -369,13 +368,9 @@ public void encode(JsonGenerator generator, ConnectorSession session, Block bloc
generator.writeStartObject();
for (int i = 0; i < map.getSize(); i++) {
// Field name is always written as String for backward compatibility,
// (except for VarbinaryType where we encode it as Base64 representation of its bytes)
// and only value is properly encoded using it's type.
// TODO: improve in v2 JSON format
switch (mapType.getKeyType().getObjectValue(session, keyBlock, offset + i)) {
case SqlVarbinary varbinary -> generator.writeFieldName(Base64.getEncoder().encodeToString(varbinary.getBytes()));
case Object value -> generator.writeFieldName(value.toString());
}
generator.writeFieldName(mapType.getKeyType().getObjectValue(session, keyBlock, offset + i).toString());
valueEncoder.encode(generator, session, valueBlock, offset + i);
}
generator.writeEndObject();
Expand Down
8 changes: 8 additions & 0 deletions core/trino-spi/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,14 @@
</item>
<!-- Backwards incompatible changes since the previous release -->
<!-- Any exclusions below can be deleted after each release -->
<item>
<ignore>true</ignore>
<code>java.annotation.removed</code>
<old>method byte[] io.trino.spi.type.SqlVarbinary::getBytes()</old>
<new>method byte[] io.trino.spi.type.SqlVarbinary::getBytes()</new>
<annotation>@com.fasterxml.jackson.annotation.JsonValue</annotation>
<justification>On-the-wire representation shouldn't rely on the Jackson format</justification>
</item>
</differences>
</revapi.differences>
</analysisConfiguration>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@
*/
package io.trino.spi.type;

import com.fasterxml.jackson.annotation.JsonValue;

import java.util.Arrays;
import java.util.Base64;
import java.util.HexFormat;

import static java.lang.Math.min;
Expand All @@ -42,7 +41,6 @@ public int compareTo(SqlVarbinary obj)
return Arrays.compare(bytes, obj.bytes);
}

@JsonValue
public byte[] getBytes()
{
return bytes;
Expand All @@ -69,6 +67,11 @@ public boolean equals(Object obj)

@Override
public String toString()
{
return Base64.getEncoder().encodeToString(bytes);
}

public String toHexString()
{
if (bytes.length == 0) {
return "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,27 @@

import org.junit.jupiter.api.Test;

import java.util.Base64;

import static java.lang.String.format;
import static org.assertj.core.api.Assertions.assertThat;

public class TestSqlVarbinary
{
@Test
public void testToString()
public void testToHexString()
{
for (int lines = 0; lines < 5; lines++) {
for (int lastLineBytes = 0; lastLineBytes < 32; lastLineBytes++) {
byte[] bytes = createBytes(lines, lastLineBytes);
String expected = simpleToString(bytes);
assertThat(expected).isEqualTo(new SqlVarbinary(bytes).toString());
String expectedHex = simpleToHex(bytes);
assertThat(expectedHex).isEqualTo(new SqlVarbinary(bytes).toHexString());
assertThat(Base64.getEncoder().encodeToString(bytes)).isEqualTo(new SqlVarbinary(bytes).toString());
}
}
}

private static String simpleToString(byte[] bytes)
private static String simpleToHex(byte[] bytes)
{
StringBuilder builder = new StringBuilder();

Expand Down

0 comments on commit 709ff9e

Please sign in to comment.