Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNOW-1374896 unify structured types string representation #1882

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
d8faeba
Refactor init converters for reuse in other code
sfc-gh-astachowski Aug 22, 2024
8499a52
Formatting
sfc-gh-astachowski Aug 22, 2024
982b13e
Removed star imports
sfc-gh-astachowski Aug 22, 2024
c82e155
Documentation style fix
sfc-gh-astachowski Aug 22, 2024
7a8382b
Moved init converters to an already public class
sfc-gh-astachowski Aug 22, 2024
7aa57d1
Moved init converters to an appropriate interface
sfc-gh-astachowski Aug 22, 2024
7246575
Merge branch 'master' into init-converters-refactor
sfc-gh-astachowski Aug 22, 2024
48c2435
Changed the version of japicmp to include a needed bugfix
sfc-gh-astachowski Aug 22, 2024
6142094
Refactored unnecessary static modifiers
sfc-gh-astachowski Aug 22, 2024
a9ca5e6
Reformat
sfc-gh-astachowski Aug 22, 2024
e1c69e6
Update japicmp to the newest version
sfc-gh-astachowski Aug 26, 2024
d9701bc
Merge branch 'master' into init-converters-refactor
sfc-gh-astachowski Aug 30, 2024
c63d35a
Implement FieldVector-based toString for structured types
sfc-gh-mkubik Aug 30, 2024
777c7d0
fix formatting
sfc-gh-mkubik Sep 2, 2024
5ee4904
mark new classes as internal
sfc-gh-mkubik Sep 2, 2024
655df20
fix failing boolean test, remove integer from quotable types
sfc-gh-mkubik Sep 2, 2024
4d2eb63
Address review comments
sfc-gh-mkubik Sep 4, 2024
3a6dd28
Refactor init converters for reuse in other code
sfc-gh-astachowski Aug 22, 2024
7668307
Formatting
sfc-gh-astachowski Aug 22, 2024
9d41eb5
Removed star imports
sfc-gh-astachowski Aug 22, 2024
0c72b4b
Documentation style fix
sfc-gh-astachowski Aug 22, 2024
9c325b9
Moved init converters to an already public class
sfc-gh-astachowski Aug 22, 2024
e7397ee
Implement FieldVector-based toString for structured types
sfc-gh-mkubik Aug 30, 2024
ac7eb94
fix formatting
sfc-gh-mkubik Sep 2, 2024
6c6df45
mark new classes as internal
sfc-gh-mkubik Sep 2, 2024
c74f967
fix failing boolean test, remove integer from quotable types
sfc-gh-mkubik Sep 2, 2024
11be102
Address review comments
sfc-gh-mkubik Sep 4, 2024
277ffab
Resolve merge conflicts
sfc-gh-mkubik Sep 4, 2024
67f3668
remove unnecessary toString override from child builder classes
sfc-gh-mkubik Sep 4, 2024
9a2c931
remove unnecessary shouldQuote call
sfc-gh-mkubik Sep 4, 2024
2f96b68
use StringJoiner instead of StringBuilder inside ArrowStringRepresent…
sfc-gh-mkubik Sep 5, 2024
4f2bb5c
resolve merge conflicts
sfc-gh-mkubik Sep 20, 2024
9887f74
Fix usages of ArrowVectorConverter utils after moving to a separate c…
sfc-gh-mkubik Sep 20, 2024
542180d
fix formatting
sfc-gh-mkubik Sep 20, 2024
6e4d4b5
Merge branch 'master' of github.com:snowflakedb/snowflake-jdbc into S…
sfc-gh-mkubik Sep 26, 2024
31489f8
Add tests for AllTypesClass and NestedStructures
sfc-gh-mkubik Sep 26, 2024
581968d
uncomment timezone ntz assert that was disabled for local testing
sfc-gh-mkubik Sep 26, 2024
97ac99a
comment back the TODO comments from another tasks since they were bre…
sfc-gh-mkubik Sep 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
sfc-gh-mkubik marked this conversation as resolved.
Show resolved Hide resolved
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
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);
sfc-gh-mkubik marked this conversation as resolved.
Show resolved Hide resolved
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(
sfc-gh-mkubik marked this conversation as resolved.
Show resolved Hide resolved
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
Loading