Consider modern APIs for JDBC connectors' predicate pushdown of TIME/TIMESTAMP WITH TIME ZONE

[findepi]

When a connector extending presto-base-jdbc gets TIME WITH TIME ZONE / TIMESTAMP WITH TIME ZONE pushed down, it passes it to the database using

• for TIME W/TZ: statement.setTime(), whereas as of JDBC 4.2 statement.setObject(OffsetTime.of(...)) should be used instead
• for TIMESTAMP W/TZ: statement.setTimestamp(), whereas as of JDBC 4.2 statement.setObject(ZonedDateTime.of(...)) (or perhaps OffsetDatetime?) should be used instead

(relevant code is in com.facebook.presto.plugin.jdbc.QueryBuilder#buildSql)

[amitdesai03]

@findepi is this change in approach suggested somewhere in JDBC 4.2 specs? Could you please point out references?

[findepi]

@amitdesai03 JDBC spec doesn't suggest a change, as always, it's backward compatible. However

Conversions by setObject and setNull from Java Object Types to JDBC Types
TABLE B-5 shows which JDBC types may be specified as the target JDBC type to the methods PreparedStatement.setObject, PreparedStatement.setNull, RowSet.setNull, and RowSet.setObject.
...
java.time.OffsetTime | CHAR, VARCHAR, LONGVARCHAR, TIME_WITH_TIMEZONE
java.time.OffsetDatetime | CHAR, VARCHAR, LONGVARCHAR, TIME_WITH_TIMEZONE, TIMESTAMP_WITH_TIMEZONE

(your question made me realize the spec talks about OffsetDatetime rather than ZonedDateTime -- thanks!)

My reasoning is that OffsetTime and ZonedDateTime are a much better representation for TIME WITH TIME ZONE / TIMESTAMP WITH TIME ZONE (since the java.sql.Time[stamp] types do not contain a zone).
However,

• i don't recall any database actually (and correctly) supporting TIME WITH TIME ZONE so this case might be a dead code anyway (if not dead, it's definitely hard to test)
  • also, any change here requires Remove support for political time zones from TIME WITH TIME ZONE #10103 first
• although java.sql.Timestamp is used for TIMESTAMP WITH TIME ZONE, it appears to work (but we don't have good test coverage for that)

Anyway, I would abstain from any work on this until #7122 is resolved.

[stale]

This issue has been automatically marked as stale because it has not had any activity in the last 2 years. If you feel that this issue is important, just comment and the stale tag will be removed; otherwise it will be closed in 7 days. This is an attempt to ensure that our open issues remain valuable and relevant so that we can keep track of what needs to be done and prioritize the right things.