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

Various precision for Oracle timestamp #17934

Merged
merged 1 commit into from
Jul 19, 2023
Merged

Various precision for Oracle timestamp #17934

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/src/main/sphinx/connector/oracle.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ Trino data type mapping:
- ``TIMESTAMP(0)``
- See :ref:`datetime mapping`
* - ``TIMESTAMP(p)``
- ``TIMESTAMP``
- ``TIMESTAMP(p)``
vlad-lyutenko marked this conversation as resolved.
Show resolved Hide resolved
- See :ref:`datetime mapping`
* - ``TIMESTAMP(p) WITH TIME ZONE``
- ``TIMESTAMP WITH TIME ZONE``
Expand Down Expand Up @@ -300,8 +300,8 @@ For Oracle ``NUMBER`` (without precision and scale), you can change
Mapping datetime types
^^^^^^^^^^^^^^^^^^^^^^

Selecting a timestamp with fractional second precision (``p``) greater than 3
Praveen2112 marked this conversation as resolved.
Show resolved Hide resolved
truncates the fractional seconds to three digits instead of rounding it.
Writing a timestamp with fractional second precision (``p``) greater than 9
rounds the fractional seconds to nine digits.

Oracle ``DATE`` type stores hours, minutes, and seconds, so it is mapped
to Trino ``TIMESTAMP(0)``.
Expand Down
153 changes: 127 additions & 26 deletions plugin/trino-oracle/src/main/java/io/trino/plugin/oracle/OracleClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import io.trino.plugin.jdbc.JdbcTypeHandle;
import io.trino.plugin.jdbc.LongReadFunction;
import io.trino.plugin.jdbc.LongWriteFunction;
import io.trino.plugin.jdbc.ObjectReadFunction;
ebyhr marked this conversation as resolved.
Show resolved Hide resolved
import io.trino.plugin.jdbc.ObjectWriteFunction;
vlad-lyutenko marked this conversation as resolved.
Show resolved Hide resolved
import io.trino.plugin.jdbc.QueryBuilder;
import io.trino.plugin.jdbc.RemoteTableName;
import io.trino.plugin.jdbc.SliceWriteFunction;
Expand Down Expand Up @@ -63,6 +65,8 @@
import io.trino.spi.type.CharType;
import io.trino.spi.type.DecimalType;
import io.trino.spi.type.Decimals;
import io.trino.spi.type.LongTimestamp;
import io.trino.spi.type.TimestampType;
import io.trino.spi.type.Type;
import io.trino.spi.type.VarcharType;
import oracle.jdbc.OraclePreparedStatement;
Expand All @@ -80,6 +84,8 @@
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
import java.time.temporal.ChronoField;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand All @@ -97,13 +103,17 @@
import static io.trino.plugin.jdbc.StandardColumnMappings.bigintColumnMapping;
import static io.trino.plugin.jdbc.StandardColumnMappings.bigintWriteFunction;
import static io.trino.plugin.jdbc.StandardColumnMappings.charReadFunction;
import static io.trino.plugin.jdbc.StandardColumnMappings.fromLongTrinoTimestamp;
import static io.trino.plugin.jdbc.StandardColumnMappings.fromTrinoTimestamp;
import static io.trino.plugin.jdbc.StandardColumnMappings.integerWriteFunction;
import static io.trino.plugin.jdbc.StandardColumnMappings.longDecimalReadFunction;
import static io.trino.plugin.jdbc.StandardColumnMappings.longDecimalWriteFunction;
import static io.trino.plugin.jdbc.StandardColumnMappings.shortDecimalReadFunction;
import static io.trino.plugin.jdbc.StandardColumnMappings.shortDecimalWriteFunction;
import static io.trino.plugin.jdbc.StandardColumnMappings.smallintWriteFunction;
import static io.trino.plugin.jdbc.StandardColumnMappings.tinyintWriteFunction;
import static io.trino.plugin.jdbc.StandardColumnMappings.toLongTrinoTimestamp;
import static io.trino.plugin.jdbc.StandardColumnMappings.toTrinoTimestamp;
import static io.trino.plugin.jdbc.StandardColumnMappings.varbinaryWriteFunction;
import static io.trino.plugin.jdbc.StandardColumnMappings.varcharWriteFunction;
import static io.trino.plugin.jdbc.TypeHandlingJdbcSessionProperties.getUnsupportedTypeHandling;
Expand All @@ -123,12 +133,11 @@
import static io.trino.spi.type.IntegerType.INTEGER;
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.MAX_SHORT_PRECISION;
import static io.trino.spi.type.TimestampType.TIMESTAMP_SECONDS;
import static io.trino.spi.type.TimestampType.createTimestampType;
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;
import static io.trino.spi.type.Timestamps.NANOSECONDS_PER_MICROSECOND;
import static io.trino.spi.type.TinyintType.TINYINT;
import static io.trino.spi.type.VarbinaryType.VARBINARY;
import static io.trino.spi.type.VarcharType.createUnboundedVarcharType;
Expand All @@ -150,6 +159,7 @@ public class OracleClient
{
public static final int ORACLE_MAX_LIST_EXPRESSIONS = 1000;

private static final int MAX_ORACLE_TIMESTAMP_PRECISION = 9;
private static final int MAX_BYTES_PER_CHAR = 4;

private static final int ORACLE_VARCHAR2_MAX_BYTES = 4000;
Expand All @@ -164,7 +174,13 @@ public class OracleClient

private static final DateTimeFormatter DATE_FORMATTER = DateTimeFormatter.ofPattern("uuuu-MM-dd");
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 DateTimeFormatter TIMESTAMP_NANO_OPTIONAL_FORMATTER = new DateTimeFormatterBuilder()
.appendPattern("uuuu-MM-dd HH:mm:ss")
.optionalStart()
.appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true)
.optionalEnd()
.toFormatter();

