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

MySQL Java time instances #1791

Closed
wants to merge 3 commits into from
Closed

MySQL Java time instances #1791

wants to merge 3 commits into from

Conversation

guymers
Copy link
Contributor

@guymers guymers commented Dec 25, 2022

No description provided.

build.sbt Outdated Show resolved Hide resolved

implicit val JavaTimeLocalDateTimeMeta: Meta[java.time.LocalDateTime] =
Basic.oneObject(
JT.Timestamp,
Copy link
Contributor

Choose a reason for hiding this comment

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

MySQL/MariaDB has only TIMESTAMP which behaves as if WITH TIME ZONE, right? It is the equivalent of Java's Instant.

So that would mean that LocalDateTime is not suitable for TIMESTAMP, right?

Copy link
Contributor Author

@guymers guymers Dec 27, 2022

Choose a reason for hiding this comment

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

For MySQL a DATETIME column = JDBC Timestamp, and a TIMESTAMP column = JDBC Timestamp. I've added a hack like for Postgres to treat a TIMESTAMP column as TimestampWithTimezone and adjusted the instances.

classOf[java.time.OffsetDateTime]
)

implicit val JavaTimeInstantMeta: Meta[java.time.Instant] =
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be the "primary" Meta instance and the one for OffsetDateTime be derived from it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instant isn't mentioned in the JDBC spec so drivers don't have to provide a way to convert a column to/from it. The MySQL driver errors if you ask it for an Instant so it is implemented in terms of OffsetDateTime.

Copy link
Contributor

@sideeffffect sideeffffect left a comment

Choose a reason for hiding this comment

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

❤️

@sideeffffect
Copy link
Contributor

@guymers why did you close this? Is it superseded by #1789 ?

@guymers
Copy link
Contributor Author

guymers commented May 31, 2023

Its been 5 months and I don't use mysql.

@jatcwang
Copy link
Collaborator

I'll get this through the finish line. Thanks

@jatcwang jatcwang reopened this May 31, 2023
@sideeffffect
Copy link
Contributor

sbt mergifyGenerate needs to be run on this branch

@sideeffffect
Copy link
Contributor

@guymers should this be merged before or after #1789 ?

@guymers
Copy link
Contributor Author

guymers commented Jun 1, 2023

Doesn't really matter, but may as well do this one.

@jatcwang jatcwang added this to the 1.0 milestone Jan 7, 2024
# Conflicts:
#	.github/workflows/ci.yml
#	build.sbt
#	modules/core/src/main/scala/doobie/util/analysis.scala
@jatcwang jatcwang self-assigned this Jan 17, 2024
@jatcwang
Copy link
Collaborator

Superceded by #2024 after #2023 allowing more precise typechecking based on DB vendor's type name.
@guymers I've added you as a coauthor in the commit. Thanks!

@jatcwang jatcwang closed this Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants