[jira] [Comment Edited] (CALCITE-5488) Avatica double-adjusts timestamps when calling Array.getResultSet()

["Will Noble (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CALCITE-5488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17680016#comment-17680016 ] 

Will Noble edited comment on CALCITE-5488 at 2/13/23 7:26 PM:
--------------------------------------------------------------

This is actually a pretty narrow bug. I'll step through the problem at a higher level than in the ticket description to illuminate what I see as the solution:

Problem:
1. We get a {{ResultSet}} that contains an array that contains timestamps.
2. We properly adjust all the timestamps in the array when we call {{{}getArray(){}}}.
3. We decide to convert that array to a "sub-{{{}ResultSet{}}}" by calling {{java.sql.Array.getResultSet()}} on the array, causing the timestamps to be double-adjusted.

It's not so much that any of your 4 example constraints were violated. Rather, it's that the process of of getting an array and then converting it to a result set involves an incorrect series of operations on the primitive ({{{}long{}}}) internal representation of the result set before it's abstracted into a {{java.sql.Timestamp}} object.

My original proposal was to set the local calendar of any {{ResultSet}} created by {{java.sql.Array.getResultSet()}} to UTC. By static analysis of the code, I can verify that this will *only* affect this particular narrow case: getting time/date/timestamp values out of an array result set, as well as the return value of {{{}AvaticaResultSet.getLocalCalendar(){}}}, which is a public method but is not part of the JDBC spec and is not used anywhere in the Avatica codebase. However, this would create a problematic edge-case where an explicit calendar is provided to get the individual timestamps out of the array's result set; they would be incorrect.

The only real solution I see now is to "de-adjust" the data in the result set during {{ArrayFactoryImpl.create()}} (in between steps 2 and 3 of the problem above), effectively undoing the first adjustment and making it so that the second adjustment (whether implicit or explicit) is correct.


was (Author: wnoble):
This is actually a pretty narrow bug. I'll step through the problem at a higher level than in the ticket description to illuminate what I see as the two possible solutions:

Problem:
1. We get a {{ResultSet}} that contains an array that contains timestamps.
2. We properly adjust all the timestamps in the array when we call {{getArray()}}.
3. We decide to convert that array to a "sub-{{ResultSet}}" by calling {{java.sql.Array.getResultSet()}} on the array, causing the timestamps to be double-adjusted.

It's not so much that any of your 4 example constraints were violated. Rather, it's that the process of of getting an array and then converting it to a result set involves an incorrect series of operations on the primitive ({{long}}) internal representation of the result set before it's abstracted into a {{java.sql.Timestamp}} object.

My original proposal was to set the local calendar of any {{ResultSet}} created by {{java.sql.Array.getResultSet()}} to UTC. By static analysis of the code, I can verify that this will *only* affect this particular narrow case: getting time/date/timestamp values out of an array result set, as well as the return value of {{AvaticaResultSet.getLocalCalendar()}}, which is a public method but is not part of the JDBC spec and is not used anywhere in the Avatica codebase. However, this would create a problematic edge-case where an explicit calendar is provided to get the individual timestamps out of the array's result set; they would be incorrect.

The only real solution I see now is to "de-adjust" the data in the result set during {{ArrayFactoryImpl.create()}} (in between steps 2 and 3 of the problem above), effectively undoing the first adjustment and making it so that the second adjustment (whether implicit or explicit) is correct.

> Avatica double-adjusts timestamps when calling Array.getResultSet()
> -------------------------------------------------------------------
>
>                 Key: CALCITE-5488
>                 URL: https://issues.apache.org/jira/browse/CALCITE-5488
>             Project: Calcite
>          Issue Type: Bug
>          Components: avatica
>            Reporter: Will Noble
>            Assignee: Will Noble
>            Priority: Minor
>
> If you go into {{AvaticaResultSetConversionTest}} and delete the [timeZone|https://github.com/apache/calcite-avatica/blob/dbe9b1d8c2e53474eb40cfaf5721aceca3bdb57f/core/src/test/java/org/apache/calcite/avatica/AvaticaResultSetConversionsTest.java#L1121] property for the connection, which is currently set to "GMT", several {{getString()}} and {{getArray()}} tests start to fail. The {{getString()}} failures are self-explanatory -- the expected value has become incorrect because of the time zone change -- but the {{getArray()}} failures are tricky. They're double-adjusting the timestamps:
> * Once when they call [getObject()|https://github.com/apache/calcite-avatica/blob/810acf80771310431d7ef576f3404299ebb8eaf2/core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java#L1432] (which invokes {{getTimestamp()}} with the connection's default calendar, which was previously GMT but is now the system default).
> * Then, in [ArrayImpl.equalContents()|https://github.com/apache/calcite-avatica/blob/820edf6f653607afb5a2a280a32f315aff1f64cb/core/src/main/java/org/apache/calcite/avatica/util/ArrayImpl.java#L233] the arrays are converted to result sets for traversal via [Array.getResultSet()|https://docs.oracle.com/javase/8/docs/api/java/sql/Array.html#getResultSet--] , which uses the same time zone as the "factory" result set and eventually invokes {{getObject()}} again, thus applying the time zone offset twice.
> This is a bug in the implementation of {{Array.getResultSet()}} that only manifests when the array contains timestamps and when the connection default time zone is anything besides GMT, and this is not currently covered by tests. It's easy to cause the failure, however, by changing the connection default time zone for {{AvaticaResultSetConversionTest}} to anything besides GMT or UTC.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)