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

MariaDB: wrong Date value during select #1476

Closed
wants to merge 1 commit into from

Conversation

pepakriz
Copy link
Contributor

Hi,

I'm facing a strange bug with MariaDB driver. When I'm trying to load date property, the value is not shifted. It's still in UTC. Saving works correctly. The mysql driver contains a test for this scenario, but the MariaDB driver not. So I copy&pasted them and It's really failing just for MariaDB.

So here is PR with a failing code. Any idea, how to fix it?

@pepakriz
Copy link
Contributor Author

pepakriz commented Feb 22, 2021

Heh, Github pipelines are probably running in UTC, so everything seems OK. But try to run this branch locally and set process.env.TZ = 'Europe/Prague'.

Edit: fixed

@pepakriz
Copy link
Contributor Author

The difference between mariadb and mysql driver is that MySQL converts datetime type to native Date type. But MariaDB uses string representation. It is because the dateStrings option as a string[] is not supported by mariadb connector.

@B4nan
Copy link
Member

B4nan commented Feb 22, 2021

So better to raise the issue there - or a PR right ahead. I had to implement this myself too in the mysql2 package.

sidorares/node-mysql2#1200

@pepakriz
Copy link
Contributor Author

@B4nan nice. Ok, I'll try to prepare an equivalent solution for mariadb.

btw why you prefer this solution over custom types? I am just curious about that. I feel more confident with encoding/decoding on my own instead of random magic converts provided by DB drivers. But I'm not so familiar with internal things, so not sure about all consequences

B4nan added a commit that referenced this pull request May 27, 2023
Previously, mapping of datetime columns to JS `Date` objects was
dependent on the driver, while SQLite didn't have this out of box
support and required manual conversion on various places. There were
also places that didn't convert dates properly, e.g. inside embedded
objects. All drivers now have disabled `Date` conversion and this is
handled explicitly, in the same way for all the drivers.

Moreover, the `date` type was previously seen as a `datetime`, while now
only `Date` (with uppercase `D`) will be considered as `datetime`, while
`date` is just a `date`.

Closes #4362
Closes #4360
Closes #1476
@B4nan
Copy link
Member

B4nan commented May 27, 2023

This took way longer than it should have, but v6 should be finally good after #4362 (although the result mapped would deserve a bigger rewrite, I can see at least one more edge case that won't work properly). FYI doing this only via mapped types won't work, as we need to have normalized db value to compare, and using Date objects for that is actually pretty handy.

@B4nan B4nan closed this May 27, 2023
B4nan added a commit that referenced this pull request Jun 11, 2023
Previously, mapping of datetime columns to JS `Date` objects was
dependent on the driver, while SQLite didn't have this out of box
support and required manual conversion on various places. There were
also places that didn't convert dates properly, e.g. inside embedded
objects. All drivers now have disabled `Date` conversion and this is
handled explicitly, in the same way for all the drivers.

Moreover, the `date` type was previously seen as a `datetime`, while now
only `Date` (with uppercase `D`) will be considered as `datetime`, while
`date` is just a `date`.

Closes #4362
Closes #4360
Closes #1476
B4nan added a commit that referenced this pull request Sep 10, 2023
Previously, mapping of datetime columns to JS `Date` objects was
dependent on the driver, while SQLite didn't have this out of box
support and required manual conversion on various places. There were
also places that didn't convert dates properly, e.g. inside embedded
objects. All drivers now have disabled `Date` conversion and this is
handled explicitly, in the same way for all the drivers.

Moreover, the `date` type was previously seen as a `datetime`, while now
only `Date` (with uppercase `D`) will be considered as `datetime`, while
`date` is just a `date`.

Closes #4362
Closes #4360
Closes #1476
B4nan added a commit that referenced this pull request Sep 20, 2023
Previously, mapping of datetime columns to JS `Date` objects was
dependent on the driver, while SQLite didn't have this out of box
support and required manual conversion on various places. There were
also places that didn't convert dates properly, e.g. inside embedded
objects. All drivers now have disabled `Date` conversion and this is
handled explicitly, in the same way for all the drivers.

Moreover, the `date` type was previously seen as a `datetime`, while now
only `Date` (with uppercase `D`) will be considered as `datetime`, while
`date` is just a `date`.

Closes #4362
Closes #4360
Closes #1476
B4nan added a commit that referenced this pull request Sep 24, 2023
Previously, mapping of datetime columns to JS `Date` objects was
dependent on the driver, while SQLite didn't have this out of box
support and required manual conversion on various places. There were
also places that didn't convert dates properly, e.g. inside embedded
objects. All drivers now have disabled `Date` conversion and this is
handled explicitly, in the same way for all the drivers.

Moreover, the `date` type was previously seen as a `datetime`, while now
only `Date` (with uppercase `D`) will be considered as `datetime`, while
`date` is just a `date`.

Closes #4362
Closes #4360
Closes #1476
B4nan added a commit that referenced this pull request Sep 30, 2023
Previously, mapping of datetime columns to JS `Date` objects was
dependent on the driver, while SQLite didn't have this out of box
support and required manual conversion on various places. There were
also places that didn't convert dates properly, e.g. inside embedded
objects. All drivers now have disabled `Date` conversion and this is
handled explicitly, in the same way for all the drivers.

Moreover, the `date` type was previously seen as a `datetime`, while now
only `Date` (with uppercase `D`) will be considered as `datetime`, while
`date` is just a `date`.

Closes #4362
Closes #4360
Closes #1476
B4nan added a commit that referenced this pull request Oct 2, 2023
Previously, mapping of datetime columns to JS `Date` objects was
dependent on the driver, while SQLite didn't have this out of box
support and required manual conversion on various places. There were
also places that didn't convert dates properly, e.g. inside embedded
objects. All drivers now have disabled `Date` conversion and this is
handled explicitly, in the same way for all the drivers.

Moreover, the `date` type was previously seen as a `datetime`, while now
only `Date` (with uppercase `D`) will be considered as `datetime`, while
`date` is just a `date`.

Closes #4362
Closes #4360
Closes #1476
B4nan added a commit that referenced this pull request Oct 17, 2023
Previously, mapping of datetime columns to JS `Date` objects was
dependent on the driver, while SQLite didn't have this out of box
support and required manual conversion on various places. There were
also places that didn't convert dates properly, e.g. inside embedded
objects. All drivers now have disabled `Date` conversion and this is
handled explicitly, in the same way for all the drivers.

Moreover, the `date` type was previously seen as a `datetime`, while now
only `Date` (with uppercase `D`) will be considered as `datetime`, while
`date` is just a `date`.

Closes #4362
Closes #4360
Closes #1476
B4nan added a commit that referenced this pull request Oct 21, 2023
Previously, mapping of datetime columns to JS `Date` objects was
dependent on the driver, while SQLite didn't have this out of box
support and required manual conversion on various places. There were
also places that didn't convert dates properly, e.g. inside embedded
objects. All drivers now have disabled `Date` conversion and this is
handled explicitly, in the same way for all the drivers.

Moreover, the `date` type was previously seen as a `datetime`, while now
only `Date` (with uppercase `D`) will be considered as `datetime`, while
`date` is just a `date`.

Closes #4362
Closes #4360
Closes #1476
B4nan added a commit that referenced this pull request Oct 25, 2023
Previously, mapping of datetime columns to JS `Date` objects was
dependent on the driver, while SQLite didn't have this out of box
support and required manual conversion on various places. There were
also places that didn't convert dates properly, e.g. inside embedded
objects. All drivers now have disabled `Date` conversion and this is
handled explicitly, in the same way for all the drivers.

Moreover, the `date` type was previously seen as a `datetime`, while now
only `Date` (with uppercase `D`) will be considered as `datetime`, while
`date` is just a `date`.

Closes #4362
Closes #4360
Closes #1476
B4nan added a commit that referenced this pull request Nov 2, 2023
Previously, mapping of datetime columns to JS `Date` objects was
dependent on the driver, while SQLite didn't have this out of box
support and required manual conversion on various places. There were
also places that didn't convert dates properly, e.g. inside embedded
objects. All drivers now have disabled `Date` conversion and this is
handled explicitly, in the same way for all the drivers.

Moreover, the `date` type was previously seen as a `datetime`, while now
only `Date` (with uppercase `D`) will be considered as `datetime`, while
`date` is just a `date`.

Closes #4362
Closes #4360
Closes #1476
B4nan added a commit that referenced this pull request Nov 5, 2023
Previously, mapping of datetime columns to JS `Date` objects was
dependent on the driver, while SQLite didn't have this out of box
support and required manual conversion on various places. There were
also places that didn't convert dates properly, e.g. inside embedded
objects. All drivers now have disabled `Date` conversion and this is
handled explicitly, in the same way for all the drivers.

Moreover, the `date` type was previously seen as a `datetime`, while now
only `Date` (with uppercase `D`) will be considered as `datetime`, while
`date` is just a `date`.

Closes #4362
Closes #4360
Closes #1476
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.

None yet

2 participants