Skip to content

Conversation

@labkey-adam
Copy link
Contributor

Rationale

Related PR fixes issues with timestamps used in compare clauses on SQL Server. This adds a junit test for the fix.

Related Pull Requests

Changes

  • Optional ability to pass a Connection to a TableSelector
  • Exercise timestamps with various compare clause scenarios. On SQL Server, exercise the test steps with connection set to English and then French.

@labkey-tchad
Copy link
Member

Verifying on Old Dependencies suite. If there were to be surprise failures, that seems like a likely place.

Copy link
Member

@labkey-tchad labkey-tchad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be causing a connection leak.

@labkey-tchad
Copy link
Member

Yup, got a run with mem tracker stacktraces enabled:

org.labkey.api.util.MemTracker.put(MemTracker.java:371)
org.labkey.api.data.ConnectionWrapper.<init>(ConnectionWrapper.java:119)
org.labkey.bigiron.mssql.MicrosoftSqlServer2016Dialect$TestCase$1.getWrapped(MicrosoftSqlServer2016Dialect.java:365)
org.labkey.bigiron.mssql.MicrosoftSqlServer2016Dialect$TestCase$1.<init>(MicrosoftSqlServer2016Dialect.java:354)
org.labkey.bigiron.mssql.MicrosoftSqlServer2016Dialect$TestCase.testCompareClauses(MicrosoftSqlServer2016Dialect.java:352)

@labkey-adam
Copy link
Contributor Author

Yup, got a run with mem tracker stacktraces enabled:

org.labkey.api.util.MemTracker.put(MemTracker.java:371)
org.labkey.api.data.ConnectionWrapper.<init>(ConnectionWrapper.java:119)
org.labkey.bigiron.mssql.MicrosoftSqlServer2016Dialect$TestCase$1.getWrapped(MicrosoftSqlServer2016Dialect.java:365)
org.labkey.bigiron.mssql.MicrosoftSqlServer2016Dialect$TestCase$1.<init>(MicrosoftSqlServer2016Dialect.java:354)
org.labkey.bigiron.mssql.MicrosoftSqlServer2016Dialect$TestCase.testCompareClauses(MicrosoftSqlServer2016Dialect.java:352)

Yep, I see that. Will track it down...

@labkey-adam
Copy link
Contributor Author

labkey-adam commented Oct 30, 2024

@labkey-adam labkey-adam merged commit c71d298 into release24.7-SNAPSHOT Oct 30, 2024
@labkey-adam labkey-adam deleted the 24.7_fb_test_french branch October 30, 2024 16:11
Copy link
Member

@labkey-tchad labkey-tchad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is making me very paranoid about anonymous classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants