eclipse-ee4j / eclipselink

Eclipselink project
https://eclipse.dev/eclipselink/
Other
192 stars 159 forks source link

Map JDK 8 type Instant to TIMESTAMP instead of LONGVARBINARY #259

Open arjantijms opened 5 years ago

arjantijms commented 5 years ago

JPA 2.2 defined that a subset of the (then) new JSR 310 date/time types were to be mapped automatically by the JPA implementation.

Unfortunately, the Instant type wasn't among those types being specified.

Hibernate and DataNucleus map these to TIMESTAMP, while EclipseLink just serialises the object into LONGVARBINARY, which makes it an opaque blob in the database.

As pre-cursor to add this forgotten type to the JPA spec, it might be good for EclipseLink to align with Hibernate and DataNucleus.

Also see this SO question: https://stackoverflow.com/q/50142822/472792

arjantijms commented 5 years ago

p.s. also see this JPA spec issue:

https://github.com/eclipse-ee4j/jpa-api/issues/163

dazey3 commented 5 years ago

Interesting topic! Thanks for bringing this up. As was quoted from the SO post:

Section 2.2 of the JPA 2.2 specification:

The persistent fields or properties of an entity may be of the following types: 
    Java primitive types, 
    java.lang.String, 
    other Java serializable types (including 
        wrappers of the primitive types,  
        java.math.BigInteger, java.math.BigDecimal,  
        java.util.Date, java.sql.Date,  
        java.util.Calendar[5],  
        java.sql.Time, java.sql.Timestamp,  
        byte[], Byte[], char[], Character[],  
        java.time.LocalDate, 
        java.time.Local-Time, java.time.LocalDateTime,  
        java.time.OffsetTime,  java.time.OffsetDateTime,  
        and user-defined types that implement the Serializable interface); 
    enums; 
    entity types; 
    collections of entity types; 
    embeddable classes (see Section 2.5); 
    collections of basic and embeddable types (see Section 2.6).

Since EclipseLink is the reference implementation, I don't think we should start deviating from the specification and add support for other types not listed there; like java.time.Instant. That being said, I think this is a great topic to bring up for the next JPA specification discussion. I think if you open an issue here: https://github.com/eclipse-ee4j/jpa-api/issues, then the specification group can discuss it for the next release! :)

arjantijms commented 5 years ago

I think if you open an issue here: https://github.com/eclipse-ee4j/jpa-api/issues, then the specification group can discuss it for the next release! :)

The issue is already there, see https://github.com/eclipse-ee4j/jpa-api/issues/163 ;)

dazey3 commented 5 years ago

Oh, lol, not sure how I missed that comment! Thanks for pointing that out. Looks like @lukasj is on top of it then. Not sure how he wants to proceed with adding support given it is omitted from the spec. Maybe this was in error and the spec was meant to support this. Not sure.

lukasj commented 5 years ago

Instant is not covered by JDBC, so is not by JPA. At least not yet.

MasatoshiTada commented 5 years ago

There's a similar issue with java.time.LocalDate (mapped to VARCHAR). I'm using org.eclipse.persistence.jpa:2.7.3 and PostgreSQL 9.4.20. https://www.eclipse.org/forums/index.php/m/1789940/?srch=localdate#msg_1789940

cen1 commented 4 years ago

I don't know the spec lingo so I am not sure whether the phrase "may be of the following types" explicitely prohibits supporting additional types but since other implementations can do that and remain compliant I'd say you could. Ommiting java.time.Instant from JPA 2.2 was really weird (even funny to be honest). Hopefully this gets into some minor release with or without the spec, in the meantime we will survive with AttributeConverters.