-
Notifications
You must be signed in to change notification settings - Fork 166
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
add support to save/fetch java.time data types #756
Conversation
Looking at different databases and the current Joda prototypes I see that timezones are unfortunately sometimes a problem. The current JDBC specification states that java.time.OffsetDateTime should be mapped to TIMESTAMP WITH TIMEZONE but alas there are quite many database vendors out there who only support that data type partially or not at all. Currently there would/should be the following data types:
The last one is currently supported in:
Timezones are currently not supported as data type in:
So there are a couple of ideas:
I would tend to 1. as to prevent confusion. If you really need timestamps with timezones you can either create your own prototype if your database supports it or otherwise use two attributes and put some logic into your EO to save a UTC timestamp and the timezone ID or offset separately so you can reconstruct the correct value after fetch. Please post your thoughts :) |
Firstly, thanks for doing this work.
Here's a thought: times, dates, timestamps and timezones are horrible.
Could you be more specific about exactly which prototype you're referring to? You're not talking about dropping any legacy prototypes, right? You mean a prototype whose Java-side representation is a Java 8 "date time with specific timezone"? |
No dropping of legacy prototypes, just not create an equivalent to the current jodaDateTime. |
Great, thanks. (Although in the past I've been a lot more blasé about backwards incompatibility and the need to move things forward where we can, dates and times is an area that makes me a little uncomfortable. I know from bitter past experience how hard it can be to get things right, and I also know what we're doing here right now Just Works because of that pain, so I'd be keen to keep the legacy prototypes around.) |
I think the proposed prototypes are OK, they match the JodaDate. I'm not sure about the code for the localdatetime and dateTime. The easiest way is to insert some values in a time zone and restart the app in another timezone to check how they react. Users of database without timezone support are either not using them or already save the information in another attribute. |
I did some testing with different settings and timezones. I would tend for this behavior:
Using that you don't need time zone support on the database side and thus can use any vendor. |
Can you name this prototype "javaDateTimeToUTC" to make it clear, with what we are dealing here? How does this approach work with existing tables in databases with full timestamp support, when the new date time classes get used in the EOModel? |
To comple te the tests, the server need to be started in a different time zone than the client. I spend a lot of time trying to figure it out before fixing the FrontBase prototypes. For based on the SQL standard, you should uses Timestamp with Time zone if the server and client lib are TZ aware. At least in FB some case does not work as expected with regular timestamp because the client lib adjust the result with the offset difference between the server and client timezone. The WITH TIME ZONE indicate the client software understand timezone and will handle them. PostgreSQL changed the default timestamp behaviour in the past, from their doc:
They are many possible TZ conversions in the process that may depend from the database server, client and frameworks used. |
I suppose with "server" you mean the db server? I don't think that that time zone matters as we are sending UTC timestamps to it and save in a data type that is time zone agnostic. All the time zone mungling happens at the client. I think that is the very same behavior as it is for the JodaTime prototypes?
I will try to test with FrontBase in the days coming.
To support all databases we cannot assume that the db server is TZ aware. That would be the fifth prototype where we would preserve the original time zone. But this must be left as an exercise to the programmer if his/her db is supporting that data type ;) |
I am not sure if that would be a good idea because UTC is only an implementation detail of the database. Within your WO-App you get a timestamp in your local time zone not UTC.
You mean if you currently have the JodaTime-prototypes and want to switch? That would need some testing but as long as the external data types are the same it probably should be no problem at all to switch to the new ones. In worst case you would throw in an SQL-update to adapt the values if you really need to change to java.time. |
add optional pattern variable for consumers that need the pattern of the formatter which you cannot reconstruct from a DateTimeFormatter unfortunately
many tests still need fixing as EO conversion seems to include primary key which did not when the tests were originally written
The test project ERRestTest is now runnable as JUnit target. In Eclipse you can right-click on the class file ERRestTest and choose Debug As > JUnit Test. Currently there are 11 of 40 test methods that are failing. Most of them due to ERRest automatically adding the primary key for EOs which apparently was not the case back when the test were written and by this not really critical. Though it would be good to fix those tests. If someone feels elected… ;-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested these new prototypes using PostgreSQL and they look good to me. Unless anyone else wants to test using their favourite database, I suggest this gets merged.
Thanks for your work here Johann. Really nice feature. |
This patch aims to add support of the new Java 8 Date & Time API and is a work-in-progress to be discussed. The current Joda data types are kept for backward compatibility.