PHOENIX-5422 Use Java8 DateTime APIs instead of joda-time APIs

[richardantal]

Change-Id: Icb0e89060528930282c139af1c7254cc8d36e054

[richardantal]

Thank you @gjacoby126 for the review and for the insightful questions.

I will be on holiday for a week and a half but after that I'll give further updates.

[gjacoby126]

@richardantal - I see that I missed a bunch of discussion back in PHOENIX-6486, but I'm not yet convinced that having an equivalent to GJChronology / BritishCutoverChronology is the right design.

It looks like some other databases, such as MySQL, just use the Gregorian calendar for all time and require users to make adjustments. (See https://dev.mysql.com/doc/refman/8.0/en/mysql-calendar.html).

This pushes potentially annoying conversions on our users, but I think the user is the only one who can actually do this correctly. That's because the exact calendar in use on a particular date is going to be use-case and culturally-specific -- there's no one right answer.

GJChronology is going to be "right" for areas that were part of the Spanish Empire and a few other countries in 1582 and wrong for everyone else. (Even some countries, like France, that adopted the Gregorian calendar in 1582 didn't do it on the same day that GJChronology assumes.)

BritishCutoverChronology is going to be "right" for areas that were a part of the British Empire in 1752, but nowhere else. (See https://en.wikipedia.org/wiki/List_of_adoption_dates_of_the_Gregorian_calendar_by_country)

Perhaps we can make it configurable, but I don't see a way to use any single calendar other than the ISO calendar, as MySQL does, as a one-size-fits-all.

[stoty]

I agree with you @gjacoby126, but let me provide some context:

In my experience the Chronology issue has less to with real historical use cases than with being able to handle synthetic "minus infinity" constants like '01-01-01' or '1000-01-01' consistently.

The reason we stuck with GJChornology at the last change was because that's what Phoenix has effectively been using already, based on the default behaviour of java.util.Calendar, and this was the easiest way to be consistent on the read and write paths (and date functions) and between Joda and j.u.Calendar, which is still extensively used in Phoenix code.

GJ is also the only Chronology that works without surprises with naive java Date code, i.e. when you do

// assume we're in UTC
j.u.t.Date d = new j.u.d(1000,1,1,1,1,1);
// write d to Phoenix with a PreparedStatement
// read d back
assertEquals("1000-01-01 01:01:01", rs.getString("d"))

It is only going to succeed with GJChronology (or similar, like BritishCutoverChronology).

Consequently, as long as Phoenix uses j.u.Date / Calender with the default Chronology, it is the only Calendar that can guarantee internal consistency in Phoenix. The above is also true for views on HBase tables where custom timestamps are naively generated.

In a nutshell, GJChronology is the default chronology for j.u.Calendar/Date, while ISOChronology is the default for Joda and java.time, and we need to handle this discrepancy somehow.

At the same time, using a cutover chronology is both against the standard, and has a performance cost, so I support using ISOChronology as the default, and basically pruging all j.u.Date/Calendar code from Phoenix.

However, for backwards compatibility both with existing code and existing data, I believe that we also ought to make Chronology configurable.

[gjacoby126]

Thanks @stoty , I hadn't realized that the Java 7 (and before) GregorianCalendar had an internal cutover similar to GJChronology.

So for me then the question is what version is this PR targetted for? If this is for 5.2, I can see it making sense for a short-term change that keeps the old behavior but removes the need for joda. (But would there be any problem to keeping joda for the 5.2 cycle and replacing it in 5.3 as part of a more comprehensive rework? If the answer is yes, that's fine, it's just a pretty big patch if a lot of this is temporary.)

If this is for 5.3, it makes sense to do the larger work to get the date and time logic to what you suggest -- no j.u.Date / Calendar, switching to ISOChronology with the ability for users to configure an alternative.