Support legacy date/time format

[richardantal]

[SQLLINE-66] Add dateFormat, timeFormat and timestampFormat properties a75929d
Introduced Parameters for timeformat dateformat and timestampformat but this change calls the getObject instead of the getString function.
In Phoenix it results the following result after setting the timeformat to "yyyy-MM-dd HH:mm:ss.SSS"
Note that I am in GMT+2

0: jdbc:phoenix:localhost:49467> select current_time();
+--------------------------------+
| TIME '2021-06-09 12:50:51.060' |
+--------------------------------+
| 2021-06-09 14:50:51.060           |
+--------------------------------+

Before the same query would have given the result in GMT
Create a parameter like LEGACY_DATE_AND_TIME_FOMRAT to be able to have the result using getString method.

[stoty]

We should take a look the older behaviour, and if non-temporal field formats were also changed, then handle those as well.

perhaps having a universal option, that uses the JDBC driver's getString() for every type would work.

[richardantal]

I link my PR to this issue: #442
I am not sure if it makes sense to use getString() for every types like numbers

[julianhyde]

Using the JDBC driver's ResultSet.getString() is certainly an option. The problem is that the is a lot of variation among JDBC drivers. For example, given a SQL TIMESTAMP value (which has no time zone) some drivers will print the zoneless value (e.g. 2021-01-01 00:00:00) and others will convert to a java.util.Timestamp and call toString() on that, resulting in values like 2020-12-31 17:00:00 PST. Then there are differences of precision, scale, etc.

So I recommend that SQLLine stays in control of formatting.

[stoty]

The current formatting works for databases that have JDBC compliant date/time handling.
However, at least Phoenix definitely does not.

Phoenix stores nd displays all temporal fields in UTC, and when sqlline tries to handle these according to the local timezone, the results make little sense.

Richard's patch doesn't change the current formatting, it just adds an option to restore the old one, which accidentally works with Phoenix.

[stoty]

Can we revisit this @julianhyde ?

I agree that for most DBs the current behaviour is fine, however it causes big problems with Phoenix, where some users have to downgrade the sqlline library to be able to use their applications.

This patch doesn't change the default behaviour, it merely adds a new option to revert to the old behaviour for those who need it. This new option must be explicitly enabled to change the current behaviour.

We have plans to fix the base problem in Phoenix, but we also must keep the old broken behaviour as an option for ppl who have come to rely on the broken implementation, so the problem won't go away soon.

I also have an alternate patch, that uses a magic format string to enable the legacy behavior instead of a new property. I can upload that one if you prefer.

I found some existing tests using H2's SimpleResultSet, which could be used to make a unit test for the change, if you think that tests are necessary.

[julianhyde]

Would it be sufficient for users to unset dateFormat, timeFormat, timestampFormat properties, and for SQLLine to call getString rather than getObject in the event that they are unset?

Otherwise (or perhaps in addition) if the problem is that Phoenix treats SQL TIMESTAMP values as an instant (relative to UTC) as opposed to a local-date-time (not relative to any time zone), maybe we could add a boolean property SQLLine so that it could format those values correctly.

Something similar to Snowflake's TIMESTAMP_TYPE_MAPPING property (which has allowable values TIMESTAMP_LTZ, TIMESTAMP_NTZ, TIMESTAMP_WTZ) might work.

[stoty]

Thanks for your response.

Yes, the objective is to somehow get sqline to get to use toString() for temporal types.
Using the empty string as a magic value to enable the toString() code path is fine.

It's getting late for me, but I will put up another PR with your proposed behaviour tomorrow, and ping you on it.

The following is more than most ppl want to know about Phoenix's time handling, so feel free to skip the rest.

Actually, Phoenix MOSTLY teats its Temporal types as WITHOUT TIMEZONE (internally represented as Instants in UTC), it just fails to apply the TZ displacement on the ResultSet.getTimestamp()/Date()/Time()/Object() and the corresponding PreparedStatement.setXX() methods. However, when using time literals, or getString() / setString(), those are parsed/formatted as UTC, and in these cases Phoenix behaves close to the standard for WITHOUT TIMEZONE types.

So yes, Phoenix not applying the TZ displacement on getDate() etc is half of the problem, and having the option to apply it in sqlline would solve the TZ offset problem.

However, Phoenix's date handling has another problem, in that Phoenix only really has nano- and millisecond resolution TIMESTAMP types, and DATE and TIME are effectively just millisecond resolution timestamps. The actual TIMESTAMP type is nanosecond resolution, which most users don't need. As they are only aliases, Phoenix by default formats (and parses) all temporal types as TIMESTAMP, with both the date and time parts printed.

Because of this, a lot of users just use DATE for millisecond resolution TIMESTAMP / DATETIME values.

However, the toString() method in java.sql.Date and java.sql.Time do treat those values as real DATE and TIME types, and format them accordingly, omitting the TIME or DATE part.

So even if sqlline had the option to apply the missing TZ displacement, users would still have to cast every misued type to TIMESTAMP in their queries to see the time parts that they need.

I am working on adding proper TZ displacement handling to Phoenix, but as this would break a lot existing applications, this is going to be off by default.

Most of the above problems are sidestepped if we just use toString() to print the temporal types, which uses the correct (UTC) TZ, and prints the full timestamp.

[stoty]

Uploaded the new PR #478 , @julianhyde.