From e4ac59787aad85e13a7805ecda8cecaaeac52bb6 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 26 Aug 2024 11:34:01 -0700 Subject: [PATCH 1/3] Fix SQL Server timestamp handling with non-US date formats --- .../src/org/labkey/bigiron/BigIronModule.java | 2 + .../mssql/MicrosoftSqlServer2016Dialect.java | 173 +++++++++++++----- 2 files changed, 127 insertions(+), 48 deletions(-) diff --git a/bigiron/src/org/labkey/bigiron/BigIronModule.java b/bigiron/src/org/labkey/bigiron/BigIronModule.java index 3e1a46202e6..0e91fa5e989 100644 --- a/bigiron/src/org/labkey/bigiron/BigIronModule.java +++ b/bigiron/src/org/labkey/bigiron/BigIronModule.java @@ -24,6 +24,7 @@ import org.labkey.api.module.ModuleContext; import org.labkey.api.view.WebPartFactory; import org.labkey.bigiron.mssql.GroupConcatInstallationManager; +import org.labkey.bigiron.mssql.MicrosoftSqlServer2016Dialect; import org.labkey.bigiron.mssql.MicrosoftSqlServerDialectFactory; import org.labkey.bigiron.mssql.MicrosoftSqlServerVersion; import org.labkey.bigiron.mssql.synonym.SynonymTestCase; @@ -87,6 +88,7 @@ public Set getIntegrationTests() { return Set.of( GroupConcatInstallationManager.TestCase.class, + MicrosoftSqlServer2016Dialect.TestCase.class, MicrosoftSqlServerVersion.TestCase.class, OracleVersion.TestCase.class, SynonymTestCase.class diff --git a/bigiron/src/org/labkey/bigiron/mssql/MicrosoftSqlServer2016Dialect.java b/bigiron/src/org/labkey/bigiron/mssql/MicrosoftSqlServer2016Dialect.java index 5b6f7cc53a8..1d9d7905d46 100644 --- a/bigiron/src/org/labkey/bigiron/mssql/MicrosoftSqlServer2016Dialect.java +++ b/bigiron/src/org/labkey/bigiron/mssql/MicrosoftSqlServer2016Dialect.java @@ -15,21 +15,54 @@ */ package org.labkey.bigiron.mssql; +import org.apache.logging.log4j.Logger; +import org.junit.Assert; +import org.junit.Test; import org.labkey.api.data.ConnectionWrapper; +import org.labkey.api.data.DbScope; +import org.labkey.api.data.SqlSelector; import org.labkey.api.data.dialect.StatementWrapper; +import org.labkey.api.module.ModuleLoader; +import org.labkey.api.util.logging.LogHelper; +import java.sql.CallableStatement; +import java.sql.Connection; +import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Statement; import java.sql.Timestamp; +import java.sql.Types; +import java.time.ZoneId; +import java.time.format.DateTimeFormatter; import java.util.Calendar; +import java.util.Date; +import java.util.Map; -/** - * User: adam - * Date: 8/11/2015 - * Time: 1:19 PM - */ public class MicrosoftSqlServer2016Dialect extends MicrosoftSqlServer2014Dialect { + private static final Logger LOG = LogHelper.getLogger(MicrosoftSqlServer2016Dialect.class, "SQL Server settings"); + + private volatile String _language = null; + private volatile String _dateFormat = null; + private volatile DateTimeFormatter _timestampFormatter = null; + + @Override + public void prepare(DbScope scope) + { + super.prepare(scope); + + Map map = new SqlSelector(scope, "SELECT language, date_format FROM sys.dm_exec_sessions WHERE session_id = @@spid").getMap(); + _language = (String) map.get("language"); + _dateFormat = (String) map.get("date_format"); + + // This seems to be the only string format acceptable for sending Timestamps, but unfortunately it's ambiguous; + // SQL Server interprets the "MM-dd" portion based on the database's regional settings. So we must query the + // current date format and switch the formatter pattern based on what we find. See Issue 51129. + _timestampFormatter = DateTimeFormatter.ofPattern("yyyy-" + ("mdy".equals(_dateFormat) ? "MM-dd" : "dd-MM") + " HH:mm:ss.SSS"); + + LOG.info("\n Language: {}\n DateFormat: {}", _language, _dateFormat); + } + @Override public StatementWrapper getStatementWrapper(ConnectionWrapper conn, Statement stmt) { @@ -44,17 +77,15 @@ public StatementWrapper getStatementWrapper(ConnectionWrapper conn, Statement st /** * Per the SQL Server JDBC driver docs at ... - * - * Note that java.sql.Timestamp values can no longer be used to compare values from a datetime column starting + * "Note that java.sql.Timestamp values can no longer be used to compare values from a datetime column starting * from SQL Server 2016. This limitation is due to a server-side change that converts datetime to datetime2 * differently, resulting in non-equitable values. The workaround to this issue is to either change datetime * columns to datetime2(3), use String instead of java.sql.Timestamp, or change database compatibility level - * to 120 or below. - * - * - * java.sql.Timestamp.toString() includes the nanos in a ISO 8061-like format + * to 120 or below." We can't change column types in external schemas, and we don't want a low compatibility level, + * so we send Timestamps as Strings. SQL Server is very picky about this format; for example, Timestamp.toString(), + * which is basically ISO, is actually ambiguous and fails if language is French (e.g.). See Issue 51129. */ - private static class TimestampStatementWrapper extends StatementWrapper + private class TimestampStatementWrapper extends StatementWrapper { public TimestampStatementWrapper(ConnectionWrapper conn, Statement stmt) { @@ -67,12 +98,11 @@ public TimestampStatementWrapper(ConnectionWrapper conn, Statement stmt, String } @Override - public void setTimestamp(String parameterName, Timestamp x) - throws SQLException + public void setTimestamp(String parameterName, Timestamp x) throws SQLException { if (x != null) { - setObject(parameterName, x.toString()); + setObject(parameterName, convert(x)); } else { @@ -81,12 +111,11 @@ public void setTimestamp(String parameterName, Timestamp x) } @Override - public void setTimestamp(String parameterName, Timestamp x, Calendar cal) - throws SQLException + public void setTimestamp(String parameterName, Timestamp x, Calendar cal) throws SQLException { if (x != null) { - setObject(parameterName, x.toString()); + setObject(parameterName, convert(x)); } else { @@ -95,12 +124,11 @@ public void setTimestamp(String parameterName, Timestamp x, Calendar cal) } @Override - public void setTimestamp(int parameterIndex, Timestamp x) - throws SQLException + public void setTimestamp(int parameterIndex, Timestamp x) throws SQLException { if (x != null) { - setObject(parameterIndex, x.toString()); + setObject(parameterIndex, convert(x)); } else { @@ -109,12 +137,11 @@ public void setTimestamp(int parameterIndex, Timestamp x) } @Override - public void setTimestamp(int parameterIndex, Timestamp x, Calendar cal) - throws SQLException + public void setTimestamp(int parameterIndex, Timestamp x, Calendar cal) throws SQLException { if (x != null) { - setObject(parameterIndex, x.toString()); + setObject(parameterIndex, convert(x)); } else { @@ -123,51 +150,101 @@ public void setTimestamp(int parameterIndex, Timestamp x, Calendar cal) } @Override - public void setObject(int parameterIndex, Object x, int targetSqlType, int scale) - throws SQLException + public void setObject(int parameterIndex, Object x, int targetSqlType, int scale) throws SQLException { - x = x instanceof Timestamp ? x.toString() : x; - super.setObject(parameterIndex, x, targetSqlType, scale); + if (targetSqlType == Types.TIMESTAMP) + setObject(parameterIndex, x); + else + super.setObject(parameterIndex, x, targetSqlType, scale); } @Override - public void setObject(int parameterIndex, Object x, int targetSqlType) - throws SQLException + public void setObject(int parameterIndex, Object x, int targetSqlType) throws SQLException { - x = x instanceof Timestamp ? x.toString() : x; - super.setObject(parameterIndex, x, targetSqlType); + if (targetSqlType == Types.TIMESTAMP) + setObject(parameterIndex, x); + else + super.setObject(parameterIndex, x, targetSqlType); } @Override - public void setObject(int parameterIndex, Object x) - throws SQLException + public void setObject(int parameterIndex, Object x) throws SQLException { - x = x instanceof Timestamp ? x.toString() : x; - super.setObject(parameterIndex, x); + super.setObject(parameterIndex, convert(x)); } @Override - public void setObject(String parameterName, Object x, int targetSqlType, int scale) - throws SQLException + public void setObject(String parameterName, Object x, int targetSqlType, int scale) throws SQLException { - x = x instanceof Timestamp ? x.toString() : x; - super.setObject(parameterName, x, targetSqlType, scale); + if (targetSqlType == Types.TIMESTAMP) + setObject(parameterName, x); + else + super.setObject(parameterName, x, targetSqlType, scale); } @Override - public void setObject(String parameterName, Object x, int targetSqlType) - throws SQLException + public void setObject(String parameterName, Object x, int targetSqlType) throws SQLException { - x = x instanceof Timestamp ? x.toString() : x; - super.setObject(parameterName, x, targetSqlType); + if (targetSqlType == Types.TIMESTAMP) + setObject(parameterName, x); + else + super.setObject(parameterName, x, targetSqlType); } @Override - public void setObject(String parameterName, Object x) - throws SQLException + public void setObject(String parameterName, Object x) throws SQLException { - x = x instanceof Timestamp ? x.toString() : x; - super.setObject(parameterName, x); + super.setObject(parameterName, convert(x)); + } + + private Object convert(Object x) + { + return x instanceof Timestamp ts ? convert(ts) : x; + } + + private String convert(Timestamp ts) + { + return _timestampFormatter.format(ts.toInstant().atZone(ZoneId.systemDefault())); + } + } + + public static class TestCase + { + @Test + public void testTimestamps() + { + try (Connection conn = DbScope.getLabKeyScope().getConnection()) + { + Timestamp ts = new Timestamp(new Date().getTime()); + Calendar cal = Calendar.getInstance(); + + try (PreparedStatement statement = conn.prepareStatement("SELECT ?")) + { + Assert.assertTrue(statement instanceof TimestampStatementWrapper); + statement.setTimestamp(1, ts); + statement.setTimestamp(1, ts, cal); + statement.setObject(1, ts, Types.TIMESTAMP, 0); + statement.setObject(1, ts, Types.TIMESTAMP); + statement.setObject(1, ts); + } + + if (ModuleLoader.getInstance().hasModule("DataIntegration")) + { + try (CallableStatement statement = conn.prepareCall("{call etltest.etlTest(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)}")) + { + Assert.assertTrue(statement instanceof TimestampStatementWrapper); + statement.setTimestamp("filterStartTimeStamp", ts); + statement.setTimestamp("filterStartTimeStamp", ts, cal); + statement.setObject("filterStartTimeStamp", ts, Types.TIMESTAMP, 0); + statement.setObject("filterStartTimeStamp", ts, Types.TIMESTAMP); + statement.setObject("filterStartTimeStamp", ts); + } + } + } + catch (SQLException e) + { + throw new RuntimeException(e); + } } } } From 77f9b6fe62ee5d5068b83b7b40749600036333de Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 26 Aug 2024 13:19:13 -0700 Subject: [PATCH 2/3] Probably should run this on SQL Server 2016+ only --- .../mssql/MicrosoftSqlServer2016Dialect.java | 55 +++++++++++-------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/bigiron/src/org/labkey/bigiron/mssql/MicrosoftSqlServer2016Dialect.java b/bigiron/src/org/labkey/bigiron/mssql/MicrosoftSqlServer2016Dialect.java index 1d9d7905d46..974c3bc8642 100644 --- a/bigiron/src/org/labkey/bigiron/mssql/MicrosoftSqlServer2016Dialect.java +++ b/bigiron/src/org/labkey/bigiron/mssql/MicrosoftSqlServer2016Dialect.java @@ -21,6 +21,7 @@ import org.labkey.api.data.ConnectionWrapper; import org.labkey.api.data.DbScope; import org.labkey.api.data.SqlSelector; +import org.labkey.api.data.dialect.SqlDialect; import org.labkey.api.data.dialect.StatementWrapper; import org.labkey.api.module.ModuleLoader; import org.labkey.api.util.logging.LogHelper; @@ -213,37 +214,43 @@ public static class TestCase @Test public void testTimestamps() { - try (Connection conn = DbScope.getLabKeyScope().getConnection()) - { - Timestamp ts = new Timestamp(new Date().getTime()); - Calendar cal = Calendar.getInstance(); + DbScope scope = DbScope.getLabKeyScope(); + SqlDialect dialect = scope.getSqlDialect(); - try (PreparedStatement statement = conn.prepareStatement("SELECT ?")) + if (dialect.isSqlServer() && dialect instanceof MicrosoftSqlServer2016Dialect) + { + try (Connection conn = DbScope.getLabKeyScope().getConnection()) { - Assert.assertTrue(statement instanceof TimestampStatementWrapper); - statement.setTimestamp(1, ts); - statement.setTimestamp(1, ts, cal); - statement.setObject(1, ts, Types.TIMESTAMP, 0); - statement.setObject(1, ts, Types.TIMESTAMP); - statement.setObject(1, ts); - } + Timestamp ts = new Timestamp(new Date().getTime()); + Calendar cal = Calendar.getInstance(); - if (ModuleLoader.getInstance().hasModule("DataIntegration")) - { - try (CallableStatement statement = conn.prepareCall("{call etltest.etlTest(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)}")) + try (PreparedStatement statement = conn.prepareStatement("SELECT ?")) { Assert.assertTrue(statement instanceof TimestampStatementWrapper); - statement.setTimestamp("filterStartTimeStamp", ts); - statement.setTimestamp("filterStartTimeStamp", ts, cal); - statement.setObject("filterStartTimeStamp", ts, Types.TIMESTAMP, 0); - statement.setObject("filterStartTimeStamp", ts, Types.TIMESTAMP); - statement.setObject("filterStartTimeStamp", ts); + statement.setTimestamp(1, ts); + statement.setTimestamp(1, ts, cal); + statement.setObject(1, ts, Types.TIMESTAMP, 0); + statement.setObject(1, ts, Types.TIMESTAMP); + statement.setObject(1, ts); + } + + if (ModuleLoader.getInstance().hasModule("DataIntegration")) + { + try (CallableStatement statement = conn.prepareCall("{call etltest.etlTest(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)}")) + { + Assert.assertTrue(statement instanceof TimestampStatementWrapper); + statement.setTimestamp("filterStartTimeStamp", ts); + statement.setTimestamp("filterStartTimeStamp", ts, cal); + statement.setObject("filterStartTimeStamp", ts, Types.TIMESTAMP, 0); + statement.setObject("filterStartTimeStamp", ts, Types.TIMESTAMP); + statement.setObject("filterStartTimeStamp", ts); + } } } - } - catch (SQLException e) - { - throw new RuntimeException(e); + catch (SQLException e) + { + throw new RuntimeException(e); + } } } } From ee2b5e01452124541735c8fa3de00f3e0814f16e Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 26 Aug 2024 15:25:40 -0700 Subject: [PATCH 3/3] Flag unsupported date formats --- .../bigiron/mssql/MicrosoftSqlServer2016Dialect.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/bigiron/src/org/labkey/bigiron/mssql/MicrosoftSqlServer2016Dialect.java b/bigiron/src/org/labkey/bigiron/mssql/MicrosoftSqlServer2016Dialect.java index 974c3bc8642..44ba6912cc7 100644 --- a/bigiron/src/org/labkey/bigiron/mssql/MicrosoftSqlServer2016Dialect.java +++ b/bigiron/src/org/labkey/bigiron/mssql/MicrosoftSqlServer2016Dialect.java @@ -59,7 +59,14 @@ public void prepare(DbScope scope) // This seems to be the only string format acceptable for sending Timestamps, but unfortunately it's ambiguous; // SQL Server interprets the "MM-dd" portion based on the database's regional settings. So we must query the // current date format and switch the formatter pattern based on what we find. See Issue 51129. - _timestampFormatter = DateTimeFormatter.ofPattern("yyyy-" + ("mdy".equals(_dateFormat) ? "MM-dd" : "dd-MM") + " HH:mm:ss.SSS"); + String mdFormat = switch (_dateFormat) + { + case "mdy" -> "MM-dd"; + case "dmy" -> "dd-MM"; + default -> throw new IllegalStateException("Unsupported date format: " + _dateFormat); + }; + + _timestampFormatter = DateTimeFormatter.ofPattern("yyyy-" + mdFormat + " HH:mm:ss.SSS"); LOG.info("\n Language: {}\n DateFormat: {}", _language, _dateFormat); }