Skip to content

Commit

Permalink
SNOW-1374896 unify structured types string representation (#1882)
Browse files Browse the repository at this point in the history
Co-authored-by: sfc-gh-astachowski <antoni.stachowski@snowflake.com>

Build string representations of Snowflake structured types recursively to reuse existing converters designed for specific logical types (e.g. timestamps/binary)

https://snowflakecomputing.atlassian.net/browse/SNOW-1374896
  • Loading branch information
sfc-gh-mkubik committed Sep 27, 2024
1 parent 9597576 commit 9e221ea
Show file tree
Hide file tree
Showing 18 changed files with 459 additions and 93 deletions.
2 changes: 1 addition & 1 deletion src/main/java/net/snowflake/client/core/ResultUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ public static String getSFTimeAsString(
* @return boolean in string
*/
public static String getBooleanAsString(boolean bool) {
return bool ? "TRUE" : "FALSE";
return bool ? "true" : "false";
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

import net.snowflake.client.core.DataConversionContext;
import net.snowflake.client.core.SFException;
import net.snowflake.client.core.arrow.tostringhelpers.ArrowArrayStringRepresentationBuilder;
import net.snowflake.client.jdbc.SnowflakeSQLException;
import net.snowflake.client.jdbc.SnowflakeType;
import org.apache.arrow.vector.FieldVector;
import org.apache.arrow.vector.complex.ListVector;

public class ArrayConverter extends AbstractArrowVectorConverter {
Expand All @@ -21,6 +24,25 @@ public Object toObject(int index) throws SFException {

@Override
public String toString(int index) throws SFException {
return vector.getObject(index).toString();
FieldVector vectorUnpacked = vector.getChildrenFromFields().get(0);
SnowflakeType logicalType =
ArrowVectorConverterUtil.getSnowflakeTypeFromFieldMetadata(vectorUnpacked.getField());

ArrowArrayStringRepresentationBuilder builder =
new ArrowArrayStringRepresentationBuilder(logicalType);

final ArrowVectorConverter converter;

try {
converter = ArrowVectorConverterUtil.initConverter(vectorUnpacked, context, columnIndex);
} catch (SnowflakeSQLException e) {
return vector.getObject(index).toString();
}

for (int i = vector.getElementStartIndex(index); i < vector.getElementEndIndex(index); i++) {
builder.appendValue(converter.toString(i));
}

return builder.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,28 @@
import net.snowflake.client.jdbc.SnowflakeSQLLoggedException;
import net.snowflake.client.jdbc.SnowflakeType;
import net.snowflake.common.core.SqlState;
import org.apache.arrow.vector.FieldVector;
import org.apache.arrow.vector.ValueVector;
import org.apache.arrow.vector.complex.FixedSizeListVector;
import org.apache.arrow.vector.complex.ListVector;
import org.apache.arrow.vector.complex.MapVector;
import org.apache.arrow.vector.complex.StructVector;
import org.apache.arrow.vector.types.Types;
import org.apache.arrow.vector.types.pojo.Field;

@SnowflakeJdbcInternalApi
public final class ArrowVectorConverterUtil {
private ArrowVectorConverterUtil() {}

public static SnowflakeType getSnowflakeTypeFromFieldMetadata(Field field) {
Map<String, String> customMeta = field.getMetadata();
if (customMeta != null && customMeta.containsKey("logicalType")) {
return SnowflakeType.valueOf(customMeta.get("logicalType"));
}

return null;
}

/**
* Given an arrow vector (a single column in a single record batch), return an arrow vector
* converter. Note, converter is built on top of arrow vector, so that arrow data can be converted
Expand Down Expand Up @@ -51,12 +62,11 @@ public static ArrowVectorConverter initConverter(
Types.MinorType type = Types.getMinorTypeForArrowType(vector.getField().getType());

// each column's metadata
Map<String, String> customMeta = vector.getField().getMetadata();
SnowflakeType st = getSnowflakeTypeFromFieldMetadata(vector.getField());
if (type == Types.MinorType.DECIMAL) {
// Note: Decimal vector is different from others
return new DecimalToScaledFixedConverter(vector, idx, context);
} else if (!customMeta.isEmpty()) {
SnowflakeType st = SnowflakeType.valueOf(customMeta.get("logicalType"));
} else if (st != null) {
switch (st) {
case ANY:
case CHAR:
Expand Down Expand Up @@ -216,4 +226,10 @@ public static ArrowVectorConverter initConverter(
"Unexpected Arrow Field for ",
type.toString());
}

public static ArrowVectorConverter initConverter(
FieldVector vector, DataConversionContext context, int columnIndex)
throws SnowflakeSQLException {
return initConverter(vector, context, context.getSession(), columnIndex);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public Object toObject(int index) {

@Override
public String toString(int index) {
return isNull(index) ? null : toBoolean(index) ? "TRUE" : "FALSE";
return isNull(index) ? null : toBoolean(index) ? "true" : "false";
}

@Override
Expand Down
29 changes: 28 additions & 1 deletion src/main/java/net/snowflake/client/core/arrow/MapConverter.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
import java.util.stream.Collectors;
import net.snowflake.client.core.DataConversionContext;
import net.snowflake.client.core.SFException;
import net.snowflake.client.core.arrow.tostringhelpers.ArrowObjectStringRepresentationBuilder;
import net.snowflake.client.jdbc.SnowflakeSQLException;
import net.snowflake.client.jdbc.SnowflakeType;
import org.apache.arrow.vector.FieldVector;
import org.apache.arrow.vector.complex.MapVector;
import org.apache.arrow.vector.util.JsonStringHashMap;

Expand All @@ -28,6 +31,30 @@ public Object toObject(int index) throws SFException {

@Override
public String toString(int index) throws SFException {
return vector.getObject(index).toString();
ArrowObjectStringRepresentationBuilder builder = new ArrowObjectStringRepresentationBuilder();

FieldVector vectorUnpacked = vector.getChildrenFromFields().get(0);

FieldVector keys = vectorUnpacked.getChildrenFromFields().get(0);
FieldVector values = vectorUnpacked.getChildrenFromFields().get(1);
final ArrowVectorConverter keyConverter;
final ArrowVectorConverter valueConverter;

SnowflakeType valueLogicalType =
ArrowVectorConverterUtil.getSnowflakeTypeFromFieldMetadata(values.getField());

try {
keyConverter = ArrowVectorConverterUtil.initConverter(keys, context, columnIndex);
valueConverter = ArrowVectorConverterUtil.initConverter(values, context, columnIndex);
} catch (SnowflakeSQLException e) {
return vector.getObject(index).toString();
}

for (int i = vector.getElementStartIndex(index); i < vector.getElementEndIndex(index); i++) {
builder.appendKeyValue(
keyConverter.toString(i), valueConverter.toString(i), valueLogicalType);
}

return builder.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import net.snowflake.client.core.DataConversionContext;
import net.snowflake.client.core.SFException;
import net.snowflake.client.core.SnowflakeJdbcInternalApi;
import net.snowflake.client.core.arrow.tostringhelpers.ArrowObjectStringRepresentationBuilder;
import net.snowflake.client.jdbc.SnowflakeSQLException;
import net.snowflake.client.jdbc.SnowflakeType;
import org.apache.arrow.vector.FieldVector;
import org.apache.arrow.vector.complex.StructVector;

@SnowflakeJdbcInternalApi
Expand All @@ -23,6 +26,19 @@ public Object toObject(int index) throws SFException {

@Override
public String toString(int index) throws SFException {
return structVector.getObject(index).toString();
ArrowObjectStringRepresentationBuilder builder = new ArrowObjectStringRepresentationBuilder();
for (String childName : structVector.getChildFieldNames()) {
FieldVector fieldVector = structVector.getChild(childName);
SnowflakeType logicalType =
ArrowVectorConverterUtil.getSnowflakeTypeFromFieldMetadata(fieldVector.getField());
try {
ArrowVectorConverter converter =
ArrowVectorConverterUtil.initConverter(fieldVector, context, columnIndex);
builder.appendKeyValue(childName, converter.toString(index), logicalType);
} catch (SnowflakeSQLException e) {
return structVector.getObject(index).toString();
}
}
return builder.toString();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package net.snowflake.client.core.arrow.tostringhelpers;

import net.snowflake.client.core.SnowflakeJdbcInternalApi;
import net.snowflake.client.jdbc.SnowflakeType;

@SnowflakeJdbcInternalApi
public class ArrowArrayStringRepresentationBuilder extends ArrowStringRepresentationBuilderBase {

private final SnowflakeType valueType;

public ArrowArrayStringRepresentationBuilder(SnowflakeType valueType) {
super(",", "[", "]");
this.valueType = valueType;
}

public ArrowStringRepresentationBuilderBase appendValue(String value) {
return add(quoteIfNeeded(value, valueType));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package net.snowflake.client.core.arrow.tostringhelpers;

import java.util.StringJoiner;
import net.snowflake.client.core.SnowflakeJdbcInternalApi;
import net.snowflake.client.jdbc.SnowflakeType;

@SnowflakeJdbcInternalApi
public class ArrowObjectStringRepresentationBuilder extends ArrowStringRepresentationBuilderBase {

public ArrowObjectStringRepresentationBuilder() {
super(",", "{", "}");
}

public ArrowStringRepresentationBuilderBase appendKeyValue(
String key, String value, SnowflakeType valueType) {
StringJoiner joiner = new StringJoiner(": ");
joiner.add('"' + key + '"');
joiner.add(quoteIfNeeded(value, valueType));
return add(joiner.toString());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package net.snowflake.client.core.arrow.tostringhelpers;

import java.util.HashSet;
import java.util.Set;
import java.util.StringJoiner;
import net.snowflake.client.core.SnowflakeJdbcInternalApi;
import net.snowflake.client.jdbc.SnowflakeType;

@SnowflakeJdbcInternalApi
public abstract class ArrowStringRepresentationBuilderBase {
private final StringJoiner joiner;
private static final Set<SnowflakeType> quotableTypes;

static {
quotableTypes = new HashSet<>();
quotableTypes.add(SnowflakeType.ANY);
quotableTypes.add(SnowflakeType.CHAR);
quotableTypes.add(SnowflakeType.TEXT);
quotableTypes.add(SnowflakeType.VARIANT);
quotableTypes.add(SnowflakeType.BINARY);
quotableTypes.add(SnowflakeType.DATE);
quotableTypes.add(SnowflakeType.TIME);
quotableTypes.add(SnowflakeType.TIMESTAMP_LTZ);
quotableTypes.add(SnowflakeType.TIMESTAMP_NTZ);
quotableTypes.add(SnowflakeType.TIMESTAMP_TZ);
}

public ArrowStringRepresentationBuilderBase(String delimiter, String prefix, String suffix) {
joiner = new StringJoiner(delimiter, prefix, suffix);
}

protected ArrowStringRepresentationBuilderBase add(String string) {
joiner.add(string);
return this;
}

private boolean shouldQuoteValue(SnowflakeType type) {
return quotableTypes.contains(type);
}

protected String quoteIfNeeded(String string, SnowflakeType type) {
if (shouldQuoteValue(type)) {
return '"' + string + '"';
}

return string;
}

@Override
public String toString() {
return joiner.toString();
}
}
10 changes: 10 additions & 0 deletions src/test/java/net/snowflake/client/TestUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,14 @@ public static void expectSnowflakeLoggedFeatureNotSupportedException(MethodRaise
assertEquals(ex.getClass().getSimpleName(), "SnowflakeLoggedFeatureNotSupportedException");
}
}

/**
* Compares two string values both values are cleaned of whitespaces
*
* @param expected expected value
* @param actual actual value
*/
public static void assertEqualsIgnoringWhitespace(String expected, String actual) {
assertEquals(expected.replaceAll("\\s+", ""), actual.replaceAll("\\s+", ""));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void testConvertToString() throws SFException {
} else {
assertThat(boolVal, is(expectedValues.get(i)));
assertThat(objectVal, is(expectedValues.get(i)));
assertThat(stringVal, is(expectedValues.get(i).toString().toUpperCase()));
assertThat(stringVal, is(expectedValues.get(i).toString()));
if (boolVal) {
assertThat((byte) 0x1, is(converter.toBytes(i)[0]));
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ public void testConvertingString() throws SFException {

@Test
public void testConvertingBoolean() throws SFException {
assertEquals("TRUE", stringConverter.getString(true, Types.BOOLEAN, Types.BOOLEAN, 0));
assertEquals("TRUE", stringConverter.getString("true", Types.BOOLEAN, Types.BOOLEAN, 0));
assertEquals("FALSE", stringConverter.getString(false, Types.BOOLEAN, Types.BOOLEAN, 0));
assertEquals("FALSE", stringConverter.getString("false", Types.BOOLEAN, Types.BOOLEAN, 0));
assertEquals("true", stringConverter.getString(true, Types.BOOLEAN, Types.BOOLEAN, 0));
assertEquals("true", stringConverter.getString("true", Types.BOOLEAN, Types.BOOLEAN, 0));
assertEquals("false", stringConverter.getString(false, Types.BOOLEAN, Types.BOOLEAN, 0));
assertEquals("false", stringConverter.getString("false", Types.BOOLEAN, Types.BOOLEAN, 0));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1501,12 +1501,12 @@ public void testBoolean() throws SQLException {
ResultSet rs = statement.executeQuery("select * from " + table)) {
assertTrue(rs.next());
assertTrue(rs.getBoolean(1));
assertEquals("TRUE", rs.getString(1));
assertEquals("true", rs.getString(1));
assertTrue(rs.next());
assertFalse(rs.getBoolean(1));
assertTrue(rs.next());
assertFalse(rs.getBoolean(1));
assertEquals("FALSE", rs.getString(1));
assertEquals("false", rs.getString(1));
assertFalse(rs.next());
statement.execute("drop table if exists " + table);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1176,9 +1176,9 @@ public void testGetObjectWithType() throws SQLException {
assertResultValueAndType(statement, Double.valueOf("1.1"), "f", Double.class);
assertResultValueAndType(statement, Double.valueOf("2.2"), "d", Double.class);
assertResultValueAndType(statement, BigDecimal.valueOf(3.3), "bd", BigDecimal.class);
assertResultValueAndType(statement, "FALSE", "bool", String.class);
assertResultValueAndType(statement, "false", "bool", String.class);
assertResultValueAndType(statement, Boolean.FALSE, "bool", Boolean.class);
assertResultValueAndType(statement, Long.valueOf(0), "bool", Long.class);
assertResultValueAndType(statement, 0L, "bool", Long.class);
assertResultValueAsString(
statement,
new SnowflakeTimestampWithTimezone(
Expand Down
Loading

0 comments on commit 9e221ea

Please sign in to comment.