Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deprecate Joda library for Presto Teradata functions #15537

Merged
merged 1 commit into from Mar 8, 2021
Merged

Deprecate Joda library for Presto Teradata functions #15537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions presto-teradata-functions/pom.xml
View file
Open in desktop
Expand Up @@ -22,13 +22,13 @@
</dependency>

<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<groupId>com.facebook.airlift</groupId>
<artifactId>log</artifactId>
</dependency>

<dependency>
<groupId>joda-time</groupId>
<artifactId>joda-time</artifactId>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</dependency>

<dependency>
Expand Down
48 changes: 36 additions & 12 deletions ...functions/src/main/java/com/facebook/presto/teradata/functions/TeradataDateFunctions.java
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@
package com.facebook.presto.teradata.functions;

import com.facebook.airlift.concurrent.ThreadLocalCache;
import com.facebook.airlift.log.Logger;
import com.facebook.presto.common.function.SqlFunctionProperties;
import com.facebook.presto.common.type.StandardTypes;
import com.facebook.presto.common.type.TimeZoneKey;
Expand All @@ -22,10 +23,13 @@
import com.facebook.presto.spi.function.ScalarFunction;
import com.facebook.presto.spi.function.SqlType;
import io.airlift.slice.Slice;
import org.joda.time.DateTimeZone;
import org.joda.time.chrono.ISOChronology;
import org.joda.time.format.DateTimeFormatter;

import java.time.Instant;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.chrono.IsoChronology;
import java.time.format.DateTimeFormatter;
import java.time.zone.ZoneRulesException;
import java.util.Locale;

import static com.facebook.presto.common.type.DateTimeEncoding.unpackMillisUtc;
Expand All @@ -35,6 +39,8 @@
import static com.facebook.presto.common.type.TimeZoneKey.getTimeZoneKeys;
import static com.facebook.presto.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR;
import static com.facebook.presto.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT;
import static com.facebook.presto.teradata.functions.dateformat.DateFormatParser.Mode.FORMATTER;
import static com.facebook.presto.teradata.functions.dateformat.DateFormatParser.Mode.PARSER;
import static com.facebook.presto.teradata.functions.dateformat.DateFormatParser.createDateTimeFormatter;
import static com.google.common.base.Throwables.throwIfInstanceOf;
import static io.airlift.slice.Slices.utf8Slice;
Expand All @@ -43,15 +49,31 @@

