[
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)
