Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DatabaseWrapper: First draft of achieving independency from locales #20

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

Andrei-Errapart
Copy link

Problem to be solved
As present the DatabaseWrapper at least regarding MS Sql Server is dependent on the database on the SQL server having the same regional settings as DatabaseWrapper. The problem becomes worse when there are multiple databases with different locales.

Corresponding issue:
#17

Solution
The solution resolves the problem by using parametrized SQL queries. The amount of code is about the same as when sanitizing all the inputs.

Let me note that the reduction of lines of code in DatabaseWrapper.SqlServer was not due to conversion to use parametrized SQL queries.

As soon as the SQL query parametrization is OK for DatabaseWrapper.SqlServer, I will implement it for other drivers, too.

I am all ears for the improvements.

@Andrei-Errapart
Copy link
Author

Andrei-Errapart commented Dec 28, 2023

  1. I think removing the method TimeStamp and associated members was a mistake. I will put them back.
  2. In the case of Postgresql, the Npsql library has changed the handling of DateTimeOffsets in version 6.0: https://www.npgsql.org/doc/types/datetime.html. The Npsql only accepts DateTimeOffsets which have 0 for Offset.

@Andrei-Errapart
Copy link
Author

At first sight it seems to work with RestDb.

@jchristn
Copy link
Owner

Hi @Andrei-Errapart sorry for the delay on getting back to you on this one, I will do my best to get to it ASAP. I appreciate your contribution!

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.

2 participants