From 3fee6f9240599049c0d179606236666b79f3bd10 Mon Sep 17 00:00:00 2001 From: John Yun <140559986+sfc-gh-ext-simba-jy@users.noreply.github.com> Date: Tue, 2 Jul 2024 16:02:32 +0900 Subject: [PATCH] SNOW-1196082: FIx inserting and reading timestamps not symetric if too much columns inserted with batch (#1794) --- .../client/core/bind/BindUploader.java | 6 +- .../client/jdbc/BindingDataLatestIT.java | 361 ++++++++++++------ 2 files changed, 249 insertions(+), 118 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/bind/BindUploader.java b/src/main/java/net/snowflake/client/core/bind/BindUploader.java index 2332f0150..6b901da44 100644 --- a/src/main/java/net/snowflake/client/core/bind/BindUploader.java +++ b/src/main/java/net/snowflake/client/core/bind/BindUploader.java @@ -159,6 +159,7 @@ private synchronized String synchronizedTimestampFormat(String o, String type) { int nano = times.right; Timestamp v1 = new Timestamp(sec * 1000); + ZoneOffset offsetId; // For timestamp_ntz, use UTC timezone. For timestamp_ltz, use the local timezone to minimise // the gap. if ("TIMESTAMP_LTZ".equals(type)) { @@ -166,10 +167,11 @@ private synchronized String synchronizedTimestampFormat(String o, String type) { cal.setTimeZone(tz); cal.clear(); timestampFormat.setCalendar(cal); + offsetId = ZoneId.systemDefault().getRules().getOffset(Instant.ofEpochMilli(v1.getTime())); + } else { + offsetId = ZoneOffset.UTC; } - ZoneOffset offsetId = ZoneId.systemDefault().getRules().getOffset(Instant.now()); - return timestampFormat.format(v1) + String.format("%09d", nano) + " " + offsetId; } diff --git a/src/test/java/net/snowflake/client/jdbc/BindingDataLatestIT.java b/src/test/java/net/snowflake/client/jdbc/BindingDataLatestIT.java index 257759120..71c556686 100644 --- a/src/test/java/net/snowflake/client/jdbc/BindingDataLatestIT.java +++ b/src/test/java/net/snowflake/client/jdbc/BindingDataLatestIT.java @@ -6,6 +6,7 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.sql.Connection; import java.sql.PreparedStatement; @@ -30,26 +31,34 @@ */ @Category(TestCategoryOthers.class) public class BindingDataLatestIT extends AbstractDriverIT { + TimeZone origTz = TimeZone.getDefault(); + TimeZone tokyoTz = TimeZone.getTimeZone("Asia/Tokyo"); + TimeZone australiaTz = TimeZone.getTimeZone("Australia/Sydney"); + Calendar tokyo = Calendar.getInstance(tokyoTz); + @Test public void testBindTimestampTZ() throws SQLException { - Connection connection = getConnection(); - Statement statement = connection.createStatement(); - statement.execute( - "create or replace table testBindTimestampTZ(" + "cola int, colb timestamp_tz)"); - statement.execute("alter session set CLIENT_TIMESTAMP_TYPE_MAPPING=TIMESTAMP_TZ"); - - long millSeconds = System.currentTimeMillis(); - Timestamp ts = new Timestamp(millSeconds); - PreparedStatement prepStatement = - connection.prepareStatement("insert into testBindTimestampTZ values (?, ?)"); - prepStatement.setInt(1, 123); - prepStatement.setTimestamp(2, ts, Calendar.getInstance(TimeZone.getTimeZone("EST"))); - prepStatement.execute(); - - ResultSet resultSet = statement.executeQuery("select cola, colb from testBindTimestampTz"); - resultSet.next(); - assertThat("integer", resultSet.getInt(1), equalTo(123)); - assertThat("timestamp_tz", resultSet.getTimestamp(2), equalTo(ts)); + try (Connection connection = getConnection(); + Statement statement = connection.createStatement()) { + statement.execute("create or replace table testBindTimestampTZ(cola int, colb timestamp_tz)"); + statement.execute("alter session set CLIENT_TIMESTAMP_TYPE_MAPPING=TIMESTAMP_TZ"); + + long milliSeconds = System.currentTimeMillis(); + Timestamp ts = new Timestamp(milliSeconds); + try (PreparedStatement prepStatement = + connection.prepareStatement("insert into testBindTimestampTZ values (?, ?)")) { + prepStatement.setInt(1, 123); + prepStatement.setTimestamp(2, ts, Calendar.getInstance(TimeZone.getTimeZone("EST"))); + prepStatement.execute(); + } + + try (ResultSet resultSet = + statement.executeQuery("select cola, colb from testBindTimestampTz")) { + assertTrue(resultSet.next()); + assertThat("integer", resultSet.getInt(1), equalTo(123)); + assertThat("timestamp_tz", resultSet.getTimestamp(2), equalTo(ts)); + } + } } /** @@ -60,57 +69,52 @@ public void testBindTimestampTZ() throws SQLException { @Test @ConditionalIgnoreRule.ConditionalIgnore(condition = RunningOnGithubAction.class) public void testTimestampBindingWithNTZType() throws SQLException { - try (Connection connection = getConnection()) { - TimeZone origTz = TimeZone.getDefault(); - Statement statement = connection.createStatement(); - statement.execute( - "create or replace table stageinsert(ind int, ltz0 timestamp_ltz, tz0 timestamp_tz, ntz0 timestamp_ntz)"); - statement.execute( - "create or replace table regularinsert(ind int, ltz0 timestamp_ltz, tz0 timestamp_tz, ntz0 timestamp_ntz)"); - statement.execute("alter session set CLIENT_TIMESTAMP_TYPE_MAPPING=TIMESTAMP_NTZ"); - statement.execute("alter session set TIMEZONE='Asia/Tokyo'"); - TimeZone.setDefault(TimeZone.getTimeZone("Asia/Tokyo")); - Timestamp currT = new Timestamp(System.currentTimeMillis()); - - // insert using stage binding - PreparedStatement prepStatement = - connection.prepareStatement("insert into stageinsert values (?,?,?,?)"); - statement.execute("ALTER SESSION SET CLIENT_STAGE_ARRAY_BINDING_THRESHOLD = 1"); - prepStatement.setInt(1, 1); - prepStatement.setTimestamp(2, currT); - prepStatement.setTimestamp(3, currT); - prepStatement.setTimestamp(4, currT); - prepStatement.addBatch(); - prepStatement.executeBatch(); - statement.execute("ALTER SESSION SET CLIENT_STAGE_ARRAY_BINDING_THRESHOLD = 0"); - - // insert using regular binging - prepStatement = connection.prepareStatement("insert into regularinsert values (?,?,?,?)"); - for (int i = 1; i <= 6; i++) { - prepStatement.setInt(1, 1); - prepStatement.setTimestamp(2, currT); - prepStatement.setTimestamp(3, currT); - prepStatement.setTimestamp(4, currT); - prepStatement.addBatch(); - } - prepStatement.executeBatch(); + TimeZone.setDefault(tokyoTz); + try (Connection connection = getConnection(); + Statement statement = connection.createStatement()) { + try { + statement.execute( + "create or replace table stageinsert(ind int, ltz0 timestamp_ltz, tz0 timestamp_tz, ntz0 timestamp_ntz)"); + statement.execute( + "create or replace table regularinsert(ind int, ltz0 timestamp_ltz, tz0 timestamp_tz, ntz0 timestamp_ntz)"); + statement.execute("alter session set CLIENT_TIMESTAMP_TYPE_MAPPING=TIMESTAMP_NTZ"); + statement.execute("alter session set TIMEZONE='Asia/Tokyo'"); + Timestamp currT = new Timestamp(System.currentTimeMillis()); + + // insert using regular binging + try (PreparedStatement prepStatement = + connection.prepareStatement("insert into regularinsert values (?,?,?,?)")) { + prepStatement.setInt(1, 1); + prepStatement.setTimestamp(2, currT, tokyo); + prepStatement.setTimestamp(3, currT, tokyo); + prepStatement.setTimestamp(4, currT); + prepStatement.addBatch(); + prepStatement.executeBatch(); + } + // insert using stage binding + statement.execute("ALTER SESSION SET CLIENT_STAGE_ARRAY_BINDING_THRESHOLD = 1"); + executePsStatementForTimestampTest(connection, "stageinsert", currT); + + // Compare the results + try (ResultSet rs1 = statement.executeQuery("select * from stageinsert"); + ResultSet rs2 = statement.executeQuery("select * from regularinsert")) { + assertTrue(rs1.next()); + assertTrue(rs2.next()); + + assertEquals(rs1.getInt(1), rs2.getInt(1)); - // Compare the results - ResultSet rs1 = statement.executeQuery("select * from stageinsert"); - ResultSet rs2 = statement.executeQuery("select * from regularinsert"); - rs1.next(); - rs2.next(); - - assertEquals(rs1.getInt(1), rs2.getInt(1)); - assertEquals(rs1.getString(2), rs2.getString(2)); - assertEquals(rs1.getString(3), rs2.getString(3)); - assertEquals(rs1.getString(4), rs2.getString(4)); - - statement.execute("drop table if exists stageinsert"); - statement.execute("drop table if exists regularinsert"); - TimeZone.setDefault(origTz); - statement.close(); - prepStatement.close(); + // Check tz type and ltz type columns have the same value. + assertEquals(rs1.getTimestamp(2), rs1.getTimestamp(3)); + + assertEquals(rs1.getTimestamp(2), rs2.getTimestamp(2)); + assertEquals(rs1.getTimestamp(3), rs2.getTimestamp(3)); + assertEquals(rs1.getTimestamp(4), rs2.getTimestamp(4)); + } + } finally { + statement.execute("drop table if exists stageinsert"); + statement.execute("drop table if exists regularinsert"); + TimeZone.setDefault(origTz); + } } } @@ -122,57 +126,182 @@ public void testTimestampBindingWithNTZType() throws SQLException { @Test @ConditionalIgnoreRule.ConditionalIgnore(condition = RunningOnGithubAction.class) public void testTimestampBindingWithLTZType() throws SQLException { - try (Connection connection = getConnection()) { - TimeZone origTz = TimeZone.getDefault(); - Statement statement = connection.createStatement(); - statement.execute( - "create or replace table stageinsert(ind int, ltz0 timestamp_ltz, tz0 timestamp_tz, ntz0 timestamp_ntz)"); - statement.execute( - "create or replace table regularinsert(ind int, ltz0 timestamp_ltz, tz0 timestamp_tz, ntz0 timestamp_ntz)"); - statement.execute("alter session set CLIENT_TIMESTAMP_TYPE_MAPPING=TIMESTAMP_LTZ"); - statement.execute("alter session set TIMEZONE='Asia/Tokyo'"); - TimeZone.setDefault(TimeZone.getTimeZone("Asia/Tokyo")); - Timestamp currT = new Timestamp(System.currentTimeMillis()); - - // insert using stage binding - PreparedStatement prepStatement = - connection.prepareStatement("insert into stageinsert values (?,?,?,?)"); - statement.execute("ALTER SESSION SET CLIENT_STAGE_ARRAY_BINDING_THRESHOLD = 1"); + TimeZone.setDefault(tokyoTz); + try (Connection connection = getConnection(); + Statement statement = connection.createStatement()) { + try { + statement.execute( + "create or replace table stageinsert(ind int, ltz0 timestamp_ltz, tz0 timestamp_tz, ntz0 timestamp_ntz)"); + statement.execute( + "create or replace table regularinsert(ind int, ltz0 timestamp_ltz, tz0 timestamp_tz, ntz0 timestamp_ntz)"); + statement.execute("alter session set CLIENT_TIMESTAMP_TYPE_MAPPING=TIMESTAMP_LTZ"); + statement.execute("alter session set TIMEZONE='Asia/Tokyo'"); + Timestamp currT = new Timestamp(System.currentTimeMillis()); + + // insert using regular binging + executePsStatementForTimestampTest(connection, "regularinsert", currT); + + // insert using stage binding + statement.execute("ALTER SESSION SET CLIENT_STAGE_ARRAY_BINDING_THRESHOLD = 1"); + executePsStatementForTimestampTest(connection, "stageinsert", currT); + + // Compare the results + try (ResultSet rs1 = statement.executeQuery("select * from stageinsert"); + ResultSet rs2 = statement.executeQuery("select * from regularinsert")) { + assertTrue(rs1.next()); + assertTrue(rs2.next()); + + assertEquals(rs1.getInt(1), rs2.getInt(1)); + + // Check that all the values are the same. + assertEquals(rs1.getTimestamp(2), rs1.getTimestamp(3)); + assertEquals(rs1.getTimestamp(3), rs1.getTimestamp(4)); + + assertEquals(rs1.getTimestamp(2), rs2.getTimestamp(2)); + assertEquals(rs1.getTimestamp(3), rs2.getTimestamp(3)); + assertEquals(rs1.getTimestamp(4), rs2.getTimestamp(4)); + } + } finally { + statement.execute("drop table if exists stageinsert"); + statement.execute("drop table if exists regularinsert"); + TimeZone.setDefault(origTz); + } + } + } + + /** + * Test that stage binding and regular binding insert and return the same value for timestamp_ltz + * when the local timezone has the daylight saving. This test is added in version > 3.16.1 + * + *

When CLIENT_TIMESTAMP_TYPE_MAPPING setting is mismatched with target data type (e.g + * MAPPING=LTZ and insert to NTZ or MAPPING=NTZ and insert to TZ/LTZ there could be different + * result as the timezone offset is applied on client side and removed on server side. This only + * occurs around the boundary of daylight-savings and the difference from the source data would be + * one hour. Both regular binding and stage binding have such issue but they also behave + * diffently, for some data only regular binding gets the extra hour while sometime only stage + * binding does. The workaround is to use CLIENT_TIMESTAMP_TYPE_MAPPING=LTZ to insert LTZ/TZ data + * and use CLIENT_TIMESTAMP_TYPE_MAPPING=NTZ to insert NTZ data. + * + *

This test cannot run on the GitHub testing because of the "ALTER SESSION SET + * CLIENT_STAGE_ARRAY_BINDING_THRESHOLD" This command should be executed with the system admin. + * + * @throws SQLException + */ + @Test + @ConditionalIgnoreRule.ConditionalIgnore(condition = RunningOnGithubAction.class) + public void testTimestampBindingWithLTZTypeForDayLightSavingTimeZone() throws SQLException { + Calendar australia = Calendar.getInstance(australiaTz); + TimeZone.setDefault(australiaTz); + try (Connection connection = getConnection(); + Statement statement = connection.createStatement()) { + try { + statement.execute( + "create or replace table stageinsert(ind int, ltz0 timestamp_ltz, ltz1 timestamp_ltz, ltz2 timestamp_ltz, tz0 timestamp_tz, tz1 timestamp_tz, tz2 timestamp_tz, ntz0 timestamp_ntz, ntz1 timestamp_ntz, ntz2 timestamp_ntz)"); + statement.execute( + "create or replace table regularinsert(ind int, ltz0 timestamp_ltz, ltz1 timestamp_ltz, ltz2 timestamp_ltz, tz0 timestamp_tz, tz1 timestamp_tz, tz2 timestamp_tz, ntz0 timestamp_ntz, ntz1 timestamp_ntz, ntz2 timestamp_ntz)"); + statement.execute("alter session set CLIENT_TIMESTAMP_TYPE_MAPPING=TIMESTAMP_LTZ"); + statement.execute("alter session set TIMEZONE='UTC'"); + + Timestamp ts1 = new Timestamp(1403049600000L); + Timestamp ts2 = new Timestamp(1388016000000L); + Timestamp ts3 = new Timestamp(System.currentTimeMillis()); + + // insert using regular binging + try (PreparedStatement prepStatement = + connection.prepareStatement("insert into regularinsert values (?,?,?,?,?,?,?,?,?,?)")) { + prepStatement.setInt(1, 1); + prepStatement.setTimestamp(2, ts1); + prepStatement.setTimestamp(3, ts2); + prepStatement.setTimestamp(4, ts3); + + prepStatement.setTimestamp(5, ts1); + prepStatement.setTimestamp(6, ts2); + prepStatement.setTimestamp(7, ts3); + + prepStatement.setTimestamp(8, ts1, australia); + prepStatement.setTimestamp(9, ts2, australia); + prepStatement.setTimestamp(10, ts3, australia); + + prepStatement.addBatch(); + prepStatement.executeBatch(); + } + + // insert using stage binding + statement.execute("ALTER SESSION SET CLIENT_STAGE_ARRAY_BINDING_THRESHOLD = 1"); + try (PreparedStatement prepStatement = + connection.prepareStatement("insert into stageinsert values (?,?,?,?,?,?,?,?,?,?)")) { + prepStatement.setInt(1, 1); + prepStatement.setTimestamp(2, ts1); + prepStatement.setTimestamp(3, ts2); + prepStatement.setTimestamp(4, ts3); + + prepStatement.setTimestamp(5, ts1); + prepStatement.setTimestamp(6, ts2); + prepStatement.setTimestamp(7, ts3); + + prepStatement.setTimestamp(8, ts1); + prepStatement.setTimestamp(9, ts2); + prepStatement.setTimestamp(10, ts3); + + prepStatement.addBatch(); + prepStatement.executeBatch(); + } + + // Compare the results + try (ResultSet rs1 = statement.executeQuery("select * from stageinsert"); + ResultSet rs2 = statement.executeQuery("select * from regularinsert")) { + assertTrue(rs1.next()); + assertTrue(rs2.next()); + + assertEquals(rs1.getInt(1), rs2.getInt(1)); + assertEquals(rs1.getTimestamp(2), rs2.getTimestamp(2)); + assertEquals(rs1.getTimestamp(3), rs2.getTimestamp(3)); + assertEquals(rs1.getTimestamp(4), rs2.getTimestamp(4)); + assertEquals(rs1.getTimestamp(5), rs2.getTimestamp(5)); + assertEquals(rs1.getTimestamp(6), rs2.getTimestamp(6)); + assertEquals(rs1.getTimestamp(7), rs2.getTimestamp(7)); + assertEquals(rs1.getTimestamp(8), rs2.getTimestamp(8)); + assertEquals(rs1.getTimestamp(9), rs2.getTimestamp(9)); + assertEquals(rs1.getTimestamp(10), rs2.getTimestamp(10)); + + assertEquals(ts1.getTime(), rs1.getTimestamp(2).getTime()); + assertEquals(ts2.getTime(), rs1.getTimestamp(3).getTime()); + assertEquals(ts3.getTime(), rs1.getTimestamp(4).getTime()); + assertEquals(ts1.getTime(), rs1.getTimestamp(5).getTime()); + assertEquals(ts2.getTime(), rs1.getTimestamp(6).getTime()); + assertEquals(ts3.getTime(), rs1.getTimestamp(7).getTime()); + assertEquals(ts1.getTime(), rs1.getTimestamp(8).getTime()); + assertEquals(ts2.getTime(), rs1.getTimestamp(9).getTime()); + assertEquals(ts3.getTime(), rs1.getTimestamp(10).getTime()); + + assertEquals(ts1.getTime(), rs2.getTimestamp(2).getTime()); + assertEquals(ts2.getTime(), rs2.getTimestamp(3).getTime()); + assertEquals(ts3.getTime(), rs2.getTimestamp(4).getTime()); + assertEquals(ts1.getTime(), rs2.getTimestamp(5).getTime()); + assertEquals(ts2.getTime(), rs2.getTimestamp(6).getTime()); + assertEquals(ts3.getTime(), rs2.getTimestamp(7).getTime()); + assertEquals(ts1.getTime(), rs2.getTimestamp(8).getTime()); + assertEquals(ts2.getTime(), rs2.getTimestamp(9).getTime()); + assertEquals(ts3.getTime(), rs2.getTimestamp(10).getTime()); + } + } finally { + statement.execute("drop table if exists stageinsert"); + statement.execute("drop table if exists regularinsert"); + TimeZone.setDefault(origTz); + } + } + } + + public void executePsStatementForTimestampTest( + Connection connection, String tableName, Timestamp timestamp) throws SQLException { + try (PreparedStatement prepStatement = + connection.prepareStatement("insert into " + tableName + " values (?,?,?,?)")) { prepStatement.setInt(1, 1); - prepStatement.setTimestamp(2, currT); - prepStatement.setTimestamp(3, currT); - prepStatement.setTimestamp(4, currT); + prepStatement.setTimestamp(2, timestamp); + prepStatement.setTimestamp(3, timestamp); + prepStatement.setTimestamp(4, timestamp); prepStatement.addBatch(); prepStatement.executeBatch(); - statement.execute("ALTER SESSION SET CLIENT_STAGE_ARRAY_BINDING_THRESHOLD = 0"); - - // insert using regular binging - prepStatement = connection.prepareStatement("insert into regularinsert values (?,?,?,?)"); - for (int i = 1; i <= 6; i++) { - prepStatement.setInt(1, 1); - prepStatement.setTimestamp(2, currT); - prepStatement.setTimestamp(3, currT); - prepStatement.setTimestamp(4, currT); - prepStatement.addBatch(); - } - prepStatement.executeBatch(); - - // Compare the results - ResultSet rs1 = statement.executeQuery("select * from stageinsert"); - ResultSet rs2 = statement.executeQuery("select * from regularinsert"); - rs1.next(); - rs2.next(); - - assertEquals(rs1.getInt(1), rs2.getInt(1)); - assertEquals(rs1.getString(2), rs2.getString(2)); - assertEquals(rs1.getString(3), rs2.getString(3)); - assertEquals(rs1.getString(4), rs2.getString(4)); - - statement.execute("drop table if exists stageinsert"); - statement.execute("drop table if exists regularinsert"); - TimeZone.setDefault(origTz); - statement.close(); - prepStatement.close(); } } }