julianhyde commented on code in PR #205:
URL: https://github.com/apache/calcite-avatica/pull/205#discussion_r1081816343
##########
core/src/main/java/org/apache/calcite/avatica/AvaticaResultSet.java:
##########
@@ -77,7 +78,9 @@ public AvaticaResultSet(AvaticaStatement statement,
ResultSetMetaData resultSetMetaData,
TimeZone timeZone,
Meta.Frame firstFrame) throws SQLException {
- super(timeZone);
+ // Initialize the parent ArrayFactory with the UTC time zone because
otherwise an array of
Review Comment:
I think you just tore down a Chesterton's fence.
##########
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:
after this change the method name is no longer appropriate.
the purpose of this method is to access a SQL `TIMESTAMP` value as a string
you have added logic to access a SQL `TIMESTAMP WITH LOCAL TIME ZONE` value
as a string. and paved over the old functionality, which we still need.
--
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: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
