Skip to content

Commit

Permalink
Map Oracle date to Trino timestamp(0) type
Browse files Browse the repository at this point in the history 
This change fixes peformance regression
due to 77cfd97. TO_TIMESTAMP function with
date column led to full scan in Oracle side
when it's indexed column.
  • Loading branch information
@ebyhr
ebyhr committed Jan 20, 2022
1 parent 84b210a commit c0afeb5
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 40 deletions.
6 changes: 3 additions & 3 deletions docs/src/main/sphinx/connector/oracle.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ Trino data type mapping:
- ``VARBINARY``
-
* - ``DATE``
- ``TIMESTAMP``
- ``TIMESTAMP(0)``
- See :ref:`datetime mapping`
* - ``TIMESTAMP(p)``
- ``TIMESTAMP``
Expand Down Expand Up @@ -282,8 +282,8 @@ Mapping datetime types
Selecting a timestamp with fractional second precision (``p``) greater than 3
truncates the fractional seconds to three digits instead of rounding it.

Oracle ``DATE`` type may store hours, minutes, and seconds, so it is mapped
to Trino ``TIMESTAMP``.
Oracle ``DATE`` type stores hours, minutes, and seconds, so it is mapped
to Trino ``TIMESTAMP(0)``.

.. warning::

Expand Down
70 changes: 53 additions & 17 deletions plugin/trino-oracle/src/main/java/io/trino/plugin/oracle/OracleClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import java.util.Optional;
import java.util.Set;

import static com.google.common.base.Verify.verify;
import static io.airlift.slice.Slices.utf8Slice;
import static io.airlift.slice.Slices.wrappedBuffer;
import static io.trino.plugin.jdbc.JdbcErrorCode.JDBC_ERROR;
Expand Down Expand Up @@ -96,6 +97,7 @@
import static io.trino.spi.type.RealType.REAL;
import static io.trino.spi.type.SmallintType.SMALLINT;
import static io.trino.spi.type.TimestampType.TIMESTAMP_MILLIS;
import static io.trino.spi.type.TimestampType.TIMESTAMP_SECONDS;
import static io.trino.spi.type.TimestampWithTimeZoneType.TIMESTAMP_TZ_MILLIS;
import static io.trino.spi.type.Timestamps.MICROSECONDS_PER_MILLISECOND;
import static io.trino.spi.type.Timestamps.MICROSECONDS_PER_SECOND;
Expand Down Expand Up @@ -132,7 +134,8 @@ public class OracleClient
private static final int PRECISION_OF_UNSPECIFIED_NUMBER = 127;

private static final DateTimeFormatter DATE_FORMATTER = DateTimeFormatter.ofPattern("uuuu-MM-dd");
private static final DateTimeFormatter TIMESTAMP_FORMATTER = DateTimeFormatter.ofPattern("uuuu-MM-dd HH:mm:ss.SSS");
private static final DateTimeFormatter TIMESTAMP_SECONDS_FORMATTER = DateTimeFormatter.ofPattern("uuuu-MM-dd HH:mm:ss");
private static final DateTimeFormatter TIMESTAMP_MILLIS_FORMATTER = DateTimeFormatter.ofPattern("uuuu-MM-dd HH:mm:ss.SSS");

private static final Set<String> INTERNAL_SCHEMAS = ImmutableSet.<String>builder()
.add("ctxsys")
Expand Down Expand Up @@ -161,7 +164,7 @@ public class OracleClient
.put(DOUBLE, WriteMapping.doubleMapping("binary_double", oracleDoubleWriteFunction()))
.put(REAL, WriteMapping.longMapping("binary_float", oracleRealWriteFunction()))
.put(VARBINARY, WriteMapping.sliceMapping("blob", varbinaryWriteFunction()))
.put(DATE, WriteMapping.longMapping("date", oracleDateWriteFunction()))
.put(DATE, WriteMapping.longMapping("date", trinoDateToOracleDateWriteFunction()))
.put(TIMESTAMP_TZ_MILLIS, WriteMapping.longMapping("timestamp(3) with time zone", oracleTimestampWithTimeZoneWriteFunction()))
.buildOrThrow();

