wnob commented on code in PR #205:
URL: https://github.com/apache/calcite-avatica/pull/205#discussion_r1081852921
##########
core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java:
##########
@@ -238,12 +244,18 @@ protected Accessor createAccessor(ColumnMetaData
columnMetaData,
public abstract boolean next();
- /** Accesses a timestamp value as a string.
+ /**
+ * Accesses a timestamp value as a string.
* The timestamp is in SQL format (e.g. "2013-09-22 22:30:32"),
- * not Java format ("2013-09-22 22:30:32.123"). */
+ * not Java format ("2013-09-22 22:30:32.123").
+ *
+ * <p>Note that, when a TIMESTAMP is adjusted to a calendar, the offset is
subtracted.
+ * Here, on the other hand, to adjust the string to the calendar (which only
happens for type
+ * TIMESTAMP WITH LOCAL TIME ZONE), the offset is added. These are meant to
be inverse operations.
Review Comment:
This is a private method, and all existing invocations of it pass it a null
`Calendar`. If you open Avatica at head in IntelliJ, it suggests inlining the
constant parameter.
With my change, a `TIMESTAMP WITH LOCAL TIME ZONE` getter might pass it a
non-null `Calendar`, and that's the only time it could be non-null.
Note that both a regular `TIMESTAMP` and a `TIMESTAMP WITH LOCAL TIME ZONE`
are represented in JDBC as `TIMESTAMP` (since there is no JDBC type for the
latter. I chose to re-use the existing timestamp getter with the addition of
this new `fixedInstant` parameter since it seemed like a straightforward
addition.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
