Skip to content

Investigating SQL Server 2022 issue#193

Draft
LasseEngboChr wants to merge 38 commits intomainfrom
fix/SQL_Server_2022
Draft

Investigating SQL Server 2022 issue#193
LasseEngboChr wants to merge 38 commits intomainfrom
fix/SQL_Server_2022

Conversation

@LasseEngboChr
Copy link
Contributor

SQL Server 2022 courses a problem with getTableSignature. The reason may be an update in the CI version of SQL Server 2022.

Intent

Fix datatypes so that SQL Server 2022 is still supported.

Known issues

Checklist

  • The PR passes all local unit tests
  • I have documented any new features introduced
  • If the PR adds a new feature, please add an entry in NEWS.md
  • A reviewer is assigned to this PR

@LasseEngboChr
Copy link
Contributor Author

The issue is most likely related to the latest version of SQL Server 2022:
https://sqlserverbuilds.blogspot.com/2021/07/sql-server-2022-versions.html
The Github action is using build 16.0.4185.3 from 2025-03-13.

The error in the test of getTableSignature appeared the first time we pushed after that update.

The error is isolated. It is not getTableSignature that fails but:
dplyr::copy_to(conn, data_random)
Fails if data_random includes a column of type POSIXct.

@LasseEngboChr
Copy link
Contributor Author

LasseEngboChr commented Mar 28, 2025

Reverse engineering ...
dbplyr::db_copy_to
-> dplyr::db_write_table
--> DBI::dbWriteTable
(With more steps inbetween)

DBI::dbWriteTable(conn, "#testthis2", value = data_random, temporary = TRUE) # Works

dpplyr::db_write_table wraps with as_table_path which results in:
DBI::dbWriteTable(conn, DBI::SQL(dbplyr::as_table_path("#testthis2B0", conn)), value = data_random, temporary = TRUE) # Fails
DBI::SQL(dbplyr::as_table_path("#testthis2B0", conn)) is the same as just DBI::SQL("#testthis2B0")
DBI::dbWriteTable(conn, DBI::SQL("#testthis2A"), value = data_random, temporary = TRUE) # Fails

If I leave the POSIXct variable out then it works:
DBI::dbWriteTable(conn, DBI::SQL(dbplyr::as_table_path("#testthis2B0", conn)), value = data_ok, temporary = TRUE) # Works

So we are down to differences in behavior between:
getMethod("dbWriteTable", signature = c("OdbcConnection", "character", "data.frame"))
and
getMethod("dbWriteTable", signature = c("OdbcConnection", "SQL", "data.frame"))
when the data.frame contains a POSIXct as created by e.g. Sys.time()

@LasseEngboChr
Copy link
Contributor Author

I created an issue for odbc (r-dbi/odbc#906) and look forward to follow that.

@LasseEngboChr
Copy link
Contributor Author

I created an issue for odbc (r-dbi/odbc#906) and look forward to follow that.

Seems like a solution is found ...

@LasseEngboChr
Copy link
Contributor Author

I created an issue for odbc (r-dbi/odbc#906) and look forward to follow that.

Seems like a solution is found ...
A temporary fix is added in test-getTableSignature.R. It installs odbc from a branch with the fix if needed. Those lines at the top of the file should be deleted as soon as an updated odbc is out.
Or maybe we should just leave this PR as is and close it when odbc is updated...

@LasseEngboChr
Copy link
Contributor Author

This PR should not be merged into main. But it is left open as an opportunity to test with the particular branch of odbc.
The forced installation makes other tests on other platforms fail ...

@LasseEngboChr LasseEngboChr added the wontfix This will not be worked on label Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wontfix This will not be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant