dbwong commented on a change in pull request #796:
URL: https://github.com/apache/phoenix/pull/796#discussion_r488961019
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixPreparedStatement.java
##########
@@ -491,15 +517,39 @@ public void setString(int parameterIndex, String x)
throws SQLException {
@Override
public void setTime(int parameterIndex, Time x) throws SQLException {
if (x != null) { // Since Time is mutable, make a copy
- x = new Time(x.getTime());
+ String tz = this.connection.getQueryServices().getProps()
+ .get(QueryServices.DATE_FORMAT_TIMEZONE_ATTRIB, "GMT");
+ int offset = TimeZone.getTimeZone(tz).getRawOffset();
+
+ TimeZone timezone = Calendar.getInstance().getTimeZone();
+ long msFromEpochGmt = x.getTime();
+ int offsetFromUTC = timezone.getOffset(msFromEpochGmt);
+
+ Calendar gmtCal = Calendar.getInstance(TimeZone.getTimeZone(tz));
+ gmtCal.setTime(x);
+ gmtCal.add(Calendar.MILLISECOND, offsetFromUTC);
+
+ x = new Time(gmtCal.getTimeInMillis() - offset);
}
setParameter(parameterIndex, x);
}
@Override
public void setTime(int parameterIndex, Time x, Calendar cal) throws
SQLException {
if (x != null) { // Since Time is mutable, make a copy
- x = new Time(x.getTime());
+ String tz = this.connection.getQueryServices().getProps()
+ .get(QueryServices.DATE_FORMAT_TIMEZONE_ATTRIB, "GMT");
+ int offset = TimeZone.getTimeZone(tz).getRawOffset();
+
+ TimeZone timezone = Calendar.getInstance().getTimeZone();
+ long msFromEpochGmt = x.getTime();
+ int offsetFromUTC = timezone.getOffset(msFromEpochGmt);
+
+ Calendar gmtCal = Calendar.getInstance(TimeZone.getTimeZone(tz));
Review comment:
Looks like similar code again thoughts on utility function?
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/expression/LiteralExpression.java
##########
@@ -20,6 +20,7 @@
import java.io.DataInput;
import java.io.DataOutput;
import java.io.IOException;
+import java.sql.Connection;
Review comment:
nit: remove
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundDateExpression.java
##########
@@ -150,14 +153,16 @@ protected long getRoundUpAmount() {
protected long roundTime(long time) {
Review comment:
Can we add some comments on input/output? This appears like you are
converting an absolute time to a localTime for rounding which is correct.
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java
##########
@@ -331,11 +339,16 @@ public ReadOnlyProps getProps() {
int maxSizeBytes = this.services.getProps().getInt(
QueryServices.MAX_MUTATION_SIZE_BYTES_ATTRIB,
QueryServicesOptions.DEFAULT_MAX_MUTATION_SIZE_BYTES);
- String timeZoneID =
this.services.getProps().get(QueryServices.DATE_FORMAT_TIMEZONE_ATTRIB,
- DateUtil.DEFAULT_TIME_ZONE_ID);
- Format dateFormat = DateUtil.getDateFormatter(datePattern, timeZoneID);
- Format timeFormat = DateUtil.getDateFormatter(timePattern, timeZoneID);
- Format timestampFormat = DateUtil.getDateFormatter(timestampPattern,
timeZoneID);
+
+ String timeZoneID = this.services.getProps().get(
+ QueryServices.DATE_FORMAT_TIMEZONE_ATTRIB,
DateUtil.DEFAULT_TIME_ZONE_ID);
Review comment:
I saw the discussion that jdbc differs from phoenix on default timezone
being local vs phoenix's gmt, we should likely document in the code a bit here.
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundJodaDateExpression.java
##########
@@ -48,9 +49,10 @@ public boolean evaluate(Tuple tuple, ImmutableBytesWritable
ptr) {
}
PDataType dataType = getDataType();
long time = dataType.getCodec().decodeLong(ptr,
children.get(0).getSortOrder());
- DateTime dt = new DateTime(time,ISOChronology.getInstanceUTC());
+ int offset = Calendar.getInstance().getTimeZone().getRawOffset();
+ DateTime dt = new DateTime(time +
offset,ISOChronology.getInstanceUTC());
long value = roundDateTime(dt);
- Date d = new Date(value);
+ Date d = new Date(value - offset);
Review comment:
In RoundDate expression offset was handled inside of the rounding
function. I'd either recommend the same pattern or maybe handling this in a
utility class?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
