richardantal commented on code in PR #1501: URL: https://github.com/apache/phoenix/pull/1501#discussion_r982342208
########## phoenix-core/src/it/java/org/apache/phoenix/end2end/DefaultColumnValueIT.java: ########## @@ -593,7 +593,7 @@ public void testDefaultExpression() throws Exception { + "c3 DECIMAL DEFAULT TO_NUMBER('" + DEFAULT_CURRENCY_SYMBOL + "123.33', '\u00A4###.##')," + "c4 VARCHAR DEFAULT 'AB' || 'CD'," + "c5 CHAR(5) DEFAULT 'E' || 'F'," - + "c6 INTEGER DEFAULT \"MONTH\"(TO_TIMESTAMP('2015-6-05'))" + + "c6 INTEGER DEFAULT \"MONTH\"(TO_TIMESTAMP('2015-06-05'))" Review Comment: witth .parseLenient() we can support 1 digit months and days. ########## phoenix-core/src/main/java/org/apache/phoenix/expression/function/RoundWeekExpression.java: ########## @@ -17,25 +17,36 @@ */ package org.apache.phoenix.expression.function; +import java.util.Calendar; +import java.util.Date; import java.util.List; import org.apache.phoenix.expression.Expression; -import org.joda.time.DateTime; + +import static java.lang.Math.abs; /** * - * Rounds off the given {@link DateTime} to the nearest Monday. + * Rounds off the given {@link Date} to the nearest Monday. */ -public class RoundWeekExpression extends RoundJodaDateExpression { +public class RoundWeekExpression extends RoundJavaDateExpression { - public RoundWeekExpression(){} + public RoundWeekExpression() {} public RoundWeekExpression(List<Expression> children) { super(children); } @Override - public long roundDateTime(DateTime dateTime) { - return dateTime.weekOfWeekyear().roundHalfEvenCopy().getMillis(); + public long roundDateTime(Date dateTime) { + long nextMonday = new CeilWeekExpression().roundDateTime(dateTime); + long prevMonday = new FloorWeekExpression().roundDateTime(dateTime); Review Comment: DateUtils, doesn't support rounding to weeks. ########## phoenix-core/src/main/java/org/apache/phoenix/expression/function/FloorWeekExpression.java: ########## @@ -36,8 +37,7 @@ public FloorWeekExpression(List<Expression> children) { } @Override - public long roundDateTime(DateTime datetime) { - return datetime.weekOfWeekyear().roundFloorCopy().getMillis(); + public long roundDateTime(Date dateTime) { + return dateTime.getTime() - dateTime.getTime() % week - correction; Review Comment: I've tried to replace it with `return DateUtils.truncate(dateTime, Calendar.DAY_OF_YEAR).getTime();` but truncating the date to weeks is not supported `java.lang.IllegalArgumentException: The field 6 is not supported` https://github.com/apache/commons-lang/blob/49444165a4138338b867112d140b86cb10a76566/src/main/java/org/apache/commons/lang3/time/DateUtils.java#L902-L1038 -- 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: issues-unsubscr...@phoenix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org