public final class TeradataDateFunctions
{
// Separate DateTimeFormatter instance caches (for formatting and parsing) in order to keep support the following use cases:
// 1. Do not require leading zero for parsing two position date fields (MM, DD, HH, HH24, MI, SS)
// e.g. allow "to_timestamp('1988/4/8 2:3:4','yyyy/mm/dd hh24:mi:ss')"
// 2. Always add leading zero for formatting single valued two position date fields (MM, DD, HH, HH24, MI, SS)
// e.g. evaluate "to_char(TIMESTAMP '1988-4-8 2:3:4','yyyy/mm/dd hh24:mi:ss')" to "1988/04/08 02:03:04"

private static final ThreadLocalCache<Slice, DateTimeFormatter> DATETIME_PARSER_CACHE =
rk13 marked this conversation as resolved.
Show resolved Hide resolved
new ThreadLocalCache<>(100, format -> createDateTimeFormatter(format.toStringUtf8(), PARSER));

private static final ThreadLocalCache<Slice, DateTimeFormatter> DATETIME_FORMATTER_CACHE =
new ThreadLocalCache<>(100, format -> createDateTimeFormatter(format.toStringUtf8()));
new ThreadLocalCache<>(100, format -> createDateTimeFormatter(format.toStringUtf8(), FORMATTER));

private static final ISOChronology[] CHRONOLOGIES = new ISOChronology[MAX_TIME_ZONE_KEY + 1];
private static final Logger LOG = Logger.get(TeradataDateFunctions.class);

private static final ZoneId[] ZONE_IDS = new ZoneId[MAX_TIME_ZONE_KEY + 1];

static {
for (TimeZoneKey timeZoneKey : getTimeZoneKeys()) {
DateTimeZone dateTimeZone = DateTimeZone.forID(timeZoneKey.getId());
CHRONOLOGIES[timeZoneKey.getKey()] = ISOChronology.getInstance(dateTimeZone);
try {
ZONE_IDS[timeZoneKey.getKey()] = ZoneId.of(timeZoneKey.getId());
}
catch (ZoneRulesException ex) {
// Ignore this exception so that older JRE versions that might not support newer time-zone identifiers do not fail
LOG.error(ex, "Failed to obtain an instance of ZoneId for %s, ignoring this exception", timeZoneKey.getId());
}
}
}

Expand All @@ -68,10 +90,11 @@ public static Slice toChar(
@SqlType(StandardTypes.VARCHAR) Slice formatString)
{
DateTimeFormatter formatter = DATETIME_FORMATTER_CACHE.get(formatString)
.withChronology(CHRONOLOGIES[unpackZoneKey(timestampWithTimeZone).getKey()])
.withChronology(IsoChronology.INSTANCE)
.withZone(ZONE_IDS[unpackZoneKey(timestampWithTimeZone).getKey()])
.withLocale(properties.getSessionLocale());

return utf8Slice(formatter.print(unpackMillisUtc(timestampWithTimeZone)));
return utf8Slice(formatter.format(Instant.ofEpochMilli((unpackMillisUtc(timestampWithTimeZone)))));
}

@Description("Converts a string to a DATE data type")
Expand Down Expand Up @@ -112,12 +135,13 @@ private static long parseMillis(SqlFunctionProperties properties, Slice dateTime

private static long parseMillis(TimeZoneKey timeZoneKey, Locale locale, Slice dateTime, Slice formatString)
{
DateTimeFormatter formatter = DATETIME_FORMATTER_CACHE.get(formatString)
.withChronology(CHRONOLOGIES[timeZoneKey.getKey()])
DateTimeFormatter formatter = DATETIME_PARSER_CACHE.get(formatString)
.withChronology(IsoChronology.INSTANCE)
.withZone(ZONE_IDS[timeZoneKey.getKey()])
.withLocale(locale);

try {
return formatter.parseMillis(dateTime.toString(UTF_8));
return ZonedDateTime.parse(dateTime.toString(UTF_8), formatter).toInstant().toEpochMilli();
}
catch (IllegalArgumentException e) {
throw new PrestoException(INVALID_FUNCTION_ARGUMENT, e);
Expand Down
73 changes: 60 additions & 13 deletions ...ons/src/main/java/com/facebook/presto/teradata/functions/dateformat/DateFormatParser.java
View file
Open in desktop
Expand Up @@ -18,50 +18,84 @@
import com.facebook.presto.teradata.functions.DateFormat;
import org.antlr.v4.runtime.ANTLRInputStream;
import org.antlr.v4.runtime.Token;
import org.joda.time.format.DateTimeFormatter;
import org.joda.time.format.DateTimeFormatterBuilder;

import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
import java.util.List;

import static com.facebook.presto.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT;
import static java.time.format.SignStyle.NOT_NEGATIVE;
import static java.time.temporal.ChronoField.AMPM_OF_DAY;
import static java.time.temporal.ChronoField.DAY_OF_MONTH;
import static java.time.temporal.ChronoField.HOUR_OF_AMPM;
import static java.time.temporal.ChronoField.HOUR_OF_DAY;
import static java.time.temporal.ChronoField.MINUTE_OF_HOUR;
import static java.time.temporal.ChronoField.MONTH_OF_YEAR;
import static java.time.temporal.ChronoField.SECOND_OF_MINUTE;
import static java.time.temporal.ChronoField.YEAR;

public class DateFormatParser
{
public enum Mode
{
// Do not require leading zero for parsing two position date fields (MM, DD, HH, HH24, MI, SS)
// E.g. "to_timestamp('1988/4/8 2:3:4','yyyy/mm/dd hh24:mi:ss')"
PARSER(1),

// Add leading zero for formatting single valued two position date fields (MM, DD, HH, HH24, MI, SS)
// E.g. "to_char(TIMESTAMP '1988-4-8 2:3:4','yyyy/mm/dd hh24:mi:ss')" evaluates to "1988/04/08 02:03:04"
FORMATTER(2);

private final int minTwoPositionFieldWidth;

public int getMinTwoPositionFieldWidth()
{
return minTwoPositionFieldWidth;
}

Mode(int value)
{
this.minTwoPositionFieldWidth = value;
}
}

private DateFormatParser()
{
}

public static DateTimeFormatter createDateTimeFormatter(String format)
public static DateTimeFormatter createDateTimeFormatter(String format, Mode mode)
{
DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
boolean formatContainsHourOfAMPM = false;
for (Token token : tokenize(format)) {
switch (token.getType()) {
case DateFormat.TEXT:
builder.appendLiteral(token.getText());
break;
case DateFormat.DD:
builder.appendDayOfMonth(2);
builder.appendValue(DAY_OF_MONTH, mode.getMinTwoPositionFieldWidth(), 2, NOT_NEGATIVE);
break;
case DateFormat.HH24:
builder.appendHourOfDay(2);
builder.appendValue(HOUR_OF_DAY, mode.getMinTwoPositionFieldWidth(), 2, NOT_NEGATIVE);
break;
case DateFormat.HH:
builder.appendHourOfHalfday(2);
builder.appendValue(HOUR_OF_AMPM, mode.getMinTwoPositionFieldWidth(), 2, NOT_NEGATIVE);
formatContainsHourOfAMPM = true;
break;
case DateFormat.MI:
builder.appendMinuteOfHour(2);
builder.appendValue(MINUTE_OF_HOUR, mode.getMinTwoPositionFieldWidth(), 2, NOT_NEGATIVE);
break;
case DateFormat.MM:
builder.appendMonthOfYear(2);
builder.appendValue(MONTH_OF_YEAR, mode.getMinTwoPositionFieldWidth(), 2, NOT_NEGATIVE);
break;
case DateFormat.SS:
builder.appendSecondOfMinute(2);
builder.appendValue(SECOND_OF_MINUTE, mode.getMinTwoPositionFieldWidth(), 2, NOT_NEGATIVE);
break;
case DateFormat.YY:
builder.appendTwoDigitYear(2050);
builder.appendValueReduced(YEAR, 2, 2, 2000);
break;
case DateFormat.YYYY:
builder.appendYear(4, 4);
builder.appendValue(YEAR, 4);
break;
case DateFormat.UNRECOGNIZED:
default:
Expand All @@ -70,9 +104,22 @@ public static DateTimeFormatter createDateTimeFormatter(String format)
String.format("Failed to tokenize string [%s] at offset [%d]", token.getText(), token.getCharPositionInLine()));
}
}

try {
return builder.toFormatter();
// Append default values(0) for time fields(HH24, HH, MI, SS) because JSR-310 does not accept bare Date value as DateTime

if (formatContainsHourOfAMPM) {
// At the moment format does not allow to include AM/PM token, thus it was never possible to specify PM hours using 'HH' token in format
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason we couldn't or shouldn't support an AM/PM token?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I do not know about any reason why we should not.
Teradata date format arguments support AM/PM actually (https://www.docs.teradata.com/r/~_sY_PYVxZzTnqKq45UXkQ/DlNXnvtMQTAeWXYLHvEarQ).
Original grammar used to tokenise input should be extended in this case
https://github.com/prestodb/presto/blob/master/presto-teradata-functions/src/main/antlr4/com/facebook/presto/teradata/functions/DateFormat.g4
It is better to be implemented in separate PR as a new feature I think.

// Keep existing behaviour by defaulting to 0(AM) for AMPM_OF_DAY if format string contains 'HH'
builder.parseDefaulting(HOUR_OF_AMPM, 0)
.parseDefaulting(AMPM_OF_DAY, 0);
}
else {
builder.parseDefaulting(HOUR_OF_DAY, 0);
}

return builder.parseDefaulting(MINUTE_OF_HOUR, 0)
.parseDefaulting(SECOND_OF_MINUTE, 0)
.toFormatter();
}
catch (UnsupportedOperationException e) {
throw new PrestoException(INVALID_FUNCTION_ARGUMENT, e);
Expand Down
12 changes: 8 additions & 4 deletions ...tions/src/test/java/com/facebook/presto/teradata/functions/TestTeradataDateFunctions.java
View file
Open in desktop
Expand Up @@ -18,11 +18,11 @@
import com.facebook.presto.common.type.SqlDate;
import com.facebook.presto.common.type.TimestampType;
import com.facebook.presto.operator.scalar.AbstractTestFunctions;
import org.joda.time.DateTime;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

import java.time.LocalDate;
import java.time.LocalDateTime;

import static com.facebook.presto.common.type.VarcharType.VARCHAR;
import static com.facebook.presto.metadata.FunctionExtractor.extractFunctions;
Expand Down Expand Up @@ -93,16 +93,20 @@ public void testYY()
assertVarchar("to_char(TIMESTAMP '1988-04-08','yy')", "88");
assertTimestamp("to_timestamp('88/04/08','yy/mm/dd')", 2088, 4, 8, 0, 0, 0);
assertDate("to_date('88/04/08','yy/mm/dd')", 2088, 4, 8);

assertTimestamp("to_timestamp('00/04/08','yy/mm/dd')", 2000, 4, 8, 0, 0, 0);
assertTimestamp("to_timestamp('50/04/08','yy/mm/dd')", 2050, 4, 8, 0, 0, 0);
assertTimestamp("to_timestamp('99/04/08','yy/mm/dd')", 2099, 4, 8, 0, 0, 0);
}

// TODO: implement this feature SWARM-355
@Test(enabled = false)
public void testDefaultValues()
{
DateTime current = new DateTime();
assertDate("to_date('1988','yyyy')", 1988, current.getMonthOfYear(), 1);
LocalDateTime current = LocalDateTime.now();
assertDate("to_date('1988','yyyy')", 1988, current.getMonthValue(), 1);
assertDate("to_date('04','mm')", current.getYear(), 4, 1);
assertDate("to_date('8','dd')", current.getYear(), current.getMonthOfYear(), 8);
assertDate("to_date('8','dd')", current.getYear(), current.getMonthValue(), 8);
}

// TODO: implement this feature SWARM-354
Expand Down
33 changes: 27 additions & 6 deletions ...src/test/java/com/facebook/presto/teradata/functions/dateformat/TestDateFormatParser.java
View file
Open in desktop
Expand Up @@ -16,12 +16,14 @@
import com.facebook.presto.spi.PrestoException;
import com.facebook.presto.teradata.functions.DateFormat;
import org.antlr.v4.runtime.Token;
import org.joda.time.DateTime;
import org.joda.time.format.DateTimeFormatter;
import org.testng.annotations.Test;

import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.stream.Collectors;

import static com.facebook.presto.teradata.functions.dateformat.DateFormatParser.Mode.FORMATTER;
import static com.facebook.presto.teradata.functions.dateformat.DateFormatParser.Mode.PARSER;
import static java.util.Arrays.asList;
import static org.testng.Assert.assertEquals;

Expand Down Expand Up @@ -54,19 +56,38 @@ public void testInvalidTokenTokenize()
@Test(expectedExceptions = PrestoException.class)
public void testInvalidTokenCreate1()
{
DateFormatParser.createDateTimeFormatter("ala");
DateFormatParser.createDateTimeFormatter("ala", FORMATTER);
}

@Test(expectedExceptions = PrestoException.class)
public void testInvalidTokenCreate2()
{
DateFormatParser.createDateTimeFormatter("yyym/mm/dd");
DateFormatParser.createDateTimeFormatter("yyym/mm/dd", FORMATTER);
}

@Test(expectedExceptions = PrestoException.class)
public void testParserInvalidTokenCreate1()
{
DateFormatParser.createDateTimeFormatter("ala", PARSER);
}

@Test(expectedExceptions = PrestoException.class)
public void testParserInvalidTokenCreate2()
{
DateFormatParser.createDateTimeFormatter("yyym/mm/dd", PARSER);
}

@Test
public void testCreateDateTimeFormatter()
{
DateTimeFormatter formatter = DateFormatParser.createDateTimeFormatter("yyyy/mm/dd");
assertEquals(formatter.parseDateTime("1988/04/08"), new DateTime(1988, 4, 8, 0, 0));
DateTimeFormatter formatter = DateFormatParser.createDateTimeFormatter("yyyy/mm/dd", FORMATTER);
assertEquals(formatter.format(LocalDateTime.of(1988, 4, 8, 0, 0)), "1988/04/08");
}

@Test
public void testCreateDateTimeParser()
{
DateTimeFormatter formatter = DateFormatParser.createDateTimeFormatter("yyyy/mm/dd", PARSER);
assertEquals(LocalDateTime.parse("1988/04/08", formatter), LocalDateTime.of(1988, 4, 8, 0, 0));
}
}