private static final Set<String> INTERNAL_SCHEMAS = ImmutableSet.<String>builder()
.add("ctxsys")
Expand Down Expand Up @@ -385,7 +401,7 @@ public Optional<ColumnMapping> toColumnMapping(ConnectorSession session, Connect
if (jdbcTypeName.equalsIgnoreCase("date")) {
return Optional.of(ColumnMapping.longMapping(
TIMESTAMP_SECONDS,
oracleTimestampReadFunction(),
oracleTimestampReadFunction(TIMESTAMP_SECONDS),
trinoTimestampToOracleDateWriteFunction(),
FULL_PUSHDOWN));
}
Expand Down Expand Up @@ -482,11 +498,8 @@ else if (precision > Decimals.MAX_PRECISION || actualPrecision <= 0) {
DISABLE_PUSHDOWN));

case OracleTypes.TIMESTAMP:
return Optional.of(ColumnMapping.longMapping(
TIMESTAMP_MILLIS,
oracleTimestampReadFunction(),
trinoTimestampToOracleTimestampWriteFunction(),
FULL_PUSHDOWN));
int timestampPrecision = typeHandle.getRequiredDecimalDigits();
return Optional.of(oracleTimestampColumnMapping(createTimestampType(timestampPrecision)));
case OracleTypes.TIMESTAMPTZ:
return Optional.of(oracleTimestampWithTimeZoneColumnMapping());
}
Expand All @@ -496,6 +509,22 @@ else if (precision > Decimals.MAX_PRECISION || actualPrecision <= 0) {
return Optional.empty();
}

private static ColumnMapping oracleTimestampColumnMapping(TimestampType timestampType)
{
if (timestampType.isShort()) {
return ColumnMapping.longMapping(
timestampType,
oracleTimestampReadFunction(timestampType),
oracleTimestampWriteFunction(timestampType),
FULL_PUSHDOWN);
}
return ColumnMapping.objectMapping(
timestampType,
oracleLongTimestampReadFunction(timestampType),
oracleLongTimestampWriteFunction(timestampType),
FULL_PUSHDOWN);
}

@Override
public Optional<JdbcExpression> implementAggregation(ConnectorSession session, AggregateFunction aggregate, Map<String, ColumnHandle> assignments)
{
Expand Down Expand Up @@ -584,24 +613,57 @@ public void setNull(PreparedStatement statement, int index)
};
}

public static LongWriteFunction trinoTimestampToOracleTimestampWriteFunction()
private static ObjectWriteFunction oracleLongTimestampWriteFunction(TimestampType timestampType)
{
int precision = timestampType.getPrecision();
verifyLongTimestampPrecision(timestampType);

return new ObjectWriteFunction() {
@Override
public Class<?> getJavaType()
{
return LongTimestamp.class;
}

@Override
public void set(PreparedStatement statement, int index, Object value)
throws SQLException
{
LocalDateTime timestamp = fromLongTrinoTimestamp((LongTimestamp) value, precision);
statement.setString(index, TIMESTAMP_NANO_OPTIONAL_FORMATTER.format(timestamp));
}

@Override
public String getBindExpression()
{
return getOracleBindExpression(precision);
}

@Override
public void setNull(PreparedStatement statement, int index)
throws SQLException
{
statement.setNull(index, Types.VARCHAR);
}
};
}

private static LongWriteFunction oracleTimestampWriteFunction(TimestampType timestampType)
{
return new LongWriteFunction()
{
@Override
public String getBindExpression()
{
return "TO_TIMESTAMP(?, 'SYYYY-MM-DD HH24:MI:SS.FF')";
return getOracleBindExpression(timestampType.getPrecision());
}

@Override
public void set(PreparedStatement statement, int index, long utcMillis)
public void set(PreparedStatement statement, int index, long epochMicros)
throws SQLException
{
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_MILLIS_FORMATTER.format(localDateTime));
LocalDateTime timestamp = fromTrinoTimestamp(epochMicros);
statement.setString(index, TIMESTAMP_NANO_OPTIONAL_FORMATTER.format(timestamp));
}

@Override
Expand All @@ -613,18 +675,52 @@ public void setNull(PreparedStatement statement, int index)
};
}

private static LongReadFunction oracleTimestampReadFunction()
private static String getOracleBindExpression(int precision)
{
if (precision == 0) {
return "TO_TIMESTAMP(?, 'SYYYY-MM-DD HH24:MI:SS')";
}
if (precision <= 2) {
return "TO_TIMESTAMP(?, 'SYYYY-MM-DD HH24:MI:SS.FF')";
}

return format("TO_TIMESTAMP(?, 'SYYYY-MM-DD HH24:MI:SS.FF%d')", precision);
}

private static LongReadFunction oracleTimestampReadFunction(TimestampType timestampType)
{
return (resultSet, columnIndex) -> {
LocalDateTime timestamp = resultSet.getObject(columnIndex, LocalDateTime.class);
// Adjust years when the value is B.C. dates because Oracle returns +1 year unless converting to string in their server side
if (timestamp.getYear() <= 0) {
timestamp = timestamp.minusYears(1);
}
return timestamp.toInstant(ZoneOffset.UTC).toEpochMilli() * MICROSECONDS_PER_MILLISECOND;
return toTrinoTimestamp(timestampType, timestamp);
};
}

private static ObjectReadFunction oracleLongTimestampReadFunction(TimestampType timestampType)
{
verifyLongTimestampPrecision(timestampType);
return ObjectReadFunction.of(
LongTimestamp.class,
(resultSet, columnIndex) -> {
LocalDateTime timestamp = resultSet.getObject(columnIndex, LocalDateTime.class);
// Adjust years when the value is B.C. dates because Oracle returns +1 year unless converting to string in their server side
if (timestamp.getYear() <= 0) {
timestamp = timestamp.minusYears(1);
}
return toLongTrinoTimestamp(timestampType, timestamp);
});
}

private static void verifyLongTimestampPrecision(TimestampType timestampType)
{
int precision = timestampType.getPrecision();
checkArgument(precision > MAX_SHORT_PRECISION && precision <= MAX_ORACLE_TIMESTAMP_PRECISION,
"Precision is out of range: %s", precision);
}

public static ColumnMapping oracleTimestampWithTimeZoneColumnMapping()
{
return ColumnMapping.longMapping(
Expand Down Expand Up @@ -705,13 +801,18 @@ public WriteMapping toWriteMapping(ConnectorSession session, Type type)
}
return WriteMapping.objectMapping(dataType, longDecimalWriteFunction(decimalType));
}
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)", trinoTimestampToOracleTimestampWriteFunction());
if (type instanceof TimestampType timestampType) {
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());
}
int precision = min(timestampType.getPrecision(), MAX_ORACLE_TIMESTAMP_PRECISION);
String dataType = format("timestamp(%d)", precision);
if (timestampType.isShort()) {
return WriteMapping.longMapping(dataType, oracleTimestampWriteFunction(timestampType));
}
return WriteMapping.objectMapping(dataType, oracleLongTimestampWriteFunction(createTimestampType(precision)));
Copy link
Member

Choose a reason for hiding this comment

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

Calling createTimestampType looks redundant to me. oracleLongTimestampWriteFunction uses only precision. Same for oracleTimestampWriteFunction

Copy link
Contributor Author

@vlad-lyutenko vlad-lyutenko Jul 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

want to be consistent with oracleLongTimestampReadFunction which requires timestampType or need to refactor toTrinoTimestamp from base-jdbc

}
WriteMapping writeMapping = WRITE_MAPPINGS.get(type);
if (writeMapping != null) {
Expand Down
Loading