From 87722320dcf6cbb53c7a78bddbb77a9c1e56f124 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Thu, 3 Jan 2019 01:20:41 +0100 Subject: [PATCH 1/9] Remove unsupported types on write path in JDBC connectors These types are not supported in `com.facebook.presto.plugin.jdbc.JdbcPageSink#appendColumn`, so cannot be written. Since JDBC based connectors do not support CREATE TABLE except for CREATE TABLE AS, it makes no sense to attempt to create columns we cannot write to. Note: attempt to support these types must be well test-covered on per-database basis, so they should not be supported in a generic way anyway. --- .../com/facebook/presto/plugin/jdbc/BaseJdbcClient.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java index 7096fa0adc3f..5ea5567b94fb 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java @@ -58,10 +58,6 @@ import static com.facebook.presto.spi.type.IntegerType.INTEGER; import static com.facebook.presto.spi.type.RealType.REAL; import static com.facebook.presto.spi.type.SmallintType.SMALLINT; -import static com.facebook.presto.spi.type.TimeType.TIME; -import static com.facebook.presto.spi.type.TimeWithTimeZoneType.TIME_WITH_TIME_ZONE; -import static com.facebook.presto.spi.type.TimestampType.TIMESTAMP; -import static com.facebook.presto.spi.type.TimestampWithTimeZoneType.TIMESTAMP_WITH_TIME_ZONE; import static com.facebook.presto.spi.type.TinyintType.TINYINT; import static com.facebook.presto.spi.type.VarbinaryType.VARBINARY; import static com.facebook.presto.spi.type.Varchars.isVarcharType; @@ -88,10 +84,6 @@ public class BaseJdbcClient .put(REAL, "real") .put(VARBINARY, "varbinary") .put(DATE, "date") - .put(TIME, "time") - .put(TIME_WITH_TIME_ZONE, "time with timezone") - .put(TIMESTAMP, "timestamp") - .put(TIMESTAMP_WITH_TIME_ZONE, "timestamp with timezone") .build(); protected final String connectorId; From b0a44f233b1dff6c1fc27b5efccc29e8ba29e02e Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Thu, 3 Jan 2019 02:17:09 +0100 Subject: [PATCH 2/9] Remove unsupported types on read path in JDBC connectors No JDBC-based connector supports TIMESTAMP WITH TIME ZONE or TIME WITH TIME ZONE, so no need to support predicate push down for these types. --- .../com/facebook/presto/plugin/jdbc/QueryBuilder.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/QueryBuilder.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/QueryBuilder.java index b035fd46ab1b..6bc020313d80 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/QueryBuilder.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/QueryBuilder.java @@ -26,9 +26,7 @@ import com.facebook.presto.spi.type.RealType; import com.facebook.presto.spi.type.SmallintType; import com.facebook.presto.spi.type.TimeType; -import com.facebook.presto.spi.type.TimeWithTimeZoneType; import com.facebook.presto.spi.type.TimestampType; -import com.facebook.presto.spi.type.TimestampWithTimeZoneType; import com.facebook.presto.spi.type.TinyintType; import com.facebook.presto.spi.type.Type; import com.facebook.presto.spi.type.VarcharType; @@ -46,7 +44,6 @@ import java.util.ArrayList; import java.util.List; -import static com.facebook.presto.spi.type.DateTimeEncoding.unpackMillisUtc; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Strings.isNullOrEmpty; @@ -158,15 +155,9 @@ else if (typeAndValue.getType().equals(DateType.DATE)) { else if (typeAndValue.getType().equals(TimeType.TIME)) { statement.setTime(i + 1, new Time((long) typeAndValue.getValue())); } - else if (typeAndValue.getType().equals(TimeWithTimeZoneType.TIME_WITH_TIME_ZONE)) { - statement.setTime(i + 1, new Time(unpackMillisUtc((long) typeAndValue.getValue()))); - } else if (typeAndValue.getType().equals(TimestampType.TIMESTAMP)) { statement.setTimestamp(i + 1, new Timestamp((long) typeAndValue.getValue())); } - else if (typeAndValue.getType().equals(TimestampWithTimeZoneType.TIMESTAMP_WITH_TIME_ZONE)) { - statement.setTimestamp(i + 1, new Timestamp(unpackMillisUtc((long) typeAndValue.getValue()))); - } else if (typeAndValue.getType() instanceof VarcharType) { statement.setString(i + 1, ((Slice) typeAndValue.getValue()).toStringUtf8()); } @@ -193,9 +184,7 @@ private static boolean isAcceptedType(Type type) validType.equals(BooleanType.BOOLEAN) || validType.equals(DateType.DATE) || validType.equals(TimeType.TIME) || - validType.equals(TimeWithTimeZoneType.TIME_WITH_TIME_ZONE) || validType.equals(TimestampType.TIMESTAMP) || - validType.equals(TimestampWithTimeZoneType.TIMESTAMP_WITH_TIME_ZONE) || validType instanceof VarcharType || validType instanceof CharType; } From 2606a1161f90df63f08616d0ee7e3ba57c730fc5 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 31 Dec 2018 01:24:46 +0100 Subject: [PATCH 3/9] Add TYPE_NAME to JdbcTypeHandle --- .../presto/plugin/jdbc/BaseJdbcClient.java | 1 + .../presto/plugin/jdbc/JdbcTypeHandle.java | 16 +++++++++++-- .../plugin/jdbc/TestingJdbcTypeHandle.java | 24 +++++++++---------- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java index 5ea5567b94fb..e49e79948642 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java @@ -191,6 +191,7 @@ public List getColumns(ConnectorSession session, JdbcTableHand while (resultSet.next()) { JdbcTypeHandle typeHandle = new JdbcTypeHandle( resultSet.getInt("DATA_TYPE"), + resultSet.getString("TYPE_NAME"), resultSet.getInt("COLUMN_SIZE"), resultSet.getInt("DECIMAL_DIGITS")); Optional columnMapping = toPrestoType(session, typeHandle); diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcTypeHandle.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcTypeHandle.java index 250dbdbbe4c2..c1fef0adef17 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcTypeHandle.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcTypeHandle.java @@ -19,20 +19,24 @@ import java.util.Objects; import static com.google.common.base.MoreObjects.toStringHelper; +import static java.util.Objects.requireNonNull; public final class JdbcTypeHandle { private final int jdbcType; + private final String jdbcTypeName; private final int columnSize; private final int decimalDigits; @JsonCreator public JdbcTypeHandle( @JsonProperty("jdbcType") int jdbcType, + @JsonProperty("jdbcTypeName") String jdbcTypeName, @JsonProperty("columnSize") int columnSize, @JsonProperty("decimalDigits") int decimalDigits) { this.jdbcType = jdbcType; + this.jdbcTypeName = requireNonNull(jdbcTypeName, "jdbcTypeName is null"); this.columnSize = columnSize; this.decimalDigits = decimalDigits; } @@ -43,6 +47,12 @@ public int getJdbcType() return jdbcType; } + @JsonProperty + public String getJdbcTypeName() + { + return jdbcTypeName; + } + @JsonProperty public int getColumnSize() { @@ -58,7 +68,7 @@ public int getDecimalDigits() @Override public int hashCode() { - return Objects.hash(jdbcType, columnSize, decimalDigits); + return Objects.hash(jdbcType, jdbcTypeName, columnSize, decimalDigits); } @Override @@ -73,7 +83,8 @@ public boolean equals(Object o) JdbcTypeHandle that = (JdbcTypeHandle) o; return jdbcType == that.jdbcType && columnSize == that.columnSize && - decimalDigits == that.decimalDigits; + decimalDigits == that.decimalDigits && + Objects.equals(jdbcTypeName, that.jdbcTypeName); } @Override @@ -81,6 +92,7 @@ public String toString() { return toStringHelper(this) .add("jdbcType", jdbcType) + .add("jdbcTypeName", jdbcTypeName) .add("columnSize", columnSize) .add("decimalDigits", decimalDigits) .toString(); diff --git a/presto-base-jdbc/src/test/java/com/facebook/presto/plugin/jdbc/TestingJdbcTypeHandle.java b/presto-base-jdbc/src/test/java/com/facebook/presto/plugin/jdbc/TestingJdbcTypeHandle.java index 55b8b377beb6..57688159fe53 100644 --- a/presto-base-jdbc/src/test/java/com/facebook/presto/plugin/jdbc/TestingJdbcTypeHandle.java +++ b/presto-base-jdbc/src/test/java/com/facebook/presto/plugin/jdbc/TestingJdbcTypeHandle.java @@ -19,20 +19,20 @@ public final class TestingJdbcTypeHandle { private TestingJdbcTypeHandle() {} - public static final JdbcTypeHandle JDBC_BOOLEAN = new JdbcTypeHandle(Types.BOOLEAN, 1, 0); + public static final JdbcTypeHandle JDBC_BOOLEAN = new JdbcTypeHandle(Types.BOOLEAN, "boolean", 1, 0); - public static final JdbcTypeHandle JDBC_SMALLINT = new JdbcTypeHandle(Types.SMALLINT, 1, 0); - public static final JdbcTypeHandle JDBC_TINYINT = new JdbcTypeHandle(Types.TINYINT, 2, 0); - public static final JdbcTypeHandle JDBC_INTEGER = new JdbcTypeHandle(Types.INTEGER, 4, 0); - public static final JdbcTypeHandle JDBC_BIGINT = new JdbcTypeHandle(Types.BIGINT, 8, 0); + public static final JdbcTypeHandle JDBC_SMALLINT = new JdbcTypeHandle(Types.SMALLINT, "smallint", 1, 0); + public static final JdbcTypeHandle JDBC_TINYINT = new JdbcTypeHandle(Types.TINYINT, "tinyint", 2, 0); + public static final JdbcTypeHandle JDBC_INTEGER = new JdbcTypeHandle(Types.INTEGER, "integer", 4, 0); + public static final JdbcTypeHandle JDBC_BIGINT = new JdbcTypeHandle(Types.BIGINT, "bigint", 8, 0); - public static final JdbcTypeHandle JDBC_REAL = new JdbcTypeHandle(Types.REAL, 8, 0); - public static final JdbcTypeHandle JDBC_DOUBLE = new JdbcTypeHandle(Types.DOUBLE, 8, 0); + public static final JdbcTypeHandle JDBC_REAL = new JdbcTypeHandle(Types.REAL, "real", 8, 0); + public static final JdbcTypeHandle JDBC_DOUBLE = new JdbcTypeHandle(Types.DOUBLE, "double precision", 8, 0); - public static final JdbcTypeHandle JDBC_CHAR = new JdbcTypeHandle(Types.CHAR, 10, 0); - public static final JdbcTypeHandle JDBC_VARCHAR = new JdbcTypeHandle(Types.VARCHAR, 10, 0); + public static final JdbcTypeHandle JDBC_CHAR = new JdbcTypeHandle(Types.CHAR, "char", 10, 0); + public static final JdbcTypeHandle JDBC_VARCHAR = new JdbcTypeHandle(Types.VARCHAR, "varchar", 10, 0); - public static final JdbcTypeHandle JDBC_DATE = new JdbcTypeHandle(Types.DATE, 8, 0); - public static final JdbcTypeHandle JDBC_TIME = new JdbcTypeHandle(Types.TIME, 4, 0); - public static final JdbcTypeHandle JDBC_TIMESTAMP = new JdbcTypeHandle(Types.TIMESTAMP, 8, 0); + public static final JdbcTypeHandle JDBC_DATE = new JdbcTypeHandle(Types.DATE, "date", 8, 0); + public static final JdbcTypeHandle JDBC_TIME = new JdbcTypeHandle(Types.TIME, "time", 4, 0); + public static final JdbcTypeHandle JDBC_TIMESTAMP = new JdbcTypeHandle(Types.TIMESTAMP, "timestamp", 8, 0); } From a3ae94875ca1d326352163879ddaa88587353594 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 31 Dec 2018 01:24:47 +0100 Subject: [PATCH 4/9] Rename ReadMapping to ColumnMapping --- .../presto/plugin/jdbc/BaseJdbcClient.java | 6 +- .../{ReadMapping.java => ColumnMapping.java} | 20 ++-- .../presto/plugin/jdbc/JdbcClient.java | 2 +- .../presto/plugin/jdbc/JdbcRecordCursor.java | 2 +- ...pings.java => StandardColumnMappings.java} | 96 +++++++++---------- .../plugin/mysql/TestMySqlTypeMapping.java | 4 +- .../postgresql/TestPostgreSqlTypeMapping.java | 2 +- 7 files changed, 65 insertions(+), 67 deletions(-) rename presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/{ReadMapping.java => ColumnMapping.java} (70%) rename presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/{StandardReadMappings.java => StandardColumnMappings.java} (65%) diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java index e49e79948642..f3db895aaaee 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java @@ -48,7 +48,7 @@ import java.util.UUID; import static com.facebook.presto.plugin.jdbc.JdbcErrorCode.JDBC_ERROR; -import static com.facebook.presto.plugin.jdbc.StandardReadMappings.jdbcTypeToPrestoType; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.jdbcTypeToPrestoType; import static com.facebook.presto.spi.StandardErrorCode.NOT_FOUND; import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; import static com.facebook.presto.spi.type.BigintType.BIGINT; @@ -194,7 +194,7 @@ public List getColumns(ConnectorSession session, JdbcTableHand resultSet.getString("TYPE_NAME"), resultSet.getInt("COLUMN_SIZE"), resultSet.getInt("DECIMAL_DIGITS")); - Optional columnMapping = toPrestoType(session, typeHandle); + Optional columnMapping = toPrestoType(session, typeHandle); // skip unsupported column types if (columnMapping.isPresent()) { String columnName = resultSet.getString("COLUMN_NAME"); @@ -214,7 +214,7 @@ public List getColumns(ConnectorSession session, JdbcTableHand } @Override - public Optional toPrestoType(ConnectorSession session, JdbcTypeHandle typeHandle) + public Optional toPrestoType(ConnectorSession session, JdbcTypeHandle typeHandle) { return jdbcTypeToPrestoType(typeHandle); } diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/ReadMapping.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/ColumnMapping.java similarity index 70% rename from presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/ReadMapping.java rename to presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/ColumnMapping.java index 2db740720e08..8a6f2b1b4996 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/ReadMapping.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/ColumnMapping.java @@ -19,32 +19,32 @@ import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; -public final class ReadMapping +public final class ColumnMapping { - public static ReadMapping booleanReadMapping(Type prestoType, BooleanReadFunction readFunction) + public static ColumnMapping booleanMapping(Type prestoType, BooleanReadFunction readFunction) { - return new ReadMapping(prestoType, readFunction); + return new ColumnMapping(prestoType, readFunction); } - public static ReadMapping longReadMapping(Type prestoType, LongReadFunction readFunction) + public static ColumnMapping longMapping(Type prestoType, LongReadFunction readFunction) { - return new ReadMapping(prestoType, readFunction); + return new ColumnMapping(prestoType, readFunction); } - public static ReadMapping doubleReadMapping(Type prestoType, DoubleReadFunction readFunction) + public static ColumnMapping doubleMapping(Type prestoType, DoubleReadFunction readFunction) { - return new ReadMapping(prestoType, readFunction); + return new ColumnMapping(prestoType, readFunction); } - public static ReadMapping sliceReadMapping(Type prestoType, SliceReadFunction readFunction) + public static ColumnMapping sliceMapping(Type prestoType, SliceReadFunction readFunction) { - return new ReadMapping(prestoType, readFunction); + return new ColumnMapping(prestoType, readFunction); } private final Type type; private final ReadFunction readFunction; - private ReadMapping(Type type, ReadFunction readFunction) + private ColumnMapping(Type type, ReadFunction readFunction) { this.type = requireNonNull(type, "type is null"); this.readFunction = requireNonNull(readFunction, "readFunction is null"); diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java index b82689b24836..0719a58684fb 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java @@ -43,7 +43,7 @@ default boolean schemaExists(String schema) List getColumns(ConnectorSession session, JdbcTableHandle tableHandle); - Optional toPrestoType(ConnectorSession session, JdbcTypeHandle typeHandle); + Optional toPrestoType(ConnectorSession session, JdbcTypeHandle typeHandle); ConnectorSplitSource getSplits(JdbcTableLayoutHandle layoutHandle); diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcRecordCursor.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcRecordCursor.java index f45545baa537..4adda8fd638d 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcRecordCursor.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcRecordCursor.java @@ -63,7 +63,7 @@ public JdbcRecordCursor(JdbcClient jdbcClient, ConnectorSession session, JdbcSpl sliceReadFunctions = new SliceReadFunction[columnHandles.size()]; for (int i = 0; i < this.columnHandles.length; i++) { - ReadMapping readMapping = jdbcClient.toPrestoType(session, columnHandles.get(i).getJdbcTypeHandle()) + ColumnMapping readMapping = jdbcClient.toPrestoType(session, columnHandles.get(i).getJdbcTypeHandle()) .orElseThrow(() -> new VerifyException("Unsupported column type")); Class javaType = readMapping.getType().getJavaType(); ReadFunction readFunction = readMapping.getReadFunction(); diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/StandardReadMappings.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/StandardColumnMappings.java similarity index 65% rename from presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/StandardReadMappings.java rename to presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/StandardColumnMappings.java index 43d7af5060b8..973b30bf5138 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/StandardReadMappings.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/StandardColumnMappings.java @@ -26,8 +26,6 @@ import java.sql.Types; import java.util.Optional; -import static com.facebook.presto.plugin.jdbc.ReadMapping.longReadMapping; -import static com.facebook.presto.plugin.jdbc.ReadMapping.sliceReadMapping; import static com.facebook.presto.spi.type.BigintType.BIGINT; import static com.facebook.presto.spi.type.BooleanType.BOOLEAN; import static com.facebook.presto.spi.type.CharType.createCharType; @@ -54,76 +52,76 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.joda.time.DateTimeZone.UTC; -public final class StandardReadMappings +public final class StandardColumnMappings { - private StandardReadMappings() {} + private StandardColumnMappings() {} private static final ISOChronology UTC_CHRONOLOGY = ISOChronology.getInstanceUTC(); - public static ReadMapping booleanReadMapping() + public static ColumnMapping booleanColumnMapping() { - return ReadMapping.booleanReadMapping(BOOLEAN, ResultSet::getBoolean); + return ColumnMapping.booleanMapping(BOOLEAN, ResultSet::getBoolean); } - public static ReadMapping tinyintReadMapping() + public static ColumnMapping tinyintColumnMapping() { - return longReadMapping(TINYINT, ResultSet::getByte); + return ColumnMapping.longMapping(TINYINT, ResultSet::getByte); } - public static ReadMapping smallintReadMapping() + public static ColumnMapping smallintColumnMapping() { - return longReadMapping(SMALLINT, ResultSet::getShort); + return ColumnMapping.longMapping(SMALLINT, ResultSet::getShort); } - public static ReadMapping integerReadMapping() + public static ColumnMapping integerColumnMapping() { - return longReadMapping(INTEGER, ResultSet::getInt); + return ColumnMapping.longMapping(INTEGER, ResultSet::getInt); } - public static ReadMapping bigintReadMapping() + public static ColumnMapping bigintColumnMapping() { - return longReadMapping(BIGINT, ResultSet::getLong); + return ColumnMapping.longMapping(BIGINT, ResultSet::getLong); } - public static ReadMapping realReadMapping() + public static ColumnMapping realColumnMapping() { - return longReadMapping(REAL, (resultSet, columnIndex) -> floatToRawIntBits(resultSet.getFloat(columnIndex))); + return ColumnMapping.longMapping(REAL, (resultSet, columnIndex) -> floatToRawIntBits(resultSet.getFloat(columnIndex))); } - public static ReadMapping doubleReadMapping() + public static ColumnMapping doubleColumnMapping() { - return ReadMapping.doubleReadMapping(DOUBLE, ResultSet::getDouble); + return ColumnMapping.doubleMapping(DOUBLE, ResultSet::getDouble); } - public static ReadMapping decimalReadMapping(DecimalType decimalType) + public static ColumnMapping decimalColumnMapping(DecimalType decimalType) { // JDBC driver can return BigDecimal with lower scale than column's scale when there are trailing zeroes int scale = decimalType.getScale(); if (decimalType.isShort()) { - return longReadMapping(decimalType, (resultSet, columnIndex) -> encodeShortScaledValue(resultSet.getBigDecimal(columnIndex), scale)); + return ColumnMapping.longMapping(decimalType, (resultSet, columnIndex) -> encodeShortScaledValue(resultSet.getBigDecimal(columnIndex), scale)); } - return sliceReadMapping(decimalType, (resultSet, columnIndex) -> encodeScaledValue(resultSet.getBigDecimal(columnIndex), scale)); + return ColumnMapping.sliceMapping(decimalType, (resultSet, columnIndex) -> encodeScaledValue(resultSet.getBigDecimal(columnIndex), scale)); } - public static ReadMapping charReadMapping(CharType charType) + public static ColumnMapping charColumnMapping(CharType charType) { requireNonNull(charType, "charType is null"); - return sliceReadMapping(charType, (resultSet, columnIndex) -> utf8Slice(CharMatcher.is(' ').trimTrailingFrom(resultSet.getString(columnIndex)))); + return ColumnMapping.sliceMapping(charType, (resultSet, columnIndex) -> utf8Slice(CharMatcher.is(' ').trimTrailingFrom(resultSet.getString(columnIndex)))); } - public static ReadMapping varcharReadMapping(VarcharType varcharType) + public static ColumnMapping varcharColumnMapping(VarcharType varcharType) { - return sliceReadMapping(varcharType, (resultSet, columnIndex) -> utf8Slice(resultSet.getString(columnIndex))); + return ColumnMapping.sliceMapping(varcharType, (resultSet, columnIndex) -> utf8Slice(resultSet.getString(columnIndex))); } - public static ReadMapping varbinaryReadMapping() + public static ColumnMapping varbinaryColumnMapping() { - return sliceReadMapping(VARBINARY, (resultSet, columnIndex) -> wrappedBuffer(resultSet.getBytes(columnIndex))); + return ColumnMapping.sliceMapping(VARBINARY, (resultSet, columnIndex) -> wrappedBuffer(resultSet.getBytes(columnIndex))); } - public static ReadMapping dateReadMapping() + public static ColumnMapping dateColumnMapping() { - return longReadMapping(DATE, (resultSet, columnIndex) -> { + return ColumnMapping.longMapping(DATE, (resultSet, columnIndex) -> { /* * JDBC returns a date using a timestamp at midnight in the JVM timezone, or earliest time after that if there was no midnight. * This works correctly for all dates and zones except when the missing local times 'gap' is 24h. I.e. this fails when JVM time @@ -140,9 +138,9 @@ public static ReadMapping dateReadMapping() }); } - public static ReadMapping timeReadMapping() + public static ColumnMapping timeColumnMapping() { - return longReadMapping(TIME, (resultSet, columnIndex) -> { + return ColumnMapping.longMapping(TIME, (resultSet, columnIndex) -> { /* * TODO `resultSet.getTime(columnIndex)` returns wrong value if JVM's zone had forward offset change during 1970-01-01 * and the time value being retrieved was not present in local time (a 'gap'), e.g. time retrieved is 00:10:00 and JVM zone is America/Hermosillo @@ -153,9 +151,9 @@ public static ReadMapping timeReadMapping() }); } - public static ReadMapping timestampReadMapping() + public static ColumnMapping timestampColumnMapping() { - return longReadMapping(TIMESTAMP, (resultSet, columnIndex) -> { + return ColumnMapping.longMapping(TIMESTAMP, (resultSet, columnIndex) -> { /* * TODO `resultSet.getTimestamp(columnIndex)` returns wrong value if JVM's zone had forward offset change and the local time * corresponding to timestamp value being retrieved was not present (a 'gap'), this includes regular DST changes (e.g. Europe/Warsaw) @@ -167,32 +165,32 @@ public static ReadMapping timestampReadMapping() }); } - public static Optional jdbcTypeToPrestoType(JdbcTypeHandle type) + public static Optional jdbcTypeToPrestoType(JdbcTypeHandle type) { int columnSize = type.getColumnSize(); switch (type.getJdbcType()) { case Types.BIT: case Types.BOOLEAN: - return Optional.of(booleanReadMapping()); + return Optional.of(booleanColumnMapping()); case Types.TINYINT: - return Optional.of(tinyintReadMapping()); + return Optional.of(tinyintColumnMapping()); case Types.SMALLINT: - return Optional.of(smallintReadMapping()); + return Optional.of(smallintColumnMapping()); case Types.INTEGER: - return Optional.of(integerReadMapping()); + return Optional.of(integerColumnMapping()); case Types.BIGINT: - return Optional.of(bigintReadMapping()); + return Optional.of(bigintColumnMapping()); case Types.REAL: - return Optional.of(realReadMapping()); + return Optional.of(realColumnMapping()); case Types.FLOAT: case Types.DOUBLE: - return Optional.of(doubleReadMapping()); + return Optional.of(doubleColumnMapping()); case Types.NUMERIC: case Types.DECIMAL: @@ -201,36 +199,36 @@ public static Optional jdbcTypeToPrestoType(JdbcTypeHandle type) if (precision > Decimals.MAX_PRECISION) { return Optional.empty(); } - return Optional.of(decimalReadMapping(createDecimalType(precision, max(decimalDigits, 0)))); + return Optional.of(decimalColumnMapping(createDecimalType(precision, max(decimalDigits, 0)))); case Types.CHAR: case Types.NCHAR: // TODO this is wrong, we're going to construct malformed Slice representation if source > charLength int charLength = min(columnSize, CharType.MAX_LENGTH); - return Optional.of(charReadMapping(createCharType(charLength))); + return Optional.of(charColumnMapping(createCharType(charLength))); case Types.VARCHAR: case Types.NVARCHAR: case Types.LONGVARCHAR: case Types.LONGNVARCHAR: if (columnSize > VarcharType.MAX_LENGTH) { - return Optional.of(varcharReadMapping(createUnboundedVarcharType())); + return Optional.of(varcharColumnMapping(createUnboundedVarcharType())); } - return Optional.of(varcharReadMapping(createVarcharType(columnSize))); + return Optional.of(varcharColumnMapping(createVarcharType(columnSize))); case Types.BINARY: case Types.VARBINARY: case Types.LONGVARBINARY: - return Optional.of(varbinaryReadMapping()); + return Optional.of(varbinaryColumnMapping()); case Types.DATE: - return Optional.of(dateReadMapping()); + return Optional.of(dateColumnMapping()); case Types.TIME: - return Optional.of(timeReadMapping()); + return Optional.of(timeColumnMapping()); case Types.TIMESTAMP: - return Optional.of(timestampReadMapping()); + return Optional.of(timestampColumnMapping()); } return Optional.empty(); } diff --git a/presto-mysql/src/test/java/com/facebook/presto/plugin/mysql/TestMySqlTypeMapping.java b/presto-mysql/src/test/java/com/facebook/presto/plugin/mysql/TestMySqlTypeMapping.java index 3eafa77dabd0..5b8f769dc2a6 100644 --- a/presto-mysql/src/test/java/com/facebook/presto/plugin/mysql/TestMySqlTypeMapping.java +++ b/presto-mysql/src/test/java/com/facebook/presto/plugin/mysql/TestMySqlTypeMapping.java @@ -251,13 +251,13 @@ public void testDate() @Test public void testDatetime() { - // TODO MySQL datetime is not correctly read (see comment in StandardReadMappings.timestampReadMapping), but testing this is hard because of #7122 + // TODO MySQL datetime is not correctly read (see comment in StandardColumnMappings.timestampColumnMapping), but testing this is hard because of #7122 } @Test public void testTimestamp() { - // TODO MySQL timestamp is not correctly read (see comment in StandardReadMappings.timestampReadMapping), but testing this is hard because of #7122 + // TODO MySQL timestamp is not correctly read (see comment in StandardColumnMappings.timestampColumnMapping), but testing this is hard because of #7122 } private void testUnsupportedDataType(String databaseDataType) diff --git a/presto-postgresql/src/test/java/com/facebook/presto/plugin/postgresql/TestPostgreSqlTypeMapping.java b/presto-postgresql/src/test/java/com/facebook/presto/plugin/postgresql/TestPostgreSqlTypeMapping.java index 5fe0f83743ae..600f2e80dd87 100644 --- a/presto-postgresql/src/test/java/com/facebook/presto/plugin/postgresql/TestPostgreSqlTypeMapping.java +++ b/presto-postgresql/src/test/java/com/facebook/presto/plugin/postgresql/TestPostgreSqlTypeMapping.java @@ -264,7 +264,7 @@ public void testDate() @Test public void testTimestamp() { - // TODO timestamp is not correctly read (see comment in StandardReadMappings.timestampReadMapping), but testing this is hard because of #7122 + // TODO timestamp is not correctly read (see comment in StandardColumnMappings.timestampColumnMapping), but testing this is hard because of #7122 } private void testUnsupportedDataType(String databaseDataType) From 7b374839bdf29d5e7ab9b54571edf11554823ee1 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 31 Dec 2018 01:24:48 +0100 Subject: [PATCH 5/9] Add WriteFunction to ColumnMapping --- .../plugin/jdbc/BooleanWriteFunction.java | 30 +++ .../presto/plugin/jdbc/ColumnMapping.java | 31 ++- .../plugin/jdbc/DoubleWriteFunction.java | 30 +++ .../presto/plugin/jdbc/LongWriteFunction.java | 30 +++ .../plugin/jdbc/SliceWriteFunction.java | 32 +++ .../plugin/jdbc/StandardColumnMappings.java | 234 ++++++++++++++---- .../presto/plugin/jdbc/WriteFunction.java | 19 ++ .../facebook/presto/spi/type/CharType.java | 1 + 8 files changed, 352 insertions(+), 55 deletions(-) create mode 100644 presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BooleanWriteFunction.java create mode 100644 presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/DoubleWriteFunction.java create mode 100644 presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/LongWriteFunction.java create mode 100644 presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/SliceWriteFunction.java create mode 100644 presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/WriteFunction.java diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BooleanWriteFunction.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BooleanWriteFunction.java new file mode 100644 index 000000000000..6396613654f3 --- /dev/null +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BooleanWriteFunction.java @@ -0,0 +1,30 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not 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 applicable law 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 com.facebook.presto.plugin.jdbc; + +import java.sql.PreparedStatement; +import java.sql.SQLException; + +public interface BooleanWriteFunction + extends WriteFunction +{ + @Override + default Class getJavaType() + { + return boolean.class; + } + + void set(PreparedStatement statement, int index, boolean value) + throws SQLException; +} diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/ColumnMapping.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/ColumnMapping.java index 8a6f2b1b4996..05ef55c92d8e 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/ColumnMapping.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/ColumnMapping.java @@ -21,39 +21,47 @@ public final class ColumnMapping { - public static ColumnMapping booleanMapping(Type prestoType, BooleanReadFunction readFunction) + public static ColumnMapping booleanMapping(Type prestoType, BooleanReadFunction readFunction, BooleanWriteFunction writeFunction) { - return new ColumnMapping(prestoType, readFunction); + return new ColumnMapping(prestoType, readFunction, writeFunction); } - public static ColumnMapping longMapping(Type prestoType, LongReadFunction readFunction) + public static ColumnMapping longMapping(Type prestoType, LongReadFunction readFunction, LongWriteFunction writeFunction) { - return new ColumnMapping(prestoType, readFunction); + return new ColumnMapping(prestoType, readFunction, writeFunction); } - public static ColumnMapping doubleMapping(Type prestoType, DoubleReadFunction readFunction) + public static ColumnMapping doubleMapping(Type prestoType, DoubleReadFunction readFunction, DoubleWriteFunction writeFunction) { - return new ColumnMapping(prestoType, readFunction); + return new ColumnMapping(prestoType, readFunction, writeFunction); } - public static ColumnMapping sliceMapping(Type prestoType, SliceReadFunction readFunction) + public static ColumnMapping sliceMapping(Type prestoType, SliceReadFunction readFunction, SliceWriteFunction writeFunction) { - return new ColumnMapping(prestoType, readFunction); + return new ColumnMapping(prestoType, readFunction, writeFunction); } private final Type type; private final ReadFunction readFunction; + private final WriteFunction writeFunction; - private ColumnMapping(Type type, ReadFunction readFunction) + private ColumnMapping(Type type, ReadFunction readFunction, WriteFunction writeFunction) { this.type = requireNonNull(type, "type is null"); this.readFunction = requireNonNull(readFunction, "readFunction is null"); + this.writeFunction = requireNonNull(writeFunction, "writeFunction is null"); checkArgument( type.getJavaType() == readFunction.getJavaType(), "Presto type %s is not compatible with read function %s returning %s", type, readFunction, readFunction.getJavaType()); + checkArgument( + type.getJavaType() == writeFunction.getJavaType(), + "Presto type %s is not compatible with write function %s accepting %s", + type, + writeFunction, + writeFunction.getJavaType()); } public Type getType() @@ -66,6 +74,11 @@ public ReadFunction getReadFunction() return readFunction; } + public WriteFunction getWriteFunction() + { + return writeFunction; + } + @Override public String toString() { diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/DoubleWriteFunction.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/DoubleWriteFunction.java new file mode 100644 index 000000000000..c433644cdd27 --- /dev/null +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/DoubleWriteFunction.java @@ -0,0 +1,30 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not 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 applicable law 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 com.facebook.presto.plugin.jdbc; + +import java.sql.PreparedStatement; +import java.sql.SQLException; + +public interface DoubleWriteFunction + extends WriteFunction +{ + @Override + default Class getJavaType() + { + return double.class; + } + + void set(PreparedStatement statement, int index, double value) + throws SQLException; +} diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/LongWriteFunction.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/LongWriteFunction.java new file mode 100644 index 000000000000..fea27d097703 --- /dev/null +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/LongWriteFunction.java @@ -0,0 +1,30 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not 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 applicable law 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 com.facebook.presto.plugin.jdbc; + +import java.sql.PreparedStatement; +import java.sql.SQLException; + +public interface LongWriteFunction + extends WriteFunction +{ + @Override + default Class getJavaType() + { + return long.class; + } + + void set(PreparedStatement statement, int index, long value) + throws SQLException; +} diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/SliceWriteFunction.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/SliceWriteFunction.java new file mode 100644 index 000000000000..a12d3d0e50bb --- /dev/null +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/SliceWriteFunction.java @@ -0,0 +1,32 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not 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 applicable law 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 com.facebook.presto.plugin.jdbc; + +import io.airlift.slice.Slice; + +import java.sql.PreparedStatement; +import java.sql.SQLException; + +public interface SliceWriteFunction + extends WriteFunction +{ + @Override + default Class getJavaType() + { + return Slice.class; + } + + void set(PreparedStatement statement, int index, Slice value) + throws SQLException; +} diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/StandardColumnMappings.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/StandardColumnMappings.java index 973b30bf5138..3738b4261375 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/StandardColumnMappings.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/StandardColumnMappings.java @@ -18,8 +18,16 @@ import com.facebook.presto.spi.type.Decimals; import com.facebook.presto.spi.type.VarcharType; import com.google.common.base.CharMatcher; +import com.google.common.primitives.Shorts; +import com.google.common.primitives.SignedBytes; +import org.joda.time.DateTimeZone; import org.joda.time.chrono.ISOChronology; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.math.MathContext; +import java.sql.Date; +import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.Time; import java.sql.Timestamp; @@ -31,6 +39,7 @@ import static com.facebook.presto.spi.type.CharType.createCharType; import static com.facebook.presto.spi.type.DateType.DATE; import static com.facebook.presto.spi.type.DecimalType.createDecimalType; +import static com.facebook.presto.spi.type.Decimals.decodeUnscaledValue; import static com.facebook.presto.spi.type.Decimals.encodeScaledValue; import static com.facebook.presto.spi.type.Decimals.encodeShortScaledValue; import static com.facebook.presto.spi.type.DoubleType.DOUBLE; @@ -43,12 +52,16 @@ import static com.facebook.presto.spi.type.VarbinaryType.VARBINARY; import static com.facebook.presto.spi.type.VarcharType.createUnboundedVarcharType; import static com.facebook.presto.spi.type.VarcharType.createVarcharType; +import static com.google.common.base.Preconditions.checkArgument; import static io.airlift.slice.Slices.utf8Slice; import static io.airlift.slice.Slices.wrappedBuffer; import static java.lang.Float.floatToRawIntBits; +import static java.lang.Float.intBitsToFloat; import static java.lang.Math.max; import static java.lang.Math.min; +import static java.lang.Math.toIntExact; import static java.util.Objects.requireNonNull; +import static java.util.concurrent.TimeUnit.DAYS; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.joda.time.DateTimeZone.UTC; @@ -60,37 +73,72 @@ private StandardColumnMappings() {} public static ColumnMapping booleanColumnMapping() { - return ColumnMapping.booleanMapping(BOOLEAN, ResultSet::getBoolean); + return ColumnMapping.booleanMapping(BOOLEAN, ResultSet::getBoolean, booleanWriteFunction()); + } + + public static BooleanWriteFunction booleanWriteFunction() + { + return PreparedStatement::setBoolean; } public static ColumnMapping tinyintColumnMapping() { - return ColumnMapping.longMapping(TINYINT, ResultSet::getByte); + return ColumnMapping.longMapping(TINYINT, ResultSet::getByte, tinyintWriteFunction()); + } + + public static LongWriteFunction tinyintWriteFunction() + { + return (statement, index, value) -> statement.setByte(index, SignedBytes.checkedCast(value)); } public static ColumnMapping smallintColumnMapping() { - return ColumnMapping.longMapping(SMALLINT, ResultSet::getShort); + return ColumnMapping.longMapping(SMALLINT, ResultSet::getShort, smallintWriteFunction()); + } + + public static LongWriteFunction smallintWriteFunction() + { + return (statement, index, value) -> statement.setShort(index, Shorts.checkedCast(value)); } public static ColumnMapping integerColumnMapping() { - return ColumnMapping.longMapping(INTEGER, ResultSet::getInt); + return ColumnMapping.longMapping(INTEGER, ResultSet::getInt, integerWriteFunction()); + } + + public static LongWriteFunction integerWriteFunction() + { + return (statement, index, value) -> statement.setInt(index, toIntExact(value)); } public static ColumnMapping bigintColumnMapping() { - return ColumnMapping.longMapping(BIGINT, ResultSet::getLong); + return ColumnMapping.longMapping(BIGINT, ResultSet::getLong, bigintWriteFunction()); + } + + public static LongWriteFunction bigintWriteFunction() + { + return PreparedStatement::setLong; } public static ColumnMapping realColumnMapping() { - return ColumnMapping.longMapping(REAL, (resultSet, columnIndex) -> floatToRawIntBits(resultSet.getFloat(columnIndex))); + return ColumnMapping.longMapping(REAL, (resultSet, columnIndex) -> floatToRawIntBits(resultSet.getFloat(columnIndex)), realWriteFunction()); + } + + public static LongWriteFunction realWriteFunction() + { + return (statement, index, value) -> statement.setFloat(index, intBitsToFloat(toIntExact(value))); } public static ColumnMapping doubleColumnMapping() { - return ColumnMapping.doubleMapping(DOUBLE, ResultSet::getDouble); + return ColumnMapping.doubleMapping(DOUBLE, ResultSet::getDouble, doubleWriteFunction()); + } + + public static DoubleWriteFunction doubleWriteFunction() + { + return PreparedStatement::setDouble; } public static ColumnMapping decimalColumnMapping(DecimalType decimalType) @@ -98,71 +146,165 @@ public static ColumnMapping decimalColumnMapping(DecimalType decimalType) // JDBC driver can return BigDecimal with lower scale than column's scale when there are trailing zeroes int scale = decimalType.getScale(); if (decimalType.isShort()) { - return ColumnMapping.longMapping(decimalType, (resultSet, columnIndex) -> encodeShortScaledValue(resultSet.getBigDecimal(columnIndex), scale)); + return ColumnMapping.longMapping(decimalType, + (resultSet, columnIndex) -> encodeShortScaledValue(resultSet.getBigDecimal(columnIndex), scale), + shortDecimalWriteFunction(decimalType)); } - return ColumnMapping.sliceMapping(decimalType, (resultSet, columnIndex) -> encodeScaledValue(resultSet.getBigDecimal(columnIndex), scale)); + return ColumnMapping.sliceMapping( + decimalType, + (resultSet, columnIndex) -> encodeScaledValue(resultSet.getBigDecimal(columnIndex), scale), + longDecimalWriteFunction(decimalType)); + } + + public static LongWriteFunction shortDecimalWriteFunction(DecimalType decimalType) + { + requireNonNull(decimalType, "decimalType is null"); + checkArgument(decimalType.isShort()); + return (statement, index, value) -> { + BigInteger unscaledValue = BigInteger.valueOf(value); + BigDecimal bigDecimal = new BigDecimal(unscaledValue, decimalType.getScale(), new MathContext(decimalType.getPrecision())); + statement.setBigDecimal(index, bigDecimal); + }; + } + + public static SliceWriteFunction longDecimalWriteFunction(DecimalType decimalType) + { + requireNonNull(decimalType, "decimalType is null"); + checkArgument(!decimalType.isShort()); + return (statement, index, value) -> { + BigInteger unscaledValue = decodeUnscaledValue(value); + BigDecimal bigDecimal = new BigDecimal(unscaledValue, decimalType.getScale(), new MathContext(decimalType.getPrecision())); + statement.setBigDecimal(index, bigDecimal); + }; } public static ColumnMapping charColumnMapping(CharType charType) { requireNonNull(charType, "charType is null"); - return ColumnMapping.sliceMapping(charType, (resultSet, columnIndex) -> utf8Slice(CharMatcher.is(' ').trimTrailingFrom(resultSet.getString(columnIndex)))); + return ColumnMapping.sliceMapping( + charType, + (resultSet, columnIndex) -> utf8Slice(CharMatcher.is(' ').trimTrailingFrom(resultSet.getString(columnIndex))), + charWriteFunction(charType)); + } + + public static SliceWriteFunction charWriteFunction(CharType charType) + { + requireNonNull(charType, "charType is null"); + return (statement, index, value) -> { + StringBuilder builder = new StringBuilder(charType.getLength()); + String string = value.toStringUtf8(); + builder.append(string); + for (int i = string.length(); i < charType.getLength(); i++) { + // TODO we should be padding considering length as calculated with `io.airlift.slice.SliceUtf8.countCodePoints(io.airlift.slice.Slice)` + builder.append(' '); + } + statement.setString(index, builder.toString()); + }; } public static ColumnMapping varcharColumnMapping(VarcharType varcharType) { - return ColumnMapping.sliceMapping(varcharType, (resultSet, columnIndex) -> utf8Slice(resultSet.getString(columnIndex))); + return ColumnMapping.sliceMapping(varcharType, (resultSet, columnIndex) -> utf8Slice(resultSet.getString(columnIndex)), varcharWriteFunction()); + } + + public static SliceWriteFunction varcharWriteFunction() + { + return (statement, index, value) -> statement.setString(index, value.toStringUtf8()); } public static ColumnMapping varbinaryColumnMapping() { - return ColumnMapping.sliceMapping(VARBINARY, (resultSet, columnIndex) -> wrappedBuffer(resultSet.getBytes(columnIndex))); + return ColumnMapping.sliceMapping( + VARBINARY, + (resultSet, columnIndex) -> wrappedBuffer(resultSet.getBytes(columnIndex)), + varbinaryWriteFunction()); + } + + public static SliceWriteFunction varbinaryWriteFunction() + { + return (statement, index, value) -> statement.setBytes(index, value.getBytes()); } public static ColumnMapping dateColumnMapping() { - return ColumnMapping.longMapping(DATE, (resultSet, columnIndex) -> { - /* - * JDBC returns a date using a timestamp at midnight in the JVM timezone, or earliest time after that if there was no midnight. - * This works correctly for all dates and zones except when the missing local times 'gap' is 24h. I.e. this fails when JVM time - * zone is Pacific/Apia and date to be returned is 2011-12-30. - * - * `return resultSet.getObject(columnIndex, LocalDate.class).toEpochDay()` avoids these problems but - * is currently known not to work with Redshift (old Postgres connector) and SQL Server. - */ - long localMillis = resultSet.getDate(columnIndex).getTime(); - // Convert it to a ~midnight in UTC. - long utcMillis = ISOChronology.getInstance().getZone().getMillisKeepLocal(UTC, localMillis); - // convert to days - return MILLISECONDS.toDays(utcMillis); - }); + return ColumnMapping.longMapping( + DATE, + (resultSet, columnIndex) -> { + /* + * JDBC returns a date using a timestamp at midnight in the JVM timezone, or earliest time after that if there was no midnight. + * This works correctly for all dates and zones except when the missing local times 'gap' is 24h. I.e. this fails when JVM time + * zone is Pacific/Apia and date to be returned is 2011-12-30. + * + * `return resultSet.getObject(columnIndex, LocalDate.class).toEpochDay()` avoids these problems but + * is currently known not to work with Redshift (old Postgres connector) and SQL Server. + */ + long localMillis = resultSet.getDate(columnIndex).getTime(); + // Convert it to a ~midnight in UTC. + long utcMillis = ISOChronology.getInstance().getZone().getMillisKeepLocal(UTC, localMillis); + // convert to days + return MILLISECONDS.toDays(utcMillis); + }, + dateWriteFunction()); + } + + public static LongWriteFunction dateWriteFunction() + { + return (statement, index, value) -> { + // convert to midnight in default time zone + long millis = DAYS.toMillis(value); + statement.setDate(index, new Date(UTC.getMillisKeepLocal(DateTimeZone.getDefault(), millis))); + }; } public static ColumnMapping timeColumnMapping() { - return ColumnMapping.longMapping(TIME, (resultSet, columnIndex) -> { - /* - * TODO `resultSet.getTime(columnIndex)` returns wrong value if JVM's zone had forward offset change during 1970-01-01 - * and the time value being retrieved was not present in local time (a 'gap'), e.g. time retrieved is 00:10:00 and JVM zone is America/Hermosillo - * The problem can be averted by using `resultSet.getObject(columnIndex, LocalTime.class)` -- but this is not universally supported by JDBC drivers. - */ - Time time = resultSet.getTime(columnIndex); - return UTC_CHRONOLOGY.millisOfDay().get(time.getTime()); - }); + return ColumnMapping.longMapping( + TIME, + (resultSet, columnIndex) -> { + /* + * TODO `resultSet.getTime(columnIndex)` returns wrong value if JVM's zone had forward offset change during 1970-01-01 + * and the time value being retrieved was not present in local time (a 'gap'), e.g. time retrieved is 00:10:00 and JVM zone is America/Hermosillo + * The problem can be averted by using `resultSet.getObject(columnIndex, LocalTime.class)` -- but this is not universally supported by JDBC drivers. + */ + Time time = resultSet.getTime(columnIndex); + return UTC_CHRONOLOGY.millisOfDay().get(time.getTime()); + }, + timeWriteFunction()); + } + + public static LongWriteFunction timeWriteFunction() + { + return (statement, index, value) -> { + // Copied from `QueryBuilder.buildSql` + // TODO verify correctness, add tests and support non-legacy timestamp + statement.setTime(index, new Time(value)); + }; } public static ColumnMapping timestampColumnMapping() { - return ColumnMapping.longMapping(TIMESTAMP, (resultSet, columnIndex) -> { - /* - * TODO `resultSet.getTimestamp(columnIndex)` returns wrong value if JVM's zone had forward offset change and the local time - * corresponding to timestamp value being retrieved was not present (a 'gap'), this includes regular DST changes (e.g. Europe/Warsaw) - * and one-time policy changes (Asia/Kathmandu's shift by 15 minutes on January 1, 1986, 00:00:00). - * The problem can be averted by using `resultSet.getObject(columnIndex, LocalDateTime.class)` -- but this is not universally supported by JDBC drivers. - */ - Timestamp timestamp = resultSet.getTimestamp(columnIndex); - return timestamp.getTime(); - }); + return ColumnMapping.longMapping( + TIMESTAMP, + (resultSet, columnIndex) -> { + /* + * TODO `resultSet.getTimestamp(columnIndex)` returns wrong value if JVM's zone had forward offset change and the local time + * corresponding to timestamp value being retrieved was not present (a 'gap'), this includes regular DST changes (e.g. Europe/Warsaw) + * and one-time policy changes (Asia/Kathmandu's shift by 15 minutes on January 1, 1986, 00:00:00). + * The problem can be averted by using `resultSet.getObject(columnIndex, LocalDateTime.class)` -- but this is not universally supported by JDBC drivers. + */ + Timestamp timestamp = resultSet.getTimestamp(columnIndex); + return timestamp.getTime(); + }, + timestampWriteFunction()); + } + + public static LongWriteFunction timestampWriteFunction() + { + return (statement, index, value) -> { + // Copied from `QueryBuilder.buildSql` + // TODO verify correctness, add tests and support non-legacy timestamp + statement.setTimestamp(index, new Timestamp(value)); + }; } public static Optional jdbcTypeToPrestoType(JdbcTypeHandle type) diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/WriteFunction.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/WriteFunction.java new file mode 100644 index 000000000000..674510d07732 --- /dev/null +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/WriteFunction.java @@ -0,0 +1,19 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not 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 applicable law 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 com.facebook.presto.plugin.jdbc; + +public interface WriteFunction +{ + Class getJavaType(); +} diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/type/CharType.java b/presto-spi/src/main/java/com/facebook/presto/spi/type/CharType.java index 97fcb4785c4b..c1c087e4a472 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/type/CharType.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/type/CharType.java @@ -81,6 +81,7 @@ public Object getObjectValue(ConnectorSession session, Block block, int position String value = block.getSlice(position, 0, block.getSliceLength(position)).toStringUtf8(); builder.append(value); for (int i = value.length(); i < length; i++) { + // TODO we should be padding considering length as calculated with `io.airlift.slice.SliceUtf8.countCodePoints(io.airlift.slice.Slice)` builder.append(' '); } From 5278c89146ac00ef28f5a523588f535af24b388c Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Thu, 3 Jan 2019 01:06:48 +0100 Subject: [PATCH 6/9] Add WriteMapping WriteMapping captures how data should be written in a JDBC connector: - what data type should be used in the remote database - how the data should be bound to `PreparedStatement` --- .../presto/plugin/jdbc/BaseJdbcClient.java | 84 ++++++++++++++----- .../presto/plugin/jdbc/WriteMapping.java | 67 +++++++++++++++ .../presto/plugin/mysql/MySqlClient.java | 37 +++++--- .../plugin/postgresql/PostgreSqlClient.java | 8 +- 4 files changed, 159 insertions(+), 37 deletions(-) create mode 100644 presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/WriteMapping.java diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java index f3db895aaaee..d7d48fc101bb 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java @@ -48,7 +48,24 @@ import java.util.UUID; import static com.facebook.presto.plugin.jdbc.JdbcErrorCode.JDBC_ERROR; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.bigintWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.booleanWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.charWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.dateWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.doubleWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.integerWriteFunction; import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.jdbcTypeToPrestoType; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.longDecimalWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.realWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.shortDecimalWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.smallintWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.tinyintWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.varbinaryWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.varcharWriteFunction; +import static com.facebook.presto.plugin.jdbc.WriteMapping.booleanWriteMapping; +import static com.facebook.presto.plugin.jdbc.WriteMapping.doubleWriteMapping; +import static com.facebook.presto.plugin.jdbc.WriteMapping.longWriteMapping; +import static com.facebook.presto.plugin.jdbc.WriteMapping.sliceWriteMapping; import static com.facebook.presto.spi.StandardErrorCode.NOT_FOUND; import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; import static com.facebook.presto.spi.type.BigintType.BIGINT; @@ -74,16 +91,16 @@ public class BaseJdbcClient { private static final Logger log = Logger.get(BaseJdbcClient.class); - private static final Map SQL_TYPES = ImmutableMap.builder() - .put(BOOLEAN, "boolean") - .put(BIGINT, "bigint") - .put(INTEGER, "integer") - .put(SMALLINT, "smallint") - .put(TINYINT, "tinyint") - .put(DOUBLE, "double precision") - .put(REAL, "real") - .put(VARBINARY, "varbinary") - .put(DATE, "date") + private static final Map WRITE_MAPPINGS = ImmutableMap.builder() + .put(BOOLEAN, booleanWriteMapping("boolean", booleanWriteFunction())) + .put(BIGINT, longWriteMapping("bigint", bigintWriteFunction())) + .put(INTEGER, longWriteMapping("integer", integerWriteFunction())) + .put(SMALLINT, longWriteMapping("smallint", smallintWriteFunction())) + .put(TINYINT, longWriteMapping("tinyint", tinyintWriteFunction())) + .put(DOUBLE, doubleWriteMapping("double precision", doubleWriteFunction())) + .put(REAL, longWriteMapping("real", realWriteFunction())) + .put(VARBINARY, sliceWriteMapping("varbinary", varbinaryWriteFunction())) + .put(DATE, longWriteMapping("date", dateWriteFunction())) .build(); protected final String connectorId; @@ -309,7 +326,8 @@ private JdbcOutputTableHandle beginWriteTable(ConnectorTableMetadata tableMetada columnList.add(new StringBuilder() .append(quoted(columnName)) .append(" ") - .append(toSqlType(column.getType())) + // TODO in INSERT case, we should reuse original column type and, ideally, constraints (then JdbcPageSink must get writer from toPrestoType()) + .append(toWriteMapping(column.getType()).getDataType()) .toString()); } Joiner.on(", ").appendTo(sql, columnList.build()); @@ -456,28 +474,52 @@ protected void execute(Connection connection, String query) } } - protected String toSqlType(Type type) + /** + * @deprecated Use {@link #toWriteMapping(Type)}. + */ + @Deprecated + protected final String toSqlType(Type type) + { + // TODO remove this method when all connectors updated + return toWriteMapping(type).getDataType(); + } + + protected WriteMapping toWriteMapping(Type type) { if (isVarcharType(type)) { VarcharType varcharType = (VarcharType) type; + String dataType; if (varcharType.isUnbounded()) { - return "varchar"; + dataType = "varchar"; } - return "varchar(" + varcharType.getLengthSafe() + ")"; + else { + dataType = "varchar(" + varcharType.getLengthSafe() + ")"; + } + return WriteMapping.sliceWriteMapping(dataType, varcharWriteFunction()); } if (type instanceof CharType) { - if (((CharType) type).getLength() == CharType.MAX_LENGTH) { - return "char"; + CharType charType = (CharType) type; + String dataType; + if (charType.getLength() == CharType.MAX_LENGTH) { + dataType = "char"; + } + else { + dataType = "char(" + charType.getLength() + ")"; } - return "char(" + ((CharType) type).getLength() + ")"; + return WriteMapping.sliceWriteMapping(dataType, charWriteFunction(charType)); } if (type instanceof DecimalType) { - return format("decimal(%s, %s)", ((DecimalType) type).getPrecision(), ((DecimalType) type).getScale()); + DecimalType decimalType = (DecimalType) type; + String dataType = format("decimal(%s, %s)", decimalType.getPrecision(), decimalType.getScale()); + if (decimalType.isShort()) { + return longWriteMapping(dataType, shortDecimalWriteFunction(decimalType)); + } + return WriteMapping.sliceWriteMapping(dataType, longDecimalWriteFunction(decimalType)); } - String sqlType = SQL_TYPES.get(type); - if (sqlType != null) { - return sqlType; + WriteMapping writeMapping = WRITE_MAPPINGS.get(type); + if (writeMapping != null) { + return writeMapping; } throw new PrestoException(NOT_SUPPORTED, "Unsupported column type: " + type.getDisplayName()); } diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/WriteMapping.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/WriteMapping.java new file mode 100644 index 000000000000..85fb2a83eeed --- /dev/null +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/WriteMapping.java @@ -0,0 +1,67 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not 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 applicable law 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 com.facebook.presto.plugin.jdbc; + +import static com.google.common.base.MoreObjects.toStringHelper; +import static java.util.Objects.requireNonNull; + +public final class WriteMapping +{ + public static WriteMapping booleanWriteMapping(String dataType, BooleanWriteFunction writeFunction) + { + return new WriteMapping(dataType, writeFunction); + } + + public static WriteMapping longWriteMapping(String dataType, LongWriteFunction writeFunction) + { + return new WriteMapping(dataType, writeFunction); + } + + public static WriteMapping doubleWriteMapping(String dataType, DoubleWriteFunction writeFunction) + { + return new WriteMapping(dataType, writeFunction); + } + + public static WriteMapping sliceWriteMapping(String dataType, SliceWriteFunction writeFunction) + { + return new WriteMapping(dataType, writeFunction); + } + + private final String dataType; + private final WriteFunction writeFunction; + + private WriteMapping(String dataType, WriteFunction writeFunction) + { + this.dataType = requireNonNull(dataType, "dataType is null"); + this.writeFunction = requireNonNull(writeFunction, "writeFunction is null"); + } + + public String getDataType() + { + return dataType; + } + + public WriteFunction getWriteFunction() + { + return writeFunction; + } + + @Override + public String toString() + { + return toStringHelper(this) + .add("dataType", dataType) + .toString(); + } +} diff --git a/presto-mysql/src/main/java/com/facebook/presto/plugin/mysql/MySqlClient.java b/presto-mysql/src/main/java/com/facebook/presto/plugin/mysql/MySqlClient.java index 9a694e3cc4c0..2b0befc27fe4 100644 --- a/presto-mysql/src/main/java/com/facebook/presto/plugin/mysql/MySqlClient.java +++ b/presto-mysql/src/main/java/com/facebook/presto/plugin/mysql/MySqlClient.java @@ -18,6 +18,7 @@ import com.facebook.presto.plugin.jdbc.ConnectionFactory; import com.facebook.presto.plugin.jdbc.DriverConnectionFactory; import com.facebook.presto.plugin.jdbc.JdbcConnectorId; +import com.facebook.presto.plugin.jdbc.WriteMapping; import com.facebook.presto.spi.PrestoException; import com.facebook.presto.spi.SchemaTableName; import com.facebook.presto.spi.type.Type; @@ -37,6 +38,12 @@ import java.util.Set; import static com.facebook.presto.plugin.jdbc.DriverConnectionFactory.basicConnectionProperties; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.realWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.timestampWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.varbinaryWriteFunction; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.varcharWriteFunction; +import static com.facebook.presto.plugin.jdbc.WriteMapping.longWriteMapping; +import static com.facebook.presto.plugin.jdbc.WriteMapping.sliceWriteMapping; import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; import static com.facebook.presto.spi.type.RealType.REAL; import static com.facebook.presto.spi.type.TimeWithTimeZoneType.TIME_WITH_TIME_ZONE; @@ -143,37 +150,41 @@ protected SchemaTableName getSchemaTableName(ResultSet resultSet) } @Override - protected String toSqlType(Type type) + protected WriteMapping toWriteMapping(Type type) { if (REAL.equals(type)) { - return "float"; + return longWriteMapping("float", realWriteFunction()); } if (TIME_WITH_TIME_ZONE.equals(type) || TIMESTAMP_WITH_TIME_ZONE.equals(type)) { throw new PrestoException(NOT_SUPPORTED, "Unsupported column type: " + type.getDisplayName()); } if (TIMESTAMP.equals(type)) { - return "datetime"; + return longWriteMapping("datetime", timestampWriteFunction()); } if (VARBINARY.equals(type)) { - return "mediumblob"; + return sliceWriteMapping("mediumblob", varbinaryWriteFunction()); } if (isVarcharType(type)) { VarcharType varcharType = (VarcharType) type; + String dataType; if (varcharType.isUnbounded()) { - return "longtext"; + dataType = "longtext"; } - if (varcharType.getLengthSafe() <= 255) { - return "tinytext"; + else if (varcharType.getLengthSafe() <= 255) { + dataType = "tinytext"; } - if (varcharType.getLengthSafe() <= 65535) { - return "text"; + else if (varcharType.getLengthSafe() <= 65535) { + dataType = "text"; } - if (varcharType.getLengthSafe() <= 16777215) { - return "mediumtext"; + else if (varcharType.getLengthSafe() <= 16777215) { + dataType = "mediumtext"; } - return "longtext"; + else { + dataType = "longtext"; + } + return sliceWriteMapping(dataType, varcharWriteFunction()); } - return super.toSqlType(type); + return super.toWriteMapping(type); } } diff --git a/presto-postgresql/src/main/java/com/facebook/presto/plugin/postgresql/PostgreSqlClient.java b/presto-postgresql/src/main/java/com/facebook/presto/plugin/postgresql/PostgreSqlClient.java index 4fbb8bf63528..118c59eb2477 100644 --- a/presto-postgresql/src/main/java/com/facebook/presto/plugin/postgresql/PostgreSqlClient.java +++ b/presto-postgresql/src/main/java/com/facebook/presto/plugin/postgresql/PostgreSqlClient.java @@ -18,6 +18,7 @@ import com.facebook.presto.plugin.jdbc.DriverConnectionFactory; import com.facebook.presto.plugin.jdbc.JdbcConnectorId; import com.facebook.presto.plugin.jdbc.JdbcOutputTableHandle; +import com.facebook.presto.plugin.jdbc.WriteMapping; import com.facebook.presto.spi.type.Type; import org.postgresql.Driver; @@ -29,6 +30,7 @@ import java.sql.ResultSet; import java.sql.SQLException; +import static com.facebook.presto.plugin.jdbc.StandardColumnMappings.varbinaryWriteFunction; import static com.facebook.presto.spi.type.VarbinaryType.VARBINARY; public class PostgreSqlClient @@ -82,12 +84,12 @@ protected ResultSet getTables(Connection connection, String schemaName, String t } @Override - protected String toSqlType(Type type) + protected WriteMapping toWriteMapping(Type type) { if (VARBINARY.equals(type)) { - return "bytea"; + return WriteMapping.sliceWriteMapping("bytea", varbinaryWriteFunction()); } - return super.toSqlType(type); + return super.toWriteMapping(type); } } From 8508d0433a8f8a264a07e72e9904120aa7c01a8b Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 31 Dec 2018 01:24:51 +0100 Subject: [PATCH 7/9] Use WriteFunction to bind data in JdbcPageSink --- .../presto/plugin/jdbc/BaseJdbcClient.java | 3 +- .../presto/plugin/jdbc/JdbcClient.java | 3 + .../presto/plugin/jdbc/JdbcPageSink.java | 91 +++++++------------ .../plugin/jdbc/JdbcPageSinkProvider.java | 4 +- .../presto/plugin/mysql/MySqlClient.java | 2 +- .../plugin/postgresql/PostgreSqlClient.java | 2 +- 6 files changed, 42 insertions(+), 63 deletions(-) diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java index d7d48fc101bb..13f0900c38cb 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java @@ -484,7 +484,8 @@ protected final String toSqlType(Type type) return toWriteMapping(type).getDataType(); } - protected WriteMapping toWriteMapping(Type type) + @Override + public WriteMapping toWriteMapping(Type type) { if (isVarcharType(type)) { VarcharType varcharType = (VarcharType) type; diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java index 0719a58684fb..801e7a3f73ed 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java @@ -17,6 +17,7 @@ import com.facebook.presto.spi.ConnectorSplitSource; import com.facebook.presto.spi.ConnectorTableMetadata; import com.facebook.presto.spi.SchemaTableName; +import com.facebook.presto.spi.type.Type; import javax.annotation.Nullable; @@ -45,6 +46,8 @@ default boolean schemaExists(String schema) Optional toPrestoType(ConnectorSession session, JdbcTypeHandle typeHandle); + WriteMapping toWriteMapping(Type type); + ConnectorSplitSource getSplits(JdbcTableLayoutHandle layoutHandle); Connection getConnection(JdbcSplit split) diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcPageSink.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcPageSink.java index 97ae81e6c06d..7292aad289aa 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcPageSink.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcPageSink.java @@ -14,19 +14,16 @@ package com.facebook.presto.plugin.jdbc; import com.facebook.presto.spi.ConnectorPageSink; +import com.facebook.presto.spi.ConnectorSession; import com.facebook.presto.spi.Page; import com.facebook.presto.spi.PrestoException; import com.facebook.presto.spi.block.Block; -import com.facebook.presto.spi.type.DecimalType; import com.facebook.presto.spi.type.Type; +import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableList; -import com.google.common.primitives.Shorts; -import com.google.common.primitives.SignedBytes; import io.airlift.slice.Slice; -import org.joda.time.DateTimeZone; import java.sql.Connection; -import java.sql.Date; import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.SQLNonTransientException; @@ -36,24 +33,10 @@ import static com.facebook.presto.plugin.jdbc.JdbcErrorCode.JDBC_ERROR; import static com.facebook.presto.plugin.jdbc.JdbcErrorCode.JDBC_NON_TRANSIENT_ERROR; -import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; -import static com.facebook.presto.spi.type.BigintType.BIGINT; -import static com.facebook.presto.spi.type.BooleanType.BOOLEAN; -import static com.facebook.presto.spi.type.Chars.isCharType; -import static com.facebook.presto.spi.type.DateType.DATE; -import static com.facebook.presto.spi.type.Decimals.readBigDecimal; -import static com.facebook.presto.spi.type.DoubleType.DOUBLE; -import static com.facebook.presto.spi.type.IntegerType.INTEGER; -import static com.facebook.presto.spi.type.RealType.REAL; -import static com.facebook.presto.spi.type.SmallintType.SMALLINT; -import static com.facebook.presto.spi.type.TinyintType.TINYINT; -import static com.facebook.presto.spi.type.VarbinaryType.VARBINARY; -import static com.facebook.presto.spi.type.Varchars.isVarcharType; -import static java.lang.Float.intBitsToFloat; -import static java.lang.Math.toIntExact; +import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.ImmutableList.toImmutableList; +import static java.lang.String.format; import static java.util.concurrent.CompletableFuture.completedFuture; -import static java.util.concurrent.TimeUnit.DAYS; -import static org.joda.time.chrono.ISOChronology.getInstanceUTC; public class JdbcPageSink implements ConnectorPageSink @@ -62,9 +45,10 @@ public class JdbcPageSink private final PreparedStatement statement; private final List columnTypes; + private final List columnWriters; private int batchSize; - public JdbcPageSink(JdbcOutputTableHandle handle, JdbcClient jdbcClient) + public JdbcPageSink(JdbcOutputTableHandle handle, ConnectorSession session, JdbcClient jdbcClient) { try { connection = jdbcClient.getConnection(handle); @@ -83,6 +67,19 @@ public JdbcPageSink(JdbcOutputTableHandle handle, JdbcClient jdbcClient) } columnTypes = handle.getColumnTypes(); + + columnWriters = columnTypes.stream() + .map(type -> { + WriteFunction writeFunction = jdbcClient.toWriteMapping(type).getWriteFunction(); + checkState( + type.getJavaType() == writeFunction.getJavaType(), + "Presto type %s is not compatible with write function %s accepting %s", + type, + writeFunction, + writeFunction.getJavaType()); + return writeFunction; + }) + .collect(toImmutableList()); } @Override @@ -115,52 +112,30 @@ private void appendColumn(Page page, int position, int channel) throws SQLException { Block block = page.getBlock(channel); - int parameter = channel + 1; + int parameterIndex = channel + 1; if (block.isNull(position)) { - statement.setObject(parameter, null); + statement.setObject(parameterIndex, null); return; } Type type = columnTypes.get(channel); - if (BOOLEAN.equals(type)) { - statement.setBoolean(parameter, type.getBoolean(block, position)); - } - else if (BIGINT.equals(type)) { - statement.setLong(parameter, type.getLong(block, position)); - } - else if (INTEGER.equals(type)) { - statement.setInt(parameter, toIntExact(type.getLong(block, position))); - } - else if (SMALLINT.equals(type)) { - statement.setShort(parameter, Shorts.checkedCast(type.getLong(block, position))); - } - else if (TINYINT.equals(type)) { - statement.setByte(parameter, SignedBytes.checkedCast(type.getLong(block, position))); - } - else if (DOUBLE.equals(type)) { - statement.setDouble(parameter, type.getDouble(block, position)); - } - else if (REAL.equals(type)) { - statement.setFloat(parameter, intBitsToFloat(toIntExact(type.getLong(block, position)))); - } - else if (type instanceof DecimalType) { - statement.setBigDecimal(parameter, readBigDecimal((DecimalType) type, block, position)); + Class javaType = type.getJavaType(); + WriteFunction writeFunction = columnWriters.get(channel); + if (javaType == boolean.class) { + ((BooleanWriteFunction) writeFunction).set(statement, parameterIndex, type.getBoolean(block, position)); } - else if (isVarcharType(type) || isCharType(type)) { - statement.setString(parameter, type.getSlice(block, position).toStringUtf8()); + else if (javaType == long.class) { + ((LongWriteFunction) writeFunction).set(statement, parameterIndex, type.getLong(block, position)); } - else if (VARBINARY.equals(type)) { - statement.setBytes(parameter, type.getSlice(block, position).getBytes()); + else if (javaType == double.class) { + ((DoubleWriteFunction) writeFunction).set(statement, parameterIndex, type.getDouble(block, position)); } - else if (DATE.equals(type)) { - // convert to midnight in default time zone - long utcMillis = DAYS.toMillis(type.getLong(block, position)); - long localMillis = getInstanceUTC().getZone().getMillisKeepLocal(DateTimeZone.getDefault(), utcMillis); - statement.setDate(parameter, new Date(localMillis)); + else if (javaType == Slice.class) { + ((SliceWriteFunction) writeFunction).set(statement, parameterIndex, type.getSlice(block, position)); } else { - throw new PrestoException(NOT_SUPPORTED, "Unsupported column type: " + type.getDisplayName()); + throw new VerifyException(format("Unexpected type %s with java type %s", type, javaType)); } } diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcPageSinkProvider.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcPageSinkProvider.java index 10e44ead0dcc..3791796e95cc 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcPageSinkProvider.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcPageSinkProvider.java @@ -38,12 +38,12 @@ public JdbcPageSinkProvider(JdbcClient jdbcClient) @Override public ConnectorPageSink createPageSink(ConnectorTransactionHandle transactionHandle, ConnectorSession session, ConnectorOutputTableHandle tableHandle) { - return new JdbcPageSink((JdbcOutputTableHandle) tableHandle, jdbcClient); + return new JdbcPageSink((JdbcOutputTableHandle) tableHandle, session, jdbcClient); } @Override public ConnectorPageSink createPageSink(ConnectorTransactionHandle transactionHandle, ConnectorSession session, ConnectorInsertTableHandle tableHandle) { - return new JdbcPageSink((JdbcOutputTableHandle) tableHandle, jdbcClient); + return new JdbcPageSink((JdbcOutputTableHandle) tableHandle, session, jdbcClient); } } diff --git a/presto-mysql/src/main/java/com/facebook/presto/plugin/mysql/MySqlClient.java b/presto-mysql/src/main/java/com/facebook/presto/plugin/mysql/MySqlClient.java index 2b0befc27fe4..6c253dc5242c 100644 --- a/presto-mysql/src/main/java/com/facebook/presto/plugin/mysql/MySqlClient.java +++ b/presto-mysql/src/main/java/com/facebook/presto/plugin/mysql/MySqlClient.java @@ -150,7 +150,7 @@ protected SchemaTableName getSchemaTableName(ResultSet resultSet) } @Override - protected WriteMapping toWriteMapping(Type type) + public WriteMapping toWriteMapping(Type type) { if (REAL.equals(type)) { return longWriteMapping("float", realWriteFunction()); diff --git a/presto-postgresql/src/main/java/com/facebook/presto/plugin/postgresql/PostgreSqlClient.java b/presto-postgresql/src/main/java/com/facebook/presto/plugin/postgresql/PostgreSqlClient.java index 118c59eb2477..2e1ff73d97bc 100644 --- a/presto-postgresql/src/main/java/com/facebook/presto/plugin/postgresql/PostgreSqlClient.java +++ b/presto-postgresql/src/main/java/com/facebook/presto/plugin/postgresql/PostgreSqlClient.java @@ -84,7 +84,7 @@ protected ResultSet getTables(Connection connection, String schemaName, String t } @Override - protected WriteMapping toWriteMapping(Type type) + public WriteMapping toWriteMapping(Type type) { if (VARBINARY.equals(type)) { return WriteMapping.sliceWriteMapping("bytea", varbinaryWriteFunction()); From 6309fa85b6f1caf263ab63d6cf943bd2c912cc70 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Thu, 3 Jan 2019 01:53:20 +0100 Subject: [PATCH 8/9] Use WriteFunction to bind parameters in QueryBuilder --- .../presto/plugin/jdbc/BaseJdbcClient.java | 3 +- .../presto/plugin/jdbc/JdbcClient.java | 2 +- .../presto/plugin/jdbc/JdbcRecordCursor.java | 2 +- .../presto/plugin/jdbc/QueryBuilder.java | 101 +++++++++--------- .../plugin/jdbc/TestJdbcQueryBuilder.java | 15 +-- 5 files changed, 61 insertions(+), 62 deletions(-) diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java index 13f0900c38cb..a84617c05ef6 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/BaseJdbcClient.java @@ -265,11 +265,12 @@ public Connection getConnection(JdbcSplit split) } @Override - public PreparedStatement buildSql(Connection connection, JdbcSplit split, List columnHandles) + public PreparedStatement buildSql(ConnectorSession session, Connection connection, JdbcSplit split, List columnHandles) throws SQLException { return new QueryBuilder(identifierQuote).buildSql( this, + session, connection, split.getCatalogName(), split.getSchemaName(), diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java index 801e7a3f73ed..f32d3c31436e 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java @@ -59,7 +59,7 @@ default void abortReadConnection(Connection connection) // most drivers do not need this } - PreparedStatement buildSql(Connection connection, JdbcSplit split, List columnHandles) + PreparedStatement buildSql(ConnectorSession session, Connection connection, JdbcSplit split, List columnHandles) throws SQLException; JdbcOutputTableHandle beginCreateTable(ConnectorTableMetadata tableMetadata); diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcRecordCursor.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcRecordCursor.java index 4adda8fd638d..a9d345cd3838 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcRecordCursor.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcRecordCursor.java @@ -87,7 +87,7 @@ else if (javaType == Slice.class) { try { connection = jdbcClient.getConnection(split); - statement = jdbcClient.buildSql(connection, split, columnHandles); + statement = jdbcClient.buildSql(session, connection, split, columnHandles); log.debug("Executing: %s", statement.toString()); resultSet = statement.executeQuery(); } diff --git a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/QueryBuilder.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/QueryBuilder.java index 6bc020313d80..3489d7db5a3f 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/QueryBuilder.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/QueryBuilder.java @@ -14,6 +14,7 @@ package com.facebook.presto.plugin.jdbc; import com.facebook.presto.spi.ColumnHandle; +import com.facebook.presto.spi.ConnectorSession; import com.facebook.presto.spi.predicate.Domain; import com.facebook.presto.spi.predicate.Range; import com.facebook.presto.spi.predicate.TupleDomain; @@ -31,16 +32,13 @@ import com.facebook.presto.spi.type.Type; import com.facebook.presto.spi.type.VarcharType; import com.google.common.base.Joiner; +import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableList; import io.airlift.slice.Slice; -import org.joda.time.DateTimeZone; import java.sql.Connection; -import java.sql.Date; import java.sql.PreparedStatement; import java.sql.SQLException; -import java.sql.Time; -import java.sql.Timestamp; import java.util.ArrayList; import java.util.List; @@ -48,12 +46,10 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.collect.Iterables.getOnlyElement; -import static java.lang.Float.intBitsToFloat; +import static java.lang.String.format; import static java.util.Collections.nCopies; import static java.util.Objects.requireNonNull; -import static java.util.concurrent.TimeUnit.DAYS; import static java.util.stream.Collectors.joining; -import static org.joda.time.DateTimeZone.UTC; public class QueryBuilder { @@ -66,11 +62,13 @@ public class QueryBuilder private static class TypeAndValue { private final Type type; + private final JdbcTypeHandle typeHandle; private final Object value; - public TypeAndValue(Type type, Object value) + public TypeAndValue(Type type, JdbcTypeHandle typeHandle, Object value) { this.type = requireNonNull(type, "type is null"); + this.typeHandle = requireNonNull(typeHandle, "typeHandle is null"); this.value = requireNonNull(value, "value is null"); } @@ -79,6 +77,11 @@ public Type getType() return type; } + public JdbcTypeHandle getTypeHandle() + { + return typeHandle; + } + public Object getValue() { return value; @@ -90,7 +93,15 @@ public QueryBuilder(String quote) this.quote = requireNonNull(quote, "quote is null"); } - public PreparedStatement buildSql(JdbcClient client, Connection connection, String catalog, String schema, String table, List columns, TupleDomain tupleDomain) + public PreparedStatement buildSql( + JdbcClient client, + ConnectorSession session, + Connection connection, + String catalog, + String schema, + String table, + List columns, + TupleDomain tupleDomain) throws SQLException { StringBuilder sql = new StringBuilder(); @@ -127,45 +138,30 @@ public PreparedStatement buildSql(JdbcClient client, Connection connection, Stri for (int i = 0; i < accumulator.size(); i++) { TypeAndValue typeAndValue = accumulator.get(i); - if (typeAndValue.getType().equals(BigintType.BIGINT)) { - statement.setLong(i + 1, (long) typeAndValue.getValue()); - } - else if (typeAndValue.getType().equals(IntegerType.INTEGER)) { - statement.setInt(i + 1, ((Number) typeAndValue.getValue()).intValue()); - } - else if (typeAndValue.getType().equals(SmallintType.SMALLINT)) { - statement.setShort(i + 1, ((Number) typeAndValue.getValue()).shortValue()); - } - else if (typeAndValue.getType().equals(TinyintType.TINYINT)) { - statement.setByte(i + 1, ((Number) typeAndValue.getValue()).byteValue()); + int parameterIndex = i + 1; + Type type = typeAndValue.getType(); + WriteFunction writeFunction = client.toPrestoType(session, typeAndValue.getTypeHandle()) + .orElseThrow(() -> new IllegalStateException(format("Unsupported type %s with handle %s", type, typeAndValue.getTypeHandle()))) + .getWriteFunction(); + Class javaType = type.getJavaType(); + Object value = typeAndValue.getValue(); + if (javaType == boolean.class) { + ((BooleanWriteFunction) writeFunction).set(statement, parameterIndex, (boolean) value); } - else if (typeAndValue.getType().equals(DoubleType.DOUBLE)) { - statement.setDouble(i + 1, (double) typeAndValue.getValue()); + else if (javaType == long.class) { + ((LongWriteFunction) writeFunction).set(statement, parameterIndex, (long) value); } - else if (typeAndValue.getType().equals(RealType.REAL)) { - statement.setFloat(i + 1, intBitsToFloat(((Number) typeAndValue.getValue()).intValue())); + else if (javaType == double.class) { + ((DoubleWriteFunction) writeFunction).set(statement, parameterIndex, (double) value); } - else if (typeAndValue.getType().equals(BooleanType.BOOLEAN)) { - statement.setBoolean(i + 1, (boolean) typeAndValue.getValue()); - } - else if (typeAndValue.getType().equals(DateType.DATE)) { - long millis = DAYS.toMillis((long) typeAndValue.getValue()); - statement.setDate(i + 1, new Date(UTC.getMillisKeepLocal(DateTimeZone.getDefault(), millis))); - } - else if (typeAndValue.getType().equals(TimeType.TIME)) { - statement.setTime(i + 1, new Time((long) typeAndValue.getValue())); - } - else if (typeAndValue.getType().equals(TimestampType.TIMESTAMP)) { - statement.setTimestamp(i + 1, new Timestamp((long) typeAndValue.getValue())); - } - else if (typeAndValue.getType() instanceof VarcharType) { - statement.setString(i + 1, ((Slice) typeAndValue.getValue()).toStringUtf8()); + else if (javaType == Slice.class) { + ((SliceWriteFunction) writeFunction).set(statement, parameterIndex, (Slice) value); } else if (typeAndValue.getType() instanceof CharType) { statement.setString(i + 1, ((Slice) typeAndValue.getValue()).toStringUtf8()); } else { - throw new UnsupportedOperationException("Can't handle type: " + typeAndValue.getType()); + throw new VerifyException(format("Unexpected type %s with java type %s", type, javaType)); } } @@ -197,14 +193,14 @@ private List toConjuncts(List columns, TupleDomain accumulator) + private String toPredicate(String columnName, Domain domain, JdbcColumnHandle column, List accumulator) { checkArgument(domain.getType().isOrderable(), "Domain type must be orderable"); @@ -228,10 +224,10 @@ private String toPredicate(String columnName, Domain domain, Type type, List", range.getLow().getValue(), type, accumulator)); + rangeConjuncts.add(toPredicate(columnName, ">", range.getLow().getValue(), column, accumulator)); break; case EXACTLY: - rangeConjuncts.add(toPredicate(columnName, ">=", range.getLow().getValue(), type, accumulator)); + rangeConjuncts.add(toPredicate(columnName, ">=", range.getLow().getValue(), column, accumulator)); break; case BELOW: throw new IllegalArgumentException("Low marker should never use BELOW bound"); @@ -244,10 +240,10 @@ private String toPredicate(String columnName, Domain domain, Type type, List 1) { for (Object value : singleValues) { - bindValue(value, type, accumulator); + bindValue(value, column, accumulator); } String values = Joiner.on(",").join(nCopies(singleValues.size(), "?")); disjuncts.add(quote(columnName) + " IN (" + values + ")"); @@ -280,9 +276,9 @@ else if (singleValues.size() > 1) { return "(" + Joiner.on(" OR ").join(disjuncts) + ")"; } - private String toPredicate(String columnName, String operator, Object value, Type type, List accumulator) + private String toPredicate(String columnName, String operator, Object value, JdbcColumnHandle column, List accumulator) { - bindValue(value, type, accumulator); + bindValue(value, column, accumulator); return quote(columnName) + " " + operator + " ?"; } @@ -292,9 +288,10 @@ private String quote(String name) return quote + name + quote; } - private static void bindValue(Object value, Type type, List accumulator) + private static void bindValue(Object value, JdbcColumnHandle column, List accumulator) { + Type type = column.getColumnType(); checkArgument(isAcceptedType(type), "Can't handle type: %s", type); - accumulator.add(new TypeAndValue(type, value)); + accumulator.add(new TypeAndValue(type, column.getJdbcTypeHandle(), value)); } } diff --git a/presto-base-jdbc/src/test/java/com/facebook/presto/plugin/jdbc/TestJdbcQueryBuilder.java b/presto-base-jdbc/src/test/java/com/facebook/presto/plugin/jdbc/TestJdbcQueryBuilder.java index c3106e01270d..896efeb93a2f 100644 --- a/presto-base-jdbc/src/test/java/com/facebook/presto/plugin/jdbc/TestJdbcQueryBuilder.java +++ b/presto-base-jdbc/src/test/java/com/facebook/presto/plugin/jdbc/TestJdbcQueryBuilder.java @@ -61,6 +61,7 @@ import static com.facebook.presto.spi.type.TimestampType.TIMESTAMP; import static com.facebook.presto.spi.type.TinyintType.TINYINT; import static com.facebook.presto.spi.type.VarcharType.VARCHAR; +import static com.facebook.presto.testing.TestingConnectorSession.SESSION; import static io.airlift.slice.Slices.utf8Slice; import static io.airlift.testing.Assertions.assertContains; import static java.lang.Float.floatToRawIntBits; @@ -196,7 +197,7 @@ public void testNormalBuildSql() .build()); Connection connection = database.getConnection(); - try (PreparedStatement preparedStatement = new QueryBuilder("\"").buildSql(jdbcClient, connection, "", "", "test_table", columns, tupleDomain); + try (PreparedStatement preparedStatement = new QueryBuilder("\"").buildSql(jdbcClient, SESSION, connection, "", "", "test_table", columns, tupleDomain); ResultSet resultSet = preparedStatement.executeQuery()) { ImmutableSet.Builder builder = ImmutableSet.builder(); while (resultSet.next()) { @@ -219,7 +220,7 @@ public void testBuildSqlWithFloat() false))); Connection connection = database.getConnection(); - try (PreparedStatement preparedStatement = new QueryBuilder("\"").buildSql(jdbcClient, connection, "", "", "test_table", columns, tupleDomain); + try (PreparedStatement preparedStatement = new QueryBuilder("\"").buildSql(jdbcClient, SESSION, connection, "", "", "test_table", columns, tupleDomain); ResultSet resultSet = preparedStatement.executeQuery()) { ImmutableSet.Builder longBuilder = ImmutableSet.builder(); ImmutableSet.Builder floatBuilder = ImmutableSet.builder(); @@ -245,7 +246,7 @@ public void testBuildSqlWithVarchar() false))); Connection connection = database.getConnection(); - try (PreparedStatement preparedStatement = new QueryBuilder("\"").buildSql(jdbcClient, connection, "", "", "test_table", columns, tupleDomain); + try (PreparedStatement preparedStatement = new QueryBuilder("\"").buildSql(jdbcClient, SESSION, connection, "", "", "test_table", columns, tupleDomain); ResultSet resultSet = preparedStatement.executeQuery()) { ImmutableSet.Builder builder = ImmutableSet.builder(); while (resultSet.next()) { @@ -273,7 +274,7 @@ public void testBuildSqlWithChar() false))); Connection connection = database.getConnection(); - try (PreparedStatement preparedStatement = new QueryBuilder("\"").buildSql(jdbcClient, connection, "", "", "test_table", columns, tupleDomain); + try (PreparedStatement preparedStatement = new QueryBuilder("\"").buildSql(jdbcClient, SESSION, connection, "", "", "test_table", columns, tupleDomain); ResultSet resultSet = preparedStatement.executeQuery()) { ImmutableSet.Builder builder = ImmutableSet.builder(); while (resultSet.next()) { @@ -306,7 +307,7 @@ public void testBuildSqlWithDateTime() false))); Connection connection = database.getConnection(); - try (PreparedStatement preparedStatement = new QueryBuilder("\"").buildSql(jdbcClient, connection, "", "", "test_table", columns, tupleDomain); + try (PreparedStatement preparedStatement = new QueryBuilder("\"").buildSql(jdbcClient, SESSION, connection, "", "", "test_table", columns, tupleDomain); ResultSet resultSet = preparedStatement.executeQuery()) { ImmutableSet.Builder dateBuilder = ImmutableSet.builder(); ImmutableSet.Builder