Skip to content

Add database dependent simple types [DATAJDBC-443] #663

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

Closed
spring-projects-issues opened this issue Nov 14, 2019 · 7 comments
Closed

Add database dependent simple types [DATAJDBC-443] #663

spring-projects-issues opened this issue Nov 14, 2019 · 7 comments
Assignees
Labels
type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link

Jens Schauder opened DATAJDBC-443 and commented

We should support a way to mark types as directly supported by a database (driver).
This should probably be part of a dialect that may register additional simple types.

According to a gitter discussiong postgres supports LocalDate, LocalTime, OffsetDateTime and LocalDateTime without conversions


Issue Links:

  • DATAJDBC-414 Add support for persisting OffsetDateTime/ZonedDateTime
@spring-projects-issues
Copy link
Author

Sineaggi commented

Source on the postgres driver java.time.* support is https://jdbc.postgresql.org/documentation/head/8-date-time.html

@spring-projects-issues
Copy link
Author

Jens Schauder commented

Since we now have dialects in place this should now be possible to get implemented

@spring-projects-issues
Copy link
Author

Rick Heuijerjans commented

Hi, is this ticket open for contribution? I'd like to pick this one up if you are not already actively working on it.

I already did a quick implementation for extracting the objects from the ResultSet with the correct type to get a feel for the complexity. If you want I can continue working on it to turn it into a proper feature.

My idea is to add the additional types to the Dialect (as mentioned in the description), and then in the ResultSetAccessor pass the type to the ResultSet if the targetType is in the collection of additional types, something like this:

 

if (vendorSpecificSupportedTypes.contains(targetType)) {
   return resultSet.getObject(index, targetType);
}

return resultSet.getObject(index);

were targetType is the type of the field in the @Entity.

 

I did not yet look into the query side of things, but please let me know if this ticket is open for contribution

@spring-projects-issues
Copy link
Author

Jens Schauder commented

rheuijerjans contributions would be very welcome on this issue. Let me know if you have any questions

@nanella
Copy link

nanella commented Aug 2, 2021

Hi, is there any update on this?

Currently, using a LocalDate on an entity results in funny behaviour (e.g. adding or removing a day depending on timezone), since the database result is converted to java.sql.Date before being converted to LocalDate, therefore adding in some timezone shenaningans.

Using natively supported types would be much appreciated in this case :)

@schauder
Copy link
Contributor

Implementing a database specific simple type is now simple. See https://github.com/spring-projects/spring-data-jdbc/blob/d1e8e722c198d4bcb13ad75b93014d6dc00ee0ec/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/PostgresDialect.java#L215 for an example.

For LocalDateTime this is already done by
8652418

But what you describe here and in pgjdbc/pgjdbc#2221 wouldn't be solved by this approach (and this ticket here). Simple types are only used for deciding what values should get converted before passed to the database. When loading a value we always use the database default in the optimistic assumption that it is one with full information about the data in the database and that we can extract whatever type is required by the domain model.

We might consider this strategy (and have in a few very special cases), but we would need a separate issue for that. Could you please create one? And if you could create a reproducer for this, that would be great.

@schauder
Copy link
Contributor

This was fixed by #981

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants