You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2022/09/14 14:03:43 UTC
[GitHub] [phoenix] richardantal opened a new pull request, #1501: PHOENIX-5422 Use Java8 DateTime APIs instead of joda-time APIs
richardantal opened a new pull request, #1501:
URL: https://github.com/apache/phoenix/pull/1501
Change-Id: Icb0e89060528930282c139af1c7254cc8d36e054
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] richardantal commented on a diff in pull request #1501: PHOENIX-5422 Use Java8 DateTime APIs instead of joda-time APIs
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] richardantal commented on a diff in pull request #1501: PHOENIX-5422 Use Java8 DateTime APIs instead of joda-time APIs
Posted by GitBox <gi...@apache.org>.
richardantal commented on code in PR #1501:
URL: https://github.com/apache/phoenix/pull/1501#discussion_r984341298
##########
phoenix-core/src/main/java/org/apache/phoenix/util/chrono/GJChronology.java:
##########
@@ -0,0 +1,467 @@
+/*
+ * Copyright (c) 2007-present, Stephen Colebourne & Michael Nascimento Santos
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * * Neither the name of JSR-310 nor the names of its contributors
+ * may be used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.apache.phoenix.util.chrono;
+
+import org.threeten.extra.chrono.JulianChronology;
+import org.threeten.extra.chrono.JulianEra;
+
+import java.io.Serializable;
+import java.time.Clock;
+import java.time.DateTimeException;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.ZoneId;
+import java.time.chrono.AbstractChronology;
+import java.time.chrono.ChronoLocalDateTime;
+import java.time.chrono.ChronoZonedDateTime;
+import java.time.chrono.Chronology;
+import java.time.chrono.Era;
+import java.time.chrono.IsoChronology;
+import java.time.format.ResolverStyle;
+import java.time.temporal.ChronoField;
+import java.time.temporal.TemporalAccessor;
+import java.time.temporal.TemporalField;
+import java.time.temporal.ValueRange;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * The GregorianJulian calendar system.
+ * <p>
+ * The GregorianJulian calendar system follows the rules of the Julian calendar
+ * until 1582 and the rules of the Gregorian (ISO) calendar since then.
+ * The Julian differs from the Gregorian only in terms of the leap year rule.
+ * <p>
+ * The Julian and Gregorian calendar systems are linked to Rome and the Vatican
+ * with the Julian preceding the Gregorian. The Gregorian was introduced to
+ * handle the drift of the seasons through the year due to the inaccurate
+ * Julian leap year rules. When first introduced by the Vatican in 1582,
+ * the cutover resulted in a "gap" of 10 days.
+ * <p>
+ * This chronology implements the proleptic Julian calendar system followed by
+ * the proleptic Gregorian calendar system (identical to the ISO calendar system).
+ * <p>
+ * This class implements a calendar where January 1st is the start of the year.
+ * The history of the start of the year is complex and using the current standard
+ * is the most consistent.
+ * <p>
+ * The eras of this calendar system are defined by {@link JulianEra} to avoid unnecessary duplication.
+ * <p>
+ * The fields are defined as follows:
+ * <ul>
+ * <li>era - There are two eras, the current 'Anno Domini' (AD) and the previous era 'Before Christ' (BC).
+ * <li>year-of-era - The year-of-era for the current era increases uniformly from the epoch at year one.
+ * For the previous era the year increases from one as time goes backwards.
+ * <li>proleptic-year - The proleptic year is the same as the year-of-era for the
+ * current era. For the previous era, years have zero, then negative values.
+ * <li>month-of-year - There are 12 months in a year, numbered from 1 to 12.
+ * <li>day-of-month - There are between 28 and 31 days in each month, numbered from 1 to 31.
+ * Months 4, 6, 9 and 11 have 30 days, Months 1, 3, 5, 7, 8, 10 and 12 have 31 days.
+ * Month 2 has 28 days, or 29 in a leap year.
+ * The cutover October 4, 1582 (Julian) is followed by October 15, 1582 (Gregorian)
+ * <li>day-of-year - There are 365 days in a standard year and 366 in a leap year.
+ * The days are numbered from 1 to 365 or 1 to 366.
+ * The cutover year 1582 has values from 1 to 356 and a length of 356 days.
+ * </ul>
+ *
+ * <h3>Implementation Requirements</h3>
+ * This class is immutable and thread-safe.
+ */
+public final class GJChronology
Review Comment:
This class was created based on BritishCutoverChronology in threeten-extra
There are minor changes in the CUTOVER, CUTOVER_DAYS, comments.
Added test for the cutover dates.
##########
phoenix-core/src/main/java/org/apache/phoenix/util/DateUtil.java:
##########
@@ -72,13 +74,26 @@ public class DateUtil {
public static final String DEFAULT_TIMESTAMP_FORMAT = DEFAULT_MS_DATE_FORMAT;
public static final Format DEFAULT_TIMESTAMP_FORMATTER = DEFAULT_MS_DATE_FORMATTER;
- private static final DateTimeFormatter JULIAN_DATE_TIME_FORMATTER = new DateTimeFormatterBuilder()
- .append(ISODateTimeFormat.dateParser())
- .appendOptional(new DateTimeFormatterBuilder()
- .appendLiteral(' ').toParser())
- .appendOptional(new DateTimeFormatterBuilder()
- .append(ISODateTimeFormat.timeParser()).toParser())
- .toFormatter().withChronology(GJChronology.getInstanceUTC());
+ private static final DateTimeFormatter ISO_DATE_TIME_FORMATTER = new DateTimeFormatterBuilder()
Review Comment:
added implementation for gregorianJulian Chronology
##########
phoenix-core/src/main/java/org/apache/phoenix/util/chrono/AbstractDateCopy.java:
##########
@@ -0,0 +1,398 @@
+/*
+ * Copyright (c) 2007-present, Stephen Colebourne & Michael Nascimento Santos
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * * Neither the name of JSR-310 nor the names of its contributors
+ * may be used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.apache.phoenix.util.chrono;
+
+import static java.time.temporal.ChronoField.ALIGNED_DAY_OF_WEEK_IN_MONTH;
+import static java.time.temporal.ChronoField.ALIGNED_DAY_OF_WEEK_IN_YEAR;
+import static java.time.temporal.ChronoField.ALIGNED_WEEK_OF_MONTH;
+import static java.time.temporal.ChronoField.ALIGNED_WEEK_OF_YEAR;
+import static java.time.temporal.ChronoField.ERA;
+import static java.time.temporal.ChronoField.YEAR;
+
+import java.time.chrono.ChronoLocalDate;
+import java.time.chrono.ChronoPeriod;
+import java.time.temporal.ChronoField;
+import java.time.temporal.ChronoUnit;
+import java.time.temporal.TemporalField;
+import java.time.temporal.TemporalUnit;
+import java.time.temporal.UnsupportedTemporalTypeException;
+import java.time.temporal.ValueRange;
+
+/**
+ * An abstract date based on a year, month and day.
+ *
+ * <h3>Implementation Requirements</h3>
+ * Implementations must be immutable and thread-safe.
+ */
+abstract class AbstractDateCopy
Review Comment:
This class has been copied from threeten-extra
##########
phoenix-core/src/main/java/org/apache/phoenix/util/chrono/JulianChronologyCopy.java:
##########
@@ -0,0 +1,395 @@
+/*
+ * Copyright (c) 2007-present, Stephen Colebourne & Michael Nascimento Santos
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * * Neither the name of JSR-310 nor the names of its contributors
+ * may be used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.apache.phoenix.util.chrono;
+
+import org.threeten.extra.chrono.JulianEra;
+
+import java.io.Serializable;
+import java.time.Clock;
+import java.time.DateTimeException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.chrono.AbstractChronology;
+import java.time.chrono.ChronoLocalDateTime;
+import java.time.chrono.ChronoZonedDateTime;
+import java.time.chrono.Chronology;
+import java.time.chrono.Era;
+import java.time.format.ResolverStyle;
+import java.time.temporal.ChronoField;
+import java.time.temporal.TemporalAccessor;
+import java.time.temporal.TemporalField;
+import java.time.temporal.ValueRange;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * The Julian calendar system.
+ * <p>
+ * This chronology defines the rules of the proleptic Julian calendar system.
+ * This calendar system is the forerunner to the modern Gregorian and ISO calendars.
+ * The Julian differs from the Gregorian only in terms of the leap year rule.
+ * Dates are aligned such that {@code 0001-01-01 (Julian)} is {@code 0000-12-30 (ISO)}.
+ * <p>
+ * This class is proleptic. It implements Julian rules to the entire time-line.
+ * <p>
+ * This class implements a calendar where January 1st is the start of the year.
+ * The history of the start of the year is complex and using the current standard
+ * is the most consistent.
+ * <p>
+ * The fields are defined as follows:
+ * <ul>
+ * <li>era - There are two eras, the current 'Anno Domini' (AD) and the previous era 'Before Christ' (BC).
+ * <li>year-of-era - The year-of-era for the current era increases uniformly from the epoch at year one.
+ * For the previous era the year increases from one as time goes backwards.
+ * <li>proleptic-year - The proleptic year is the same as the year-of-era for the
+ * current era. For the previous era, years have zero, then negative values.
+ * <li>month-of-year - There are 12 months in a Julian year, numbered from 1 to 12.
+ * <li>day-of-month - There are between 28 and 31 days in each Julian month, numbered from 1 to 31.
+ * Months 4, 6, 9 and 11 have 30 days, Months 1, 3, 5, 7, 8, 10 and 12 have 31 days.
+ * Month 2 has 28 days, or 29 in a leap year.
+ * <li>day-of-year - There are 365 days in a standard Julian year and 366 in a leap year.
+ * The days are numbered from 1 to 365 or 1 to 366.
+ * <li>leap-year - Leap years occur every 4 years.
+ * </ul>
+ *
+ * <h3>Implementation Requirements</h3>
+ * This class is immutable and thread-safe.
+ */
+public final class JulianChronologyCopy extends AbstractChronology implements Serializable {
Review Comment:
This class has been copied from threeten-extra
##########
phoenix-core/src/main/java/org/apache/phoenix/util/chrono/GJDate.java:
##########
@@ -0,0 +1,553 @@
+/*
+ * Copyright (c) 2007-present, Stephen Colebourne & Michael Nascimento Santos
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * * Neither the name of JSR-310 nor the names of its contributors
+ * may be used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.apache.phoenix.util.chrono;
+
+import org.threeten.extra.chrono.JulianEra;
+
+import static org.apache.phoenix.util.chrono.GJChronology.CUTOVER;
+import static org.apache.phoenix.util.chrono.GJChronology.CUTOVER_DAYS;
+import static org.apache.phoenix.util.chrono.GJChronology.CUTOVER_YEAR;
+
+import java.io.Serializable;
+import java.time.Clock;
+import java.time.DateTimeException;
+import java.time.LocalDate;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.time.chrono.ChronoLocalDate;
+import java.time.chrono.ChronoLocalDateTime;
+import java.time.chrono.ChronoPeriod;
+import java.time.temporal.ChronoField;
+import java.time.temporal.Temporal;
+import java.time.temporal.TemporalAccessor;
+import java.time.temporal.TemporalAdjuster;
+import java.time.temporal.TemporalAmount;
+import java.time.temporal.TemporalField;
+import java.time.temporal.TemporalQueries;
+import java.time.temporal.TemporalQuery;
+import java.time.temporal.TemporalUnit;
+import java.time.temporal.ValueRange;
+import java.util.Objects;
+
+/**
+ * A date in the GregorianJulian calendar system.
+ * <p>
+ * This date operates using the {@linkplain GJChronology GregorianJulian calendar}.
+ *
+ * <h3>Implementation Requirements</h3>
+ * This class is immutable and thread-safe.
+ * <p>
+ * This class must be treated as a value type. Do not synchronize, rely on the
+ * identity hash code or use the distinction between equals() and ==.
+ */
+public final class GJDate
Review Comment:
This class was created based on BritishCutoverDate in threeten-extra
There are minor changes in the lengthOfMonth, lengthOfYear, comments.
Added test for the cutover dates.
##########
phoenix-core/src/main/java/org/apache/phoenix/util/chrono/JulianDateCopy.java:
##########
@@ -0,0 +1,467 @@
+/*
+ * Copyright (c) 2007-present, Stephen Colebourne & Michael Nascimento Santos
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * * Neither the name of JSR-310 nor the names of its contributors
+ * may be used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.apache.phoenix.util.chrono;
+
+import org.threeten.extra.chrono.JulianEra;
+
+import static java.time.temporal.ChronoField.DAY_OF_MONTH;
+import static java.time.temporal.ChronoField.DAY_OF_YEAR;
+import static java.time.temporal.ChronoField.EPOCH_DAY;
+import static java.time.temporal.ChronoField.MONTH_OF_YEAR;
+import static java.time.temporal.ChronoField.YEAR;
+
+import java.io.Serializable;
+import java.time.Clock;
+import java.time.DateTimeException;
+import java.time.LocalDate;
+import java.time.LocalTime;
+import java.time.Month;
+import java.time.ZoneId;
+import java.time.chrono.ChronoLocalDate;
+import java.time.chrono.ChronoLocalDateTime;
+import java.time.chrono.ChronoPeriod;
+import java.time.temporal.ChronoField;
+import java.time.temporal.Temporal;
+import java.time.temporal.TemporalAccessor;
+import java.time.temporal.TemporalAdjuster;
+import java.time.temporal.TemporalAmount;
+import java.time.temporal.TemporalField;
+import java.time.temporal.TemporalQuery;
+import java.time.temporal.TemporalUnit;
+import java.time.temporal.ValueRange;
+
+/**
+ * A date in the Julian calendar system.
+ * <p>
+ * This date operates using the {@linkplain JulianChronologyCopy Julian calendar}.
+ * This calendar system is the forerunner to the modern Gregorian and ISO calendars.
+ * The Julian differs from the Gregorian only in terms of the leap year rule.
+ * Dates are aligned such that {@code 0001-01-01 (Julian)} is {@code 0000-12-30 (ISO)}.
+ *
+ * <h3>Implementation Requirements</h3>
+ * This class is immutable and thread-safe.
+ * <p>
+ * This class must be treated as a value type. Do not synchronize, rely on the
+ * identity hash code or use the distinction between equals() and ==.
+ */
+public final class JulianDateCopy
Review Comment:
This class has been copied from threeten-extra
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] richardantal commented on a diff in pull request #1501: PHOENIX-5422 Use Java8 DateTime APIs instead of joda-time APIs
Posted by GitBox <gi...@apache.org>.
richardantal commented on code in PR #1501:
URL: https://github.com/apache/phoenix/pull/1501#discussion_r973606255
##########
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 have to check this, to be honest I've done this patch more than a year ago but we because of 4.x branch we delayed it until that is dropped and we can use java8.
https://phoenix.apache.org/language/functions.html#floor page is generated with an old phoenix, that is not really up to date as far as I know.
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] richardantal commented on a diff in pull request #1501: PHOENIX-5422 Use Java8 DateTime APIs instead of joda-time APIs
Posted by GitBox <gi...@apache.org>.
richardantal commented on code in PR #1501:
URL: https://github.com/apache/phoenix/pull/1501#discussion_r973606768
##########
phoenix-core/src/main/java/org/apache/phoenix/util/DateUtil.java:
##########
@@ -72,13 +74,26 @@ public class DateUtil {
public static final String DEFAULT_TIMESTAMP_FORMAT = DEFAULT_MS_DATE_FORMAT;
public static final Format DEFAULT_TIMESTAMP_FORMATTER = DEFAULT_MS_DATE_FORMATTER;
- private static final DateTimeFormatter JULIAN_DATE_TIME_FORMATTER = new DateTimeFormatterBuilder()
- .append(ISODateTimeFormat.dateParser())
- .appendOptional(new DateTimeFormatterBuilder()
- .appendLiteral(' ').toParser())
- .appendOptional(new DateTimeFormatterBuilder()
- .append(ISODateTimeFormat.timeParser()).toParser())
- .toFormatter().withChronology(GJChronology.getInstanceUTC());
+ private static final DateTimeFormatter ISO_DATE_TIME_FORMATTER = new DateTimeFormatterBuilder()
Review Comment:
This is good question.
If we use IsoChronology we would face PHOENIX-6486 again.
I found that BritishCutoverChronology was the closest to the gregorian julian chronology that we have and all the tests passed. I have to check the dates between 1582 and 1752, and maybe implement a Gregorian chronology for java time.
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] richardantal commented on a diff in pull request #1501: PHOENIX-5422 Use Java8 DateTime APIs instead of joda-time APIs
Posted by GitBox <gi...@apache.org>.
richardantal commented on code in PR #1501:
URL: https://github.com/apache/phoenix/pull/1501#discussion_r973605680
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/DateTimeIT.java:
##########
@@ -577,7 +577,7 @@ public void testYearFunctionDate() throws SQLException {
assertEquals(2006, callYearFunction("\"YEAR\"(TO_DATE('2006-12-13'))"));
- assertEquals(2015, callYearFunction("\"YEAR\"(TO_DATE('2015-W05'))"));
+ assertEquals(2015, callYearFunction("\"YEAR\"(TO_DATE('2015-W05-1'))"));
Review Comment:
Yes, Joda time supports weird date formats as well.
2015-W05-1 means the fifth week of 2015 and One digit for the day-of-week. The value run from Monday (1) to Sunday (7). I guess joda time defaulted the day value to 1.
I added a separate ISO_WEEK_DATE_TIME_FORMATTER to parse dates like this.
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] gjacoby126 commented on pull request #1501: PHOENIX-5422 Use Java8 DateTime APIs instead of joda-time APIs
Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on PR #1501:
URL: https://github.com/apache/phoenix/pull/1501#issuecomment-1263985132
@richardantal - I see that I missed a bunch of discussion back in PHOENIX-6486, but I'm not yet convinced that having an equivalent to GJChronology / BritishCutoverChronology is the right design.
It looks like some other databases, such as MySQL, just use the Gregorian calendar for all time and require users to make adjustments. (See https://dev.mysql.com/doc/refman/8.0/en/mysql-calendar.html).
This pushes potentially annoying conversions on our users, but I think the user is the only one who can actually do this correctly. That's because the exact calendar in use on a particular date is going to be use-case and culturally-specific -- there's no one right answer.
GJChronology is going to be "right" for areas that were part of the Spanish Empire and a few other countries in 1582 and wrong for everyone else. (Even some countries, like France, that adopted the Gregorian calendar in 1582 didn't do it on the same _day_ that GJChronology assumes.)
BritishCutoverChronology is going to be "right" for areas that were a part of the British Empire in 1752, but nowhere else. (See https://en.wikipedia.org/wiki/List_of_adoption_dates_of_the_Gregorian_calendar_by_country)
Perhaps we can make it configurable, but I don't see a way to use any single calendar other than the ISO calendar, as MySQL does, as a one-size-fits-all.
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] richardantal commented on pull request #1501: PHOENIX-5422 Use Java8 DateTime APIs instead of joda-time APIs
Posted by GitBox <gi...@apache.org>.
richardantal commented on PR #1501:
URL: https://github.com/apache/phoenix/pull/1501#issuecomment-1250107715
Thank you @gjacoby126 for the review and for the insightful questions.
I will be on holiday for a week and a half but after that I'll give further updates.
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #1501: PHOENIX-5422 Use Java8 DateTime APIs instead of joda-time APIs
Posted by GitBox <gi...@apache.org>.
stoty commented on PR #1501:
URL: https://github.com/apache/phoenix/pull/1501#issuecomment-1264953605
I agree with you @gjacoby126, but let me provide some context:
In my experience the Chronology issue has less to with real historical use cases than with being able to handle synthetic "minus infinity" constants like '01-01-01' or '1000-01-01' consistently.
The reason we stuck with GJChornology at the last change was because that's what Phoenix has effectively been using already, based on the default behaviour of java.util.Calendar, and this was the easiest way to be consistent on the read and write paths (and date functions) and between Joda and j.u.Calendar, which is still extensively used in Phoenix code.
GJ is also the only Chronology that works without surprises with naive java Date code, i.e. when you do
```
// assume we're in UTC
j.u.t.Date d = new j.u.d(1000,1,1,1,1,1);
// write d to Phoenix with a PreparedStatement
// read d back
assertEquals("1000-01-01 01:01:01", rs.getString("d"))
```
It is only going to succeed with GJChronology (or similar, like BritishCutoverChronology).
Consequently, as long as Phoenix uses j.u.Date / Calender with the default Chronology, it is the only Calendar that can guarantee internal consistency in Phoenix.
The above is also true for views on HBase tables where custom timestamps are naively generated.
In a nutshell, GJChronology is the default chronology for j.u.Calendar/Date, while ISOChronology is the default for Joda and java.time, and we need to handle this discrepancy somehow.
At the same time, using a cutover chronology is both against the standard, and has a performance cost, so I support using ISOChronology as the default, and basically pruging all j.u.Date/Calendar code from Phoenix.
However, for backwards compatibility both with existing code and existing data, I believe that we also ought to make Chronology configurable.
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] gjacoby126 commented on a diff in pull request #1501: PHOENIX-5422 Use Java8 DateTime APIs instead of joda-time APIs
Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on code in PR #1501:
URL: https://github.com/apache/phoenix/pull/1501#discussion_r973244729
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/ExtendedQueryExecIT.java:
##########
@@ -122,7 +122,7 @@ public void testDateFunctions() throws Exception {
// // expected
// }
- queryDateArg = "a_date >= TO_DATE('1970-1-2 23:59:59') and a_date <= TO_DATE('1970-1-3 0:0:1')";
+ queryDateArg = "a_date >= TO_DATE('1970-01-02 23:59:59') and a_date <= TO_DATE('1970-01-03 00:00:01')";
Review Comment:
Likewise, Joda can parse 1 digit hour / minute / second fields and Java 8 can't?
##########
phoenix-core/src/main/java/org/apache/phoenix/expression/function/DayOfWeekFunction.java:
##########
@@ -70,8 +71,11 @@ public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
return true;
}
long dateTime = inputCodec.decodeLong(ptr, arg.getSortOrder());
- DateTime jodaDT = new DateTime(dateTime, GJChronology.getInstanceUTC());
- int day = jodaDT.getDayOfWeek();
+ Date dt = new Date(dateTime);
+ Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
+ calendar.setTime(dt);
+ int day = calendar.get(Calendar.DAY_OF_WEEK) - 1;
Review Comment:
Could we please have a comment here to explain the logic here?
##########
phoenix-core/src/main/java/org/apache/phoenix/util/DateUtil.java:
##########
@@ -72,13 +74,26 @@ public class DateUtil {
public static final String DEFAULT_TIMESTAMP_FORMAT = DEFAULT_MS_DATE_FORMAT;
public static final Format DEFAULT_TIMESTAMP_FORMATTER = DEFAULT_MS_DATE_FORMATTER;
- private static final DateTimeFormatter JULIAN_DATE_TIME_FORMATTER = new DateTimeFormatterBuilder()
- .append(ISODateTimeFormat.dateParser())
- .appendOptional(new DateTimeFormatterBuilder()
- .appendLiteral(' ').toParser())
- .appendOptional(new DateTimeFormatterBuilder()
- .append(ISODateTimeFormat.timeParser()).toParser())
- .toFormatter().withChronology(GJChronology.getInstanceUTC());
+ private static final DateTimeFormatter ISO_DATE_TIME_FORMATTER = new DateTimeFormatterBuilder()
Review Comment:
Shouldn't ISO_DATE_TIME_FORMATTER and ISO_LOCAL_DATE_TIME_FORMATTER use IsoChronology?
Note that using BritishCutoverChronology is _not_ the same as Joda's GJChronology, because GJChronology cuts over between Julian and Gregorian time in 1582 and BritishCutoverChronology cuts over in 1752.
##########
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:
So Joda will parse a slightly malformed date like 2015-6-05 but Java 8 won't?
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/ToDateFunctionIT.java:
##########
@@ -82,7 +82,7 @@ public void testToDate_Default() throws SQLException {
try {
callToDateFunction("TO_DATE('2015-01-27T16:17:57+00:00')");
callToDateFunction("TO_DATE('2015-01-27T16:17:57Z')");
- callToDateFunction("TO_DATE('2015-W05')");
+// callToDateFunction("TO_DATE('2015-W05')");
Review Comment:
If this isn't a valid test case anymore, we can just delete it.
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/DateTimeIT.java:
##########
@@ -577,7 +577,7 @@ public void testYearFunctionDate() throws SQLException {
assertEquals(2006, callYearFunction("\"YEAR\"(TO_DATE('2006-12-13'))"));
- assertEquals(2015, callYearFunction("\"YEAR\"(TO_DATE('2015-W05'))"));
+ assertEquals(2015, callYearFunction("\"YEAR\"(TO_DATE('2015-W05-1'))"));
Review Comment:
I assume this is a case that Java 8 handles differently than Joda. I've never seen this date syntax -- what does it mean?
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/ToDateFunctionIT.java:
##########
@@ -101,7 +101,7 @@ public void testToTime_Default() throws SQLException {
try {
callToTimeFunction("TO_TIME('2015-01-27T16:17:57+00:00')");
callToTimeFunction("TO_TIME('2015-01-27T16:17:57Z')");
- callToTimeFunction("TO_TIME('2015-W05')");
+// callToTimeFunction("TO_TIME('2015-W05')");
Review Comment:
Likewise here can be deleted.
##########
phoenix-core/src/main/java/org/apache/phoenix/cache/TimezoneGetter.java:
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you maynot use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicablelaw or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.cache;
+
+import java.nio.ByteBuffer;
+import java.time.ZoneId;
+import java.time.zone.ZoneRulesException;
+import java.util.TimeZone;
+import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.phoenix.schema.IllegalDataException;
+
+public class TimezoneGetter {
+
+ /**
+ * Returns java's TimeZone instance from cache or create new instance and cache it.
Review Comment:
The comment here mentions that we're caching the TimeZone as we did with Joda, but we're not. (Java 8 might or might not be doing that internally, but that's an implementation detail not part of the interface.)
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/ToDateFunctionIT.java:
##########
@@ -120,7 +120,7 @@ public void testToTimestamp_Default() throws SQLException {
try {
callToTimestampFunction("TO_TIMESTAMP('2015-01-27T16:17:57+00:00')");
callToTimestampFunction("TO_TIMESTAMP('2015-01-27T16:17:57Z')");
- callToTimestampFunction("TO_TIMESTAMP('2015-W05')");
+// callToTimestampFunction("TO_TIMESTAMP('2015-W05')");
Review Comment:
Likewise here can be deleted.
##########
phoenix-core/src/test/java/org/apache/phoenix/util/DateUtilTest.java:
##########
@@ -183,10 +183,11 @@ public void testParseTimestamp_tooLargeNanos() {
DateUtil.parseTimestamp("1970-01-01 00:00:10.9999999999");
}
- @Test(expected=IllegalDataException.class)
- public void testParseTimestamp_missingNanos() {
- DateUtil.parseTimestamp("1970-01-01 00:00:10.");
- }
+// @Test(expected=IllegalDataException.class)
+// public void testParseTimestamp_missingNanos() {
Review Comment:
If the test is no longer valid then we can just delete it.
##########
phoenix-core/src/main/java/org/apache/phoenix/expression/function/FloorMonthExpression.java:
##########
@@ -37,8 +40,9 @@ public FloorMonthExpression(List<Expression> children) {
}
@Override
- public long roundDateTime(DateTime datetime) {
- return datetime.monthOfYear().roundFloorCopy().getMillis();
+ public long roundDateTime(Date dateTime) {
+ return DateUtils.truncate(dateTime, Calendar.MONTH).getTime()
+ + Calendar.getInstance().getTimeZone().getRawOffset();
Review Comment:
How do we know that the default Calendar is in the right time zone?
##########
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:
This can't be done using the floor function in DateTimeUtils and Calendar.WEEK_OF_YEAR?
As a side note, I see in the docs in https://phoenix.apache.org/language/functions.html#floor that we only support DAY, HOUR, MINUTE, SECOND and MILLISECOND, so I'm surprised this was here...
##########
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:
Likewise, we can't use DateUtils.round here?
##########
phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilYearExpression.java:
##########
@@ -37,8 +39,9 @@ public CeilYearExpression(List<Expression> children) {
}
@Override
- public long roundDateTime(DateTime dateTime) {
- return dateTime.year().roundCeilingCopy().getMillis();
+ public long roundDateTime(Date dateTime) {
+ return DateUtils.ceiling(dateTime, Calendar.YEAR).getTime() +
+ Calendar.getInstance().getTimeZone().getRawOffset();
Review Comment:
How do we know that the default Calendar is in the right time zone?
##########
phoenix-core/src/main/java/org/apache/phoenix/expression/function/CeilWeekExpression.java:
##########
@@ -37,8 +40,7 @@ public CeilWeekExpression(List<Expression> children) {
}
@Override
- public long roundDateTime(DateTime dateTime) {
- return dateTime.weekOfWeekyear().roundCeilingCopy().getMillis();
+ public long roundDateTime(Date dateTime) {
Review Comment:
Nothing in DateUtils will do this?
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] richardantal commented on a diff in pull request #1501: PHOENIX-5422 Use Java8 DateTime APIs instead of joda-time APIs
Posted by GitBox <gi...@apache.org>.
richardantal commented on code in PR #1501:
URL: https://github.com/apache/phoenix/pull/1501#discussion_r973606336
##########
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:
I will check it again.
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] richardantal commented on a diff in pull request #1501: PHOENIX-5422 Use Java8 DateTime APIs instead of joda-time APIs
Posted by GitBox <gi...@apache.org>.
richardantal commented on code in PR #1501:
URL: https://github.com/apache/phoenix/pull/1501#discussion_r973605959
##########
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:
Yes, you are right.
ISO_LOCAL_DATE in Java time requires the month of year and the day of month to be 2 digits, but for joda 1 digit was fine.
--
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-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org