From 104dc186428c199dde2c8becc453e17dc4b40a24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Kubik?= Date: Mon, 30 Sep 2024 15:26:47 +0200 Subject: [PATCH 1/3] revert turning native arrow boolean primitives lowercase to prevent introducing a breaking change. Structured types' booleans string representation will remain lowercase to preserve JSON-compatibility --- .../java/net/snowflake/client/core/ResultUtil.java | 2 +- .../client/core/arrow/BitToBooleanConverter.java | 2 +- .../ArrowStringRepresentationBuilderBase.java | 13 +++++++++++++ .../core/arrow/BitToBooleanConverterTest.java | 2 +- .../client/core/json/StringConverterTest.java | 8 ++++---- ...turedTypesGetStringArrowJsonCompatibilityIT.java | 3 +++ 6 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/ResultUtil.java b/src/main/java/net/snowflake/client/core/ResultUtil.java index 6c00f513e..b894f4259 100644 --- a/src/main/java/net/snowflake/client/core/ResultUtil.java +++ b/src/main/java/net/snowflake/client/core/ResultUtil.java @@ -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"; } /** diff --git a/src/main/java/net/snowflake/client/core/arrow/BitToBooleanConverter.java b/src/main/java/net/snowflake/client/core/arrow/BitToBooleanConverter.java index 640ff68ca..2f5a8cf83 100644 --- a/src/main/java/net/snowflake/client/core/arrow/BitToBooleanConverter.java +++ b/src/main/java/net/snowflake/client/core/arrow/BitToBooleanConverter.java @@ -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 diff --git a/src/main/java/net/snowflake/client/core/arrow/tostringhelpers/ArrowStringRepresentationBuilderBase.java b/src/main/java/net/snowflake/client/core/arrow/tostringhelpers/ArrowStringRepresentationBuilderBase.java index 5b83f4497..8a1b78a7b 100644 --- a/src/main/java/net/snowflake/client/core/arrow/tostringhelpers/ArrowStringRepresentationBuilderBase.java +++ b/src/main/java/net/snowflake/client/core/arrow/tostringhelpers/ArrowStringRepresentationBuilderBase.java @@ -6,6 +6,12 @@ import net.snowflake.client.core.SnowflakeJdbcInternalApi; import net.snowflake.client.jdbc.SnowflakeType; +/** + * StringBuilder like class to aggregate the string representation of snowflake + * Native ARROW structured types as JSON one-liners. + * Provides some additional snowflake-specific logic in order to determine whether the value should be + * quoted or case should be changed. + */ @SnowflakeJdbcInternalApi public abstract class ArrowStringRepresentationBuilderBase { private final StringJoiner joiner; @@ -39,6 +45,13 @@ private boolean shouldQuoteValue(SnowflakeType type) { } protected String quoteIfNeeded(String string, SnowflakeType type) { + // Turn Boolean string representations lowercase to make the output JSON-compatible + // this should be changed on the converter level, but it would be a breaking change thus + // for now only structured types will be valid JSONs while in NATIVE ARROW mode + if (type == SnowflakeType.BOOLEAN) { + string = string.toLowerCase(); + } + if (shouldQuoteValue(type)) { return '"' + string + '"'; } diff --git a/src/test/java/net/snowflake/client/core/arrow/BitToBooleanConverterTest.java b/src/test/java/net/snowflake/client/core/arrow/BitToBooleanConverterTest.java index 1fd65d911..e5091d6fc 100644 --- a/src/test/java/net/snowflake/client/core/arrow/BitToBooleanConverterTest.java +++ b/src/test/java/net/snowflake/client/core/arrow/BitToBooleanConverterTest.java @@ -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())); + assertThat(stringVal, is(expectedValues.get(i).toString().toUpperCase())); if (boolVal) { assertThat((byte) 0x1, is(converter.toBytes(i)[0])); } else { diff --git a/src/test/java/net/snowflake/client/core/json/StringConverterTest.java b/src/test/java/net/snowflake/client/core/json/StringConverterTest.java index 748548966..5fe3dd2cb 100644 --- a/src/test/java/net/snowflake/client/core/json/StringConverterTest.java +++ b/src/test/java/net/snowflake/client/core/json/StringConverterTest.java @@ -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 diff --git a/src/test/java/net/snowflake/client/jdbc/structuredtypes/StructuredTypesGetStringArrowJsonCompatibilityIT.java b/src/test/java/net/snowflake/client/jdbc/structuredtypes/StructuredTypesGetStringArrowJsonCompatibilityIT.java index a4bdb1194..bccf3743e 100644 --- a/src/test/java/net/snowflake/client/jdbc/structuredtypes/StructuredTypesGetStringArrowJsonCompatibilityIT.java +++ b/src/test/java/net/snowflake/client/jdbc/structuredtypes/StructuredTypesGetStringArrowJsonCompatibilityIT.java @@ -84,6 +84,9 @@ public static Collection data() { samples.put( "select [{'a':'a'}, {'b':'b'}]::array(map(string, string))", "[{\"a\":\"a\"}, {\"b\":\"b\"}]"); + samples.put( + "select [{'a':true}, {'b':false}]::array(map(string, boolean))", + "[{\"a\":true}, {\"b\":false}]"); samples.put( "select [{'string':'a'}, {'string':'b'}]::array(object(string varchar))", "[{\"string\":\"a\"}, {\"string\":\"b\"}]"); From 1e4eddec820ad8c076bc48794297e87fbc0be21b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Kubik?= Date: Mon, 30 Sep 2024 15:30:55 +0200 Subject: [PATCH 2/3] fix formatting --- .../ArrowStringRepresentationBuilderBase.java | 7 +++---- .../StructuredTypesGetStringArrowJsonCompatibilityIT.java | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/arrow/tostringhelpers/ArrowStringRepresentationBuilderBase.java b/src/main/java/net/snowflake/client/core/arrow/tostringhelpers/ArrowStringRepresentationBuilderBase.java index 8a1b78a7b..cc25bb7e0 100644 --- a/src/main/java/net/snowflake/client/core/arrow/tostringhelpers/ArrowStringRepresentationBuilderBase.java +++ b/src/main/java/net/snowflake/client/core/arrow/tostringhelpers/ArrowStringRepresentationBuilderBase.java @@ -7,10 +7,9 @@ import net.snowflake.client.jdbc.SnowflakeType; /** - * StringBuilder like class to aggregate the string representation of snowflake - * Native ARROW structured types as JSON one-liners. - * Provides some additional snowflake-specific logic in order to determine whether the value should be - * quoted or case should be changed. + * StringBuilder like class to aggregate the string representation of snowflake Native ARROW + * structured types as JSON one-liners. Provides some additional snowflake-specific logic in order + * to determine whether the value should be quoted or case should be changed. */ @SnowflakeJdbcInternalApi public abstract class ArrowStringRepresentationBuilderBase { diff --git a/src/test/java/net/snowflake/client/jdbc/structuredtypes/StructuredTypesGetStringArrowJsonCompatibilityIT.java b/src/test/java/net/snowflake/client/jdbc/structuredtypes/StructuredTypesGetStringArrowJsonCompatibilityIT.java index bccf3743e..352d2b1a4 100644 --- a/src/test/java/net/snowflake/client/jdbc/structuredtypes/StructuredTypesGetStringArrowJsonCompatibilityIT.java +++ b/src/test/java/net/snowflake/client/jdbc/structuredtypes/StructuredTypesGetStringArrowJsonCompatibilityIT.java @@ -85,8 +85,8 @@ public static Collection data() { "select [{'a':'a'}, {'b':'b'}]::array(map(string, string))", "[{\"a\":\"a\"}, {\"b\":\"b\"}]"); samples.put( - "select [{'a':true}, {'b':false}]::array(map(string, boolean))", - "[{\"a\":true}, {\"b\":false}]"); + "select [{'a':true}, {'b':false}]::array(map(string, boolean))", + "[{\"a\":true}, {\"b\":false}]"); samples.put( "select [{'string':'a'}, {'string':'b'}]::array(object(string varchar))", "[{\"string\":\"a\"}, {\"string\":\"b\"}]"); From 16403600380b4118dd5a91863ec1bf3e6440f98d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Kubik?= Date: Mon, 30 Sep 2024 16:24:24 +0200 Subject: [PATCH 3/3] fix tests ommited in previous commit --- .../net/snowflake/client/jdbc/ResultSetJsonVsArrowIT.java | 4 ++-- .../java/net/snowflake/client/jdbc/ResultSetLatestIT.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/net/snowflake/client/jdbc/ResultSetJsonVsArrowIT.java b/src/test/java/net/snowflake/client/jdbc/ResultSetJsonVsArrowIT.java index 0dd8edd47..65cc27242 100644 --- a/src/test/java/net/snowflake/client/jdbc/ResultSetJsonVsArrowIT.java +++ b/src/test/java/net/snowflake/client/jdbc/ResultSetJsonVsArrowIT.java @@ -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); } diff --git a/src/test/java/net/snowflake/client/jdbc/ResultSetLatestIT.java b/src/test/java/net/snowflake/client/jdbc/ResultSetLatestIT.java index 3ad105cea..dc16d5dcf 100644 --- a/src/test/java/net/snowflake/client/jdbc/ResultSetLatestIT.java +++ b/src/test/java/net/snowflake/client/jdbc/ResultSetLatestIT.java @@ -1176,7 +1176,7 @@ 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, 0L, "bool", Long.class); assertResultValueAsString(