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

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

Open
arjantijms opened this issue Oct 3, 2018 · 7 comments
Open

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

arjantijms opened this issue Oct 3, 2018 · 7 comments

Comments

@arjantijms
Copy link

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
Copy link
Author

p.s. also see this JPA spec issue:

jakartaee/persistence#163

@dazey3
Copy link
Contributor

dazey3 commented Oct 4, 2018

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
Copy link
Author

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 jakartaee/persistence#163 ;)

@dazey3
Copy link
Contributor

dazey3 commented Oct 4, 2018

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
Copy link
Member

lukasj commented Oct 8, 2018

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

@MasatoshiTada
Copy link

MasatoshiTada commented Dec 12, 2018

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
Copy link

cen1 commented Feb 11, 2020

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.

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

No branches or pull requests

5 participants