Junit test for timestamps on French SQL Server (#5997)

Author: labkey-adam

api/src/org/labkey/api/data/SqlExecutingSelector.java

@@ -56,12 +56,7 @@ public abstract class SqlExecutingSelector<FACTORY extends SqlFactory, SELECTOR
     // optimizations won't mutate the ExecutingSelector's externally set state.
     abstract protected FACTORY getSqlFactory(boolean isResultSet);
 
-    SqlExecutingSelector(DbScope scope)
-    {
-        this(scope, null);
-    }
-
-    private SqlExecutingSelector(DbScope scope, Connection conn)
+    protected SqlExecutingSelector(DbScope scope, Connection conn)
     {
         this(scope, conn, new QueryLogging());
     }

api/src/org/labkey/api/data/TableSelector.java

@@ -31,6 +31,7 @@
 
 import jakarta.servlet.http.HttpServletResponse;
 import java.io.IOException;
+import java.sql.Connection;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.ArrayList;
@@ -62,9 +63,9 @@ public class TableSelector extends SqlExecutingSelector<TableSelector.TableSqlFa
     private boolean _forceSortForDisplay = false;
 
     // Primary constructor
-    private TableSelector(@NotNull TableInfo table, Collection<ColumnInfo> columns, @Nullable Filter filter, @Nullable Sort sort, boolean stableColumnOrdering)
+    protected TableSelector(@NotNull TableInfo table, @Nullable Connection conn, Collection<ColumnInfo> columns, @Nullable Filter filter, @Nullable Sort sort, boolean stableColumnOrdering)
     {
-        super(table.getSchema().getScope());
+        super(table.getSchema().getScope(), conn);
         _table = Objects.requireNonNull(table);
         _columns = columns;
         _filter = filter;
@@ -81,7 +82,7 @@ rely on column order (we return the values from the first one).
      */
     public TableSelector(@NotNull TableInfo table, Collection<ColumnInfo> columns, @Nullable Filter filter, @Nullable Sort sort)
     {
-        this(table, columns, filter, sort, isStableOrdered(columns));
+        this(table, null, columns, filter, sort, isStableOrdered(columns));
     }
 
     // Select all columns from a table, with no filter or sort
@@ -111,13 +112,13 @@ public TableSelector(@NotNull TableInfo table, @Nullable Filter filter, @Nullabl
     */
     public TableSelector(@NotNull TableInfo table, Set<String> columnNames, @Nullable Filter filter, @Nullable Sort sort)
     {
-        this(table, columnInfosList(table, columnNames), filter, sort, isStableOrdered(columnNames));
+        this(table, null, columnInfosList(table, columnNames), filter, sort, isStableOrdered(columnNames));
     }
 
     // Select a single column
     public TableSelector(@NotNull ColumnInfo column, @Nullable Filter filter, @Nullable Sort sort)
     {
-        this(column.getParentTable(), Collections.singleton(column), filter, sort, true);  // Single column is stable ordered
+        this(column.getParentTable(), null, Collections.singleton(column), filter, sort, true);  // Single column is stable ordered
     }
 
     // Select a single column from all rows

bigiron/src/org/labkey/bigiron/mssql/MicrosoftSqlServer2016Dialect.java

@@ -15,16 +15,31 @@
  */
 package org.labkey.bigiron.mssql;
 
+import jakarta.servlet.ServletException;
 import org.apache.commons.lang3.time.FastDateFormat;
 import org.apache.logging.log4j.Logger;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.junit.Assert;
 import org.junit.Test;
+import org.labkey.api.data.ColumnInfo;
+import org.labkey.api.data.CompareType;
 import org.labkey.api.data.ConnectionWrapper;
+import org.labkey.api.data.CoreSchema;
 import org.labkey.api.data.DbScope;
+import org.labkey.api.data.Filter;
+import org.labkey.api.data.RuntimeSQLException;
+import org.labkey.api.data.SQLFragment;
+import org.labkey.api.data.SimpleFilter;
+import org.labkey.api.data.Sort;
+import org.labkey.api.data.SqlExecutor;
 import org.labkey.api.data.SqlSelector;
+import org.labkey.api.data.TableInfo;
+import org.labkey.api.data.TableSelector;
 import org.labkey.api.data.dialect.SqlDialect;
 import org.labkey.api.data.dialect.StatementWrapper;
 import org.labkey.api.module.ModuleLoader;
+import org.labkey.api.query.FieldKey;
 import org.labkey.api.util.logging.LogHelper;
 
 import java.sql.CallableStatement;
@@ -35,8 +50,10 @@
 import java.sql.Timestamp;
 import java.sql.Types;
 import java.util.Calendar;
+import java.util.Collections;
 import java.util.Date;
-import java.util.Map;
+import java.util.GregorianCalendar;
+import java.util.Set;
 
 public class MicrosoftSqlServer2016Dialect extends MicrosoftSqlServer2014Dialect
 {
@@ -51,9 +68,16 @@ public void prepare(DbScope scope)
     {
         super.prepare(scope);
 
-        Map<String, Object> 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");
+        try (Connection conn = scope.getConnection())
+        {
+            LanguageSettings settings = getLanguageSettings(scope, conn);
+            _language = settings.getLanguage();
+            _dateFormat = settings.getDate_format();
+        }
+        catch (SQLException e)
+        {
+            throw new RuntimeSQLException(e);
+        }
 
         // 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
@@ -70,6 +94,48 @@ public void prepare(DbScope scope)
         LOG.info("\n    Language:                 {}\n    DateFormat:               {}", _language, _dateFormat);
     }
 
+    // TODO: Turn this into a record on 24.11 (24.7 SqlSelector doesn't support records)
+    public static class LanguageSettings
+    {
+        String _language;
+        String _date_format;
+
+        public String getLanguage()
+        {
+            return _language;
+        }
+
+        public void setLanguage(String language)
+        {
+            _language = language;
+        }
+
+        public String getDate_format()
+        {
+            return _date_format;
+        }
+
+        public void setDate_format(String date_format)
+        {
+            _date_format = date_format;
+        }
+
+        @Override
+        public String toString()
+        {
+            return "LanguageSettings{" +
+                    "_language='" + _language + '\'' +
+                    ", _date_format='" + _date_format + '\'' +
+                    '}';
+        }
+    }
+
+    private static LanguageSettings getLanguageSettings(DbScope scope, Connection conn)
+    {
+        return new SqlSelector(scope, conn, "SELECT language, date_format FROM sys.dm_exec_sessions WHERE session_id = @@spid")
+            .getObject(LanguageSettings.class);
+    }
+
     @Override
     public StatementWrapper getStatementWrapper(ConnectionWrapper conn, Statement stmt)
     {
@@ -274,5 +340,133 @@ private void test(TimestampStatementWrapper wrapper, String expected, String tes
             Timestamp ts = Timestamp.valueOf(test);
             Assert.assertEquals(expected, wrapper.convert(ts));
         }
+
+        @Test
+        public void testCompareClauses() throws SQLException, ServletException
+        {
+            // Issue 51472 pointed out issues with Timestamp conversions on French SQL Server. Primary fixes were in
+            // the DateCompareClause subclasses, so put them through their paces here.
+
+            // Use a test scope that passes out an un-pooled connection so changing the language settings don't affect
+            // connections in the pool. This also gives us a SqlDialect we can prepare every time we set the language.
+            try (TestScope scope = new TestScope(DbScope.getLabKeyScope()))
+            {
+                TableInfo containers = CoreSchema.getInstance().getTableInfoContainers();
+                ColumnInfo created = containers.getColumn("Created");
+
+                try (Connection conn = scope.getConnection())
+                {
+                    setLanguage(scope, conn, "English");
+                    testMultipleFilters(conn, containers, created.getFieldKey());
+
+                    if (scope.getSqlDialect().isSqlServer())
+                    {
+                        setLanguage(scope, conn, "French");
+                        testMultipleFilters(conn, containers, created.getFieldKey());
+                    }
+                }
+            }
+        }
+
+        private static class TestScope extends DbScope implements AutoCloseable
+        {
+            private TestConnectionWrapper _connection = getWrapped();
+
+            public TestScope(DbScope scope) throws ServletException, SQLException
+            {
+                super(scope.getDataSourceName(), scope.getLabKeyDataSource());
+            }
+
+            @Override
+            public Connection getConnection()
+            {
+                return _connection;
+            }
+
+            private TestConnectionWrapper getWrapped() throws SQLException
+            {
+                // Hand out an un-pooled connection since we might set language and don't want that to persist outside this test
+                return new TestConnectionWrapper(getUnpooledConnection(), this);
+            }
+
+            @Override
+            public void close() throws SQLException
+            {
+                _connection.closeConnection();
+                _connection = null;
+            }
+
+            private static class TestConnectionWrapper extends ConnectionWrapper
+            {
+                public TestConnectionWrapper(Connection conn, DbScope scope)
+                {
+                    super(conn, scope, null, DbScope.ConnectionType.Transaction, null);
+                }
+
+                @Override
+                public void close()
+                {
+                    // No-op
+                }
+
+                private void closeConnection() throws SQLException
+                {
+                    super.close();
+                }
+            }
+        }
+
+        private void setLanguage(DbScope scope, Connection conn, String language)
+        {
+            SqlDialect dialect = scope.getSqlDialect();
+            if (dialect.isSqlServer())
+            {
+                new SqlExecutor(scope, conn).execute("SET LANGUAGE " + language);
+                dialect.prepare(scope);
+                LOG.info(getLanguageSettings(scope, conn));
+            }
+        }
+
+        private void testMultipleFilters(Connection conn, TableInfo table, FieldKey date)
+        {
+            Calendar cal = new GregorianCalendar();
+            cal.add(Calendar.DATE, -30);
+            Date startDate = cal.getTime();
+
+            testFilter(conn, table, date, startDate, CompareType.DATE_EQUAL);
+            testFilter(conn, table, date, startDate, CompareType.DATE_NOT_EQUAL);
+            testFilter(conn, table, date, startDate, CompareType.DATE_GTE);
+            testFilter(conn, table, date, startDate, CompareType.DATE_GT);
+            testFilter(conn, table, date, startDate, CompareType.DATE_LT);
+            testFilter(conn, table, date, startDate, CompareType.DATE_LTE);
+        }
+
+        // We don't care about the row counts, just that each query executes without any exceptions
+        private void testFilter(Connection conn, TableInfo table, FieldKey fk, Object value, CompareType type)
+        {
+            SimpleFilter filter = new SimpleFilter(fk, value, type);
+
+            new TestTableSelector(table, conn, Collections.singleton(table.getColumn(fk)), filter, null).getRowCount();
+
+            // This mimics the query that UserManager.getActiveDaysCount() generates
+            SQLFragment sql = new SQLFragment("SELECT * FROM (SELECT CAST(")
+                .append(fk.getName())
+                .append(" AS DATE) AS ")
+                .append(fk.getName())
+                .append(" FROM ")
+                .append(table.getSelectName())
+                .append(") x ")
+                .append(filter.getSQLFragment(table.getSqlDialect()));
+
+            new SqlSelector(table.getSchema().getScope(), conn, sql).getRowCount();
+        }
+
+        private static class TestTableSelector extends TableSelector
+        {
+            public TestTableSelector(@NotNull TableInfo table, @NotNull Connection conn, Set<ColumnInfo> columns, @Nullable Filter filter, @Nullable Sort sort)
+            {
+                super(table, conn, columns, filter, sort, true);
+            }
+        }
     }
 }