Expand Down Expand Up @@ -254,11 +257,22 @@ public void renameSchema(ConnectorSession session, String schemaName, String new
@Override
public Optional<ColumnMapping> toColumnMapping(ConnectorSession session, Connection connection, JdbcTypeHandle typeHandle)
{
String jdbcTypeName = typeHandle.getJdbcTypeName()
.orElseThrow(() -> new TrinoException(JDBC_ERROR, "Type name is missing: " + typeHandle));

Optional<ColumnMapping> mappingToVarchar = getForcedMappingToVarchar(typeHandle);
if (mappingToVarchar.isPresent()) {
return mappingToVarchar;
}

if (jdbcTypeName.equalsIgnoreCase("date")) {
return Optional.of(ColumnMapping.longMapping(
TIMESTAMP_SECONDS,
oracleTimestampReadFunction(),
trinoTimestampToOracleDateWriteFunction(),
FULL_PUSHDOWN));
}

switch (typeHandle.getJdbcType()) {
case Types.SMALLINT:
return Optional.of(ColumnMapping.longMapping(
Expand Down Expand Up @@ -350,9 +364,12 @@ else if (precision > Decimals.MAX_PRECISION || actualPrecision <= 0) {
varbinaryWriteFunction(),
DISABLE_PUSHDOWN));

// This mapping covers both DATE and TIMESTAMP, as Oracle's DATE has second precision.
case OracleTypes.TIMESTAMP:
return Optional.of(oracleTimestampColumnMapping());
return Optional.of(ColumnMapping.longMapping(
TIMESTAMP_MILLIS,
oracleTimestampReadFunction(),
trinoTimestampToOracleTimestampWriteFunction(),
FULL_PUSHDOWN));
case OracleTypes.TIMESTAMPTZ:
return Optional.of(oracleTimestampWithTimeZoneColumnMapping());
}
Expand All @@ -368,7 +385,7 @@ protected boolean isSupportedJoinCondition(JdbcJoinCondition joinCondition)
return joinCondition.getOperator() != JoinCondition.Operator.IS_DISTINCT_FROM;
}

public static LongWriteFunction oracleDateWriteFunction()
public static LongWriteFunction trinoDateToOracleDateWriteFunction()
{
return new LongWriteFunction() {
@Override
Expand All @@ -388,7 +405,30 @@ public void set(PreparedStatement statement, int index, long value)
};
}

public static LongWriteFunction oracleTimestampWriteFunction()
private static LongWriteFunction trinoTimestampToOracleDateWriteFunction()
{
return new LongWriteFunction() {
@Override
public String getBindExpression()
{
// Oracle's DATE stores year, month, day, hour, minute, seconds, but not second fraction
return "TO_DATE(?, 'SYYYY-MM-DD HH24:MI:SS')";
}

@Override
public void set(PreparedStatement statement, int index, long value)
throws SQLException
{
long epochSecond = floorDiv(value, MICROSECONDS_PER_SECOND);
int microsOfSecond = floorMod(value, MICROSECONDS_PER_SECOND);
verify(microsOfSecond == 0, "Micros of second must be zero: '%s'", value);
LocalDateTime localDateTime = LocalDateTime.ofEpochSecond(epochSecond, 0, ZoneOffset.UTC);
statement.setString(index, TIMESTAMP_SECONDS_FORMATTER.format(localDateTime));
}
};
}

public static LongWriteFunction trinoTimestampToOracleTimestampWriteFunction()
{
return new LongWriteFunction() {
@Override
Expand All @@ -404,20 +444,11 @@ public void set(PreparedStatement statement, int index, long utcMillis)
long epochSecond = floorDiv(utcMillis, MICROSECONDS_PER_SECOND);
int nanoFraction = floorMod(utcMillis, MICROSECONDS_PER_SECOND) * NANOSECONDS_PER_MICROSECOND;
LocalDateTime localDateTime = LocalDateTime.ofEpochSecond(epochSecond, nanoFraction, ZoneOffset.UTC);
statement.setString(index, TIMESTAMP_FORMATTER.format(localDateTime));
statement.setString(index, TIMESTAMP_MILLIS_FORMATTER.format(localDateTime));
}
};
}

public static ColumnMapping oracleTimestampColumnMapping()
{
return ColumnMapping.longMapping(
TIMESTAMP_MILLIS,
oracleTimestampReadFunction(),
oracleTimestampWriteFunction(),
FULL_PUSHDOWN);
}

private static LongReadFunction oracleTimestampReadFunction()
{
return (resultSet, columnIndex) -> {
Expand Down Expand Up @@ -507,8 +538,13 @@ public WriteMapping toWriteMapping(ConnectorSession session, Type type)
}
return WriteMapping.objectMapping(dataType, longDecimalWriteFunction((DecimalType) type));
}
if (type.equals(TIMESTAMP_SECONDS)) {
// Specify 'date' instead of 'timestamp(0)' to propagate the type in case of CTAS from date columns
// Oracle date stores year, month, day, hour, minute, seconds, but not second fraction
return WriteMapping.longMapping("date", trinoTimestampToOracleDateWriteFunction());
}
if (type.equals(TIMESTAMP_MILLIS)) {
return WriteMapping.longMapping("timestamp(3)", oracleTimestampWriteFunction());
return WriteMapping.longMapping("timestamp(3)", trinoTimestampToOracleTimestampWriteFunction());
}
WriteMapping writeMapping = WRITE_MAPPINGS.get(type);
if (writeMapping != null) {
Expand Down
36 changes: 19 additions & 17 deletions plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/AbstractTestOracleTypeMapping.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import static io.trino.spi.type.TimeZoneKey.UTC_KEY;
import static io.trino.spi.type.TimeZoneKey.getTimeZoneKey;
import static io.trino.spi.type.TimestampType.TIMESTAMP_MILLIS;
import static io.trino.spi.type.TimestampType.TIMESTAMP_SECONDS;
import static io.trino.spi.type.TimestampWithTimeZoneType.TIMESTAMP_TZ_MILLIS;
import static io.trino.spi.type.VarbinaryType.VARBINARY;
import static io.trino.spi.type.VarcharType.createUnboundedVarcharType;
Expand Down Expand Up @@ -658,25 +659,25 @@ public void testDate()

SqlDataTypeTest dateTests = SqlDataTypeTest.create()
// min value in Oracle
.addRoundTrip("DATE", "DATE '-4712-01-01'", TIMESTAMP_MILLIS, "TIMESTAMP '-4712-01-01 00:00:00.000'")
.addRoundTrip("DATE", "DATE '-0001-01-01'", TIMESTAMP_MILLIS, "TIMESTAMP '-0001-01-01 00:00:00.000'")
.addRoundTrip("DATE", "DATE '0001-01-01'", TIMESTAMP_MILLIS, "TIMESTAMP '0001-01-01 00:00:00.000'")
.addRoundTrip("DATE", "DATE '-4712-01-01'", TIMESTAMP_SECONDS, "TIMESTAMP '-4712-01-01 00:00:00'")
.addRoundTrip("DATE", "DATE '-0001-01-01'", TIMESTAMP_SECONDS, "TIMESTAMP '-0001-01-01 00:00:00'")
.addRoundTrip("DATE", "DATE '0001-01-01'", TIMESTAMP_SECONDS, "TIMESTAMP '0001-01-01 00:00:00'")
// day before and after julian->gregorian calendar switch
.addRoundTrip("DATE", "DATE '1582-10-04'", TIMESTAMP_MILLIS, "TIMESTAMP '1582-10-04 00:00:00.000'")
.addRoundTrip("DATE", "DATE '1582-10-15'", TIMESTAMP_MILLIS, "TIMESTAMP '1582-10-15 00:00:00.000'")
.addRoundTrip("DATE", "DATE '1582-10-04'", TIMESTAMP_SECONDS, "TIMESTAMP '1582-10-04 00:00:00'")
.addRoundTrip("DATE", "DATE '1582-10-15'", TIMESTAMP_SECONDS, "TIMESTAMP '1582-10-15 00:00:00'")
// before epoch
.addRoundTrip("DATE", "DATE '1952-04-03'", TIMESTAMP_MILLIS, "TIMESTAMP '1952-04-03 00:00:00.000'")
.addRoundTrip("DATE", "DATE '1970-01-01'", TIMESTAMP_MILLIS, "TIMESTAMP '1970-01-01 00:00:00.000'")
.addRoundTrip("DATE", "DATE '1970-02-03'", TIMESTAMP_MILLIS, "TIMESTAMP '1970-02-03 00:00:00.000'")
.addRoundTrip("DATE", "DATE '1952-04-03'", TIMESTAMP_SECONDS, "TIMESTAMP '1952-04-03 00:00:00'")
.addRoundTrip("DATE", "DATE '1970-01-01'", TIMESTAMP_SECONDS, "TIMESTAMP '1970-01-01 00:00:00'")
.addRoundTrip("DATE", "DATE '1970-02-03'", TIMESTAMP_SECONDS, "TIMESTAMP '1970-02-03 00:00:00'")
// summer on northern hemisphere (possible DST)
.addRoundTrip("DATE", "DATE '2017-07-01'", TIMESTAMP_MILLIS, "TIMESTAMP '2017-07-01 00:00:00.000'")
.addRoundTrip("DATE", "DATE '2017-07-01'", TIMESTAMP_SECONDS, "TIMESTAMP '2017-07-01 00:00:00'")
// winter on northern hemisphere
// (possible DST on southern hemisphere)
.addRoundTrip("DATE", "DATE '2017-01-01'", TIMESTAMP_MILLIS, "TIMESTAMP '2017-01-01 00:00:00.000'")
.addRoundTrip("DATE", "DATE '1983-04-01'", TIMESTAMP_MILLIS, "TIMESTAMP '1983-04-01 00:00:00.000'")
.addRoundTrip("DATE", "DATE '1983-10-01'", TIMESTAMP_MILLIS, "TIMESTAMP '1983-10-01 00:00:00.000'")
.addRoundTrip("DATE", "DATE '2017-01-01'", TIMESTAMP_SECONDS, "TIMESTAMP '2017-01-01 00:00:00'")
.addRoundTrip("DATE", "DATE '1983-04-01'", TIMESTAMP_SECONDS, "TIMESTAMP '1983-04-01 00:00:00'")
.addRoundTrip("DATE", "DATE '1983-10-01'", TIMESTAMP_SECONDS, "TIMESTAMP '1983-10-01 00:00:00'")
// max value in Oracle
.addRoundTrip("DATE", "DATE '9999-12-31'", TIMESTAMP_MILLIS, "TIMESTAMP '9999-12-31 00:00:00.000'");
.addRoundTrip("DATE", "DATE '9999-12-31'", TIMESTAMP_SECONDS, "TIMESTAMP '9999-12-31 00:00:00'");

for (String timeZoneId : ImmutableList.of(UTC_KEY.getId(), ZoneId.systemDefault().getId(), ZoneId.of("Europe/Vilnius").getId())) {
Session session = Session.builder(getSession())
Expand All @@ -694,7 +695,7 @@ public void testJulianGregorianDate()
// Oracle TO_DATE function returns +10 days during julian and gregorian calendar switch
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_julian_dt", "(ts date)")) {
assertUpdate(format("INSERT INTO %s VALUES (DATE '1582-10-05')", table.getName()), 1);
assertQuery("SELECT * FROM " + table.getName(), "VALUES TIMESTAMP '1582-10-15 00:00:00.000'");
assertQuery("SELECT * FROM " + table.getName(), "VALUES TIMESTAMP '1582-10-15 00:00:00'");
}
}

Expand All @@ -708,9 +709,10 @@ public void testUnsupportedDate()
assertQueryFails(
format("INSERT INTO %s VALUES (DATE '0000-01-01')", table.getName()),
"\\QFailed to insert data: ORA-01841: (full) year must be between -4713 and +9999, and not be 0\n");
// The error message sounds invalid date format in the connector, but it's no problem as the max year is 9999 in Oracle
assertQueryFails(
format("INSERT INTO %s VALUES (DATE '10000-01-01')", table.getName()),
"\\QFailed to insert data: ORA-01862: the numeric value does not match the length of the format item\n");
"\\QFailed to insert data: ORA-01861: literal does not match format string\n");
}
}

Expand Down Expand Up @@ -751,10 +753,10 @@ public void testTimestamp(ZoneId sessionZone)
@Test
public void testJulianGregorianTimestamp()
{
// Oracle TO_TIMESTAMP function returns +10 days during julian and gregorian calendar switch
// Oracle TO_DATE function returns +10 days during julian and gregorian calendar switch
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_julian_ts", "(ts date)")) {
assertUpdate(format("INSERT INTO %s VALUES (timestamp '1582-10-05')", table.getName()), 1);
assertQuery("SELECT * FROM " + table.getName(), "VALUES TIMESTAMP '1582-10-15 00:00:00.000'");
assertQuery("SELECT * FROM " + table.getName(), "VALUES TIMESTAMP '1582-10-15 00:00:00'");
}
}

Expand Down
6 changes: 3 additions & 3 deletions plugin/trino-oracle/src/test/java/io/trino/plugin/oracle/BaseOracleConnectorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public void testShowColumns()
.row("custkey", "decimal(19,0)", "", "")
.row("orderstatus", "varchar(1)", "", "")
.row("totalprice", "double", "", "")
.row("orderdate", "timestamp(3)", "", "")
.row("orderdate", "timestamp(0)", "", "")
.row("orderpriority", "varchar(15)", "", "")
.row("clerk", "varchar(15)", "", "")
.row("shippriority", "decimal(10,0)", "", "")
Expand Down Expand Up @@ -196,7 +196,7 @@ public void testDescribeTable()
.row("custkey", "decimal(19,0)", "", "")
.row("orderstatus", "varchar(1)", "", "")
.row("totalprice", "double", "", "")
.row("orderdate", "timestamp(3)", "", "")
.row("orderdate", "timestamp(0)", "", "")
.row("orderpriority", "varchar(15)", "", "")
.row("clerk", "varchar(15)", "", "")
.row("shippriority", "decimal(10,0)", "", "")
Expand All @@ -217,7 +217,7 @@ public void testShowCreateTable()
" custkey decimal(19, 0),\n" +
" orderstatus varchar(1),\n" +
" totalprice double,\n" +
" orderdate timestamp(3),\n" +
" orderdate timestamp(0),\n" +
" orderpriority varchar(15),\n" +
" clerk varchar(15),\n" +
" shippriority decimal(10, 0),\n" +
Expand Down

0 comments on commit c0afeb5

Please sign in to comment.