Skip to content

Support coercing char types of different lengths in hive #16402

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
Praveen2112 merged 2 commits into from
Mar 21, 2023
Merged

Support coercing char types of different lengths in hive #16402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1f7731
Replace usage of Arrays#asList with ImmutableList#of
Praveen2112 Mar 17, 2023
8542ea2
Support coercing char types in hive connector
Praveen2112 Mar 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions plugin/trino-hive/src/main/java/io/trino/plugin/hive/HivePageSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableList;
import io.trino.plugin.hive.HivePageSourceProvider.BucketAdaptation;
import io.trino.plugin.hive.HivePageSourceProvider.ColumnMapping;
import io.trino.plugin.hive.coercions.CharCoercer;
import io.trino.plugin.hive.coercions.DoubleToFloatCoercer;
import io.trino.plugin.hive.coercions.FloatToDoubleCoercer;
import io.trino.plugin.hive.coercions.IntegerNumberToVarcharCoercer;
Expand Down Expand Up @@ -43,6 +44,7 @@
import io.trino.spi.connector.RecordCursor;
import io.trino.spi.metrics.Metrics;
import io.trino.spi.type.ArrayType;
import io.trino.spi.type.CharType;
import io.trino.spi.type.DecimalType;
import io.trino.spi.type.MapType;
import io.trino.spi.type.RowType;
Expand Down Expand Up @@ -313,6 +315,12 @@ private static Optional<Function<Block, Block>> createCoercer(TypeManager typeMa
}
return Optional.empty();
}
if (fromType instanceof CharType fromCharType && toType instanceof CharType toCharType) {
if (narrowerThan(toCharType, fromCharType)) {
return Optional.of(new CharCoercer(fromCharType, toCharType));
}
return Optional.empty();
}
if (fromHiveType.equals(HIVE_BYTE) && (toHiveType.equals(HIVE_SHORT) || toHiveType.equals(HIVE_INT) || toHiveType.equals(HIVE_LONG))) {
return Optional.of(new IntegerNumberUpscaleCoercer<>(fromType, toType));
}
Expand Down Expand Up @@ -372,6 +380,13 @@ public static boolean narrowerThan(VarcharType first, VarcharType second)
return first.getBoundedLength() < second.getBoundedLength();
}

public static boolean narrowerThan(CharType first, CharType second)
Copy link
Copy Markdown
Member

@ebyhr ebyhr Mar 13, 2023

Choose a reason for hiding this comment

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

nit: We use "short" at least in io.trino.spi.type.Chars class. No requested change as there's the existing narrowerThan method.

Suggested change
public static boolean narrowerThan(CharType first, CharType second)
public static boolean shorterThan(CharType first, CharType second)

{
requireNonNull(first, "first is null");
requireNonNull(second, "second is null");
return first.getLength() < second.getLength();
}

private static class ListCoercer
implements Function<Block, Block>
{
Expand Down
40 changes: 40 additions & 0 deletions plugin/trino-hive/src/main/java/io/trino/plugin/hive/coercions/CharCoercer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* 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 io.trino.plugin.hive.coercions;

import io.airlift.slice.Slice;
import io.trino.spi.block.Block;
import io.trino.spi.block.BlockBuilder;
import io.trino.spi.type.CharType;

import static com.google.common.base.Preconditions.checkArgument;
import static io.trino.plugin.hive.HivePageSource.narrowerThan;
import static io.trino.spi.type.Chars.truncateToLengthAndTrimSpaces;

public class CharCoercer
extends TypeCoercer<CharType, CharType>
{
public CharCoercer(CharType fromType, CharType toType)
{
super(fromType, toType);
checkArgument(narrowerThan(toType, fromType), "Coercer to a wider char type should not be required");
}

@Override
protected void applyCoercedValue(BlockBuilder blockBuilder, Block block, int position)
{
Slice value = fromType.getSlice(block, position);
toType.writeSlice(blockBuilder, truncateToLengthAndTrimSpaces(value, toType));
}
}
4 changes: 4 additions & 0 deletions plugin/trino-hive/src/main/java/io/trino/plugin/hive/util/HiveCoercionPolicy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import io.trino.plugin.hive.type.ListTypeInfo;
import io.trino.plugin.hive.type.MapTypeInfo;
import io.trino.plugin.hive.type.StructTypeInfo;
import io.trino.spi.type.CharType;
import io.trino.spi.type.DecimalType;
import io.trino.spi.type.Type;
import io.trino.spi.type.TypeManager;
Expand Down Expand Up @@ -63,6 +64,9 @@ private boolean canCoerce(HiveType fromHiveType, HiveType toHiveType, HiveTimest
toHiveType.equals(HIVE_INT) ||
toHiveType.equals(HIVE_LONG);
}
if (fromType instanceof CharType) {
return toType instanceof CharType;
}
if (toType instanceof VarcharType) {
return fromHiveType.equals(HIVE_BYTE) || fromHiveType.equals(HIVE_SHORT) || fromHiveType.equals(HIVE_INT) || fromHiveType.equals(HIVE_LONG) || fromType instanceof DecimalType;
}
Expand Down
73 changes: 46 additions & 27 deletions ...g/trino-product-tests/src/main/java/io/trino/tests/product/hive/BaseTestHiveCoercion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import static java.lang.String.format;
import static java.sql.JDBCType.ARRAY;
import static java.sql.JDBCType.BIGINT;
import static java.sql.JDBCType.CHAR;
import static java.sql.JDBCType.DECIMAL;
import static java.sql.JDBCType.DOUBLE;
import static java.sql.JDBCType.FLOAT;
Expand Down Expand Up @@ -108,6 +109,8 @@ protected void doTestHiveCoercion(HiveTableDefinition tableDefinition)
"long_decimal_to_bounded_varchar",
"varchar_to_bigger_varchar",
"varchar_to_smaller_varchar",
"char_to_bigger_char",
"char_to_smaller_char",
"id");

Function<Engine, Map<String, List<Object>>> expected = engine -> expectedValuesForEngineProvider(engine, tableName, decimalToFloatVal, floatToDecimalVal);
Expand Down Expand Up @@ -162,6 +165,8 @@ protected void insertTableRows(String tableName, String floatToDoubleType)
" DECIMAL '12345678.123456123456', " +
" 'abc', " +
" 'abc', " +
" 'abc', " +
" 'abc', " +
" 1), " +
"(" +
" CAST(ROW (NULL, 1, -100, -2323, -12345, 2) AS ROW(keep VARCHAR, ti2si TINYINT, si2int SMALLINT, int2bi INTEGER, bi2vc BIGINT, lower2uppercase BIGINT)), " +
Expand Down Expand Up @@ -190,6 +195,8 @@ protected void insertTableRows(String tableName, String floatToDoubleType)
" DECIMAL '-12345678.123456123456', " +
" '\uD83D\uDCB0\uD83D\uDCB0\uD83D\uDCB0', " +
" '\uD83D\uDCB0\uD83D\uDCB0\uD83D\uDCB0', " +
" '\uD83D\uDCB0\uD83D\uDCB0\uD83D\uDCB0', " +
" '\uD83D\uDCB0\uD83D\uDCB0\uD83D\uDCB0', " +
" 1)",
tableName,
floatToDoubleType));
Expand All @@ -210,7 +217,7 @@ else if (getHiveVersionMajor() == 3 && isFormat.test("orc")) {
}

return ImmutableMap.<String, List<Object>>builder()
.put("row_to_row", Arrays.asList(
.put("row_to_row", ImmutableList.of(
engine == Engine.TRINO ?
rowBuilder()
.addField("keep", "as is")
Expand All @@ -232,7 +239,7 @@ else if (getHiveVersionMajor() == 3 && isFormat.test("orc")) {
.addField("lower2uppercase", 2L)
.build() :
String.format("{\"keep\":null,\"ti2si\":1,\"si2int\":-100,\"int2bi\":-2323,\"bi2vc\":\"-12345\",%s}", hiveValueForCaseChangeField)))
.put("list_to_list", Arrays.asList(
.put("list_to_list", ImmutableList.of(
engine == Engine.TRINO ?
ImmutableList.of(rowBuilder()
.addField("ti2int", 2)
Expand All @@ -247,7 +254,7 @@ else if (getHiveVersionMajor() == 3 && isFormat.test("orc")) {
.addField("bi2vc", "-12345")
.build()) :
"[{\"ti2int\":-2,\"si2bi\":101,\"bi2vc\":\"-12345\"}]"))
.put("map_to_map", Arrays.asList(
.put("map_to_map", ImmutableList.of(
engine == Engine.TRINO ?
ImmutableMap.of(2, rowBuilder()
.addField("ti2bi", -3L)
Expand All @@ -264,70 +271,76 @@ else if (getHiveVersionMajor() == 3 && isFormat.test("orc")) {
.addField("add", null)
.build()) :
"{-2:{\"ti2bi\":null,\"int2bi\":-2323,\"float2double\":-1.5,\"add\":null}}"))
.put("tinyint_to_smallint", Arrays.asList(
.put("tinyint_to_smallint", ImmutableList.of(
-1,
1))
.put("tinyint_to_int", Arrays.asList(
.put("tinyint_to_int", ImmutableList.of(
2,
-2))
.put("tinyint_to_bigint", Arrays.asList(
-3L,
null))
.put("smallint_to_int", Arrays.asList(
.put("smallint_to_int", ImmutableList.of(
100,
-100))
.put("smallint_to_bigint", Arrays.asList(
.put("smallint_to_bigint", ImmutableList.of(
-101L,
101L))
.put("int_to_bigint", Arrays.asList(
.put("int_to_bigint", ImmutableList.of(
2323L,
-2323L))
.put("bigint_to_varchar", Arrays.asList(
.put("bigint_to_varchar", ImmutableList.of(
"12345",
"-12345"))
.put("float_to_double", Arrays.asList(
.put("float_to_double", ImmutableList.of(
0.5,
-1.5))
.put("double_to_float", Arrays.asList(0.5, -1.5))
.put("shortdecimal_to_shortdecimal", Arrays.asList(
.put("double_to_float", ImmutableList.of(0.5, -1.5))
.put("shortdecimal_to_shortdecimal", ImmutableList.of(
new BigDecimal("12345678.1200"),
new BigDecimal("-12345678.1200")))
.put("shortdecimal_to_longdecimal", Arrays.asList(
.put("shortdecimal_to_longdecimal", ImmutableList.of(
new BigDecimal("12345678.1200"),
new BigDecimal("-12345678.1200")))
.put("longdecimal_to_shortdecimal", Arrays.asList(
.put("longdecimal_to_shortdecimal", ImmutableList.of(
new BigDecimal("12345678.12"),
new BigDecimal("-12345678.12")))
.put("longdecimal_to_longdecimal", Arrays.asList(
.put("longdecimal_to_longdecimal", ImmutableList.of(
new BigDecimal("12345678.12345612345600"),
new BigDecimal("-12345678.12345612345600")))
.put("float_to_decimal", Arrays.asList(new BigDecimal(floatToDecimalVal), new BigDecimal("-" + floatToDecimalVal)))
.put("double_to_decimal", Arrays.asList(new BigDecimal("12345.12345"), new BigDecimal("-12345.12345")))
.put("decimal_to_float", Arrays.asList(
.put("float_to_decimal", ImmutableList.of(new BigDecimal(floatToDecimalVal), new BigDecimal("-" + floatToDecimalVal)))
.put("double_to_decimal", ImmutableList.of(new BigDecimal("12345.12345"), new BigDecimal("-12345.12345")))
.put("decimal_to_float", ImmutableList.of(
Float.parseFloat(decimalToFloatVal),
-Float.parseFloat(decimalToFloatVal)))
.put("decimal_to_double", Arrays.asList(
.put("decimal_to_double", ImmutableList.of(
12345.12345,
-12345.12345))
.put("short_decimal_to_varchar", Arrays.asList(
.put("short_decimal_to_varchar", ImmutableList.of(
"12345.12345",
"-12345.12345"))
.put("long_decimal_to_varchar", Arrays.asList(
.put("long_decimal_to_varchar", ImmutableList.of(
"12345678.123456123456",
"-12345678.123456123456"))
.put("short_decimal_to_bounded_varchar", Arrays.asList(
.put("short_decimal_to_bounded_varchar", ImmutableList.of(
"12345.12345",
"12345.12345"))
.put("long_decimal_to_bounded_varchar", Arrays.asList(
.put("long_decimal_to_bounded_varchar", ImmutableList.of(
"12345678.123456123456",
"-12345678.123456123456"))
.put("varchar_to_bigger_varchar", Arrays.asList(
.put("varchar_to_bigger_varchar", ImmutableList.of(
"abc",
"\uD83D\uDCB0\uD83D\uDCB0\uD83D\uDCB0"))
.put("varchar_to_smaller_varchar", Arrays.asList(
.put("varchar_to_smaller_varchar", ImmutableList.of(
"ab",
"\uD83D\uDCB0\uD83D\uDCB0"))
.put("char_to_bigger_char", ImmutableList.of(
"abc ",
"\uD83D\uDCB0\uD83D\uDCB0\uD83D\uDCB0 "))
.put("char_to_smaller_char", ImmutableList.of(
"ab",
"\uD83D\uDCB0\uD83D\uDCB0"))
.put("id", Arrays.asList(
.put("id", ImmutableList.of(
1,
1))
.buildOrThrow();
Expand Down Expand Up @@ -549,6 +562,8 @@ private void assertProperAlteredTableSchema(String tableName)
row("long_decimal_to_bounded_varchar", "varchar(30)"),
row("varchar_to_bigger_varchar", "varchar(4)"),
row("varchar_to_smaller_varchar", "varchar(2)"),
row("char_to_bigger_char", "char(4)"),
row("char_to_smaller_char", "char(2)"),
row("id", "bigint"));
}

Expand Down Expand Up @@ -593,6 +608,8 @@ private void assertColumnTypes(
.put("long_decimal_to_bounded_varchar", VARCHAR)
.put("varchar_to_bigger_varchar", VARCHAR)
.put("varchar_to_smaller_varchar", VARCHAR)
.put("char_to_bigger_char", CHAR)
Comment thread
Praveen2112 marked this conversation as resolved.
Outdated
.put("char_to_smaller_char", CHAR)
.put("id", BIGINT)
.put("nested_field", BIGINT)
.buildOrThrow();
Expand Down Expand Up @@ -631,6 +648,8 @@ private static void alterTableColumnTypes(String tableName)
onHive().executeQuery(format("ALTER TABLE %s CHANGE COLUMN long_decimal_to_bounded_varchar long_decimal_to_bounded_varchar varchar(30)", tableName));
onHive().executeQuery(format("ALTER TABLE %s CHANGE COLUMN varchar_to_bigger_varchar varchar_to_bigger_varchar varchar(4)", tableName));
onHive().executeQuery(format("ALTER TABLE %s CHANGE COLUMN varchar_to_smaller_varchar varchar_to_smaller_varchar varchar(2)", tableName));
onHive().executeQuery(format("ALTER TABLE %s CHANGE COLUMN char_to_bigger_char char_to_bigger_char char(4)", tableName));
onHive().executeQuery(format("ALTER TABLE %s CHANGE COLUMN char_to_smaller_char char_to_smaller_char char(2)", tableName));
}

protected static TableInstance<?> mutableTableInstanceOf(TableDefinition tableDefinition)
Expand Down Expand Up @@ -689,7 +708,7 @@ private static List<?> column(List<Row> rows, int sqlColumnIndex)
private static List<List<?>> extract(List<TrinoArray> arrays)
{
return arrays.stream()
.map(trinoArray -> Arrays.asList((Object[]) trinoArray.getArray()))
.map(trinoArray -> ImmutableList.copyOf((Object[]) trinoArray.getArray()))
.collect(toImmutableList());
}

Expand Down
4 changes: 3 additions & 1 deletion ...t-tests/src/main/java/io/trino/tests/product/hive/TestHiveCoercionOnPartitionedTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ private static HiveTableDefinition.HiveTableDefinitionBuilder tableDefinitionBui
" short_decimal_to_bounded_varchar DECIMAL(10,5)," +
" long_decimal_to_bounded_varchar DECIMAL(20,12)," +
" varchar_to_bigger_varchar VARCHAR(3)," +
" varchar_to_smaller_varchar VARCHAR(3)" +
" varchar_to_smaller_varchar VARCHAR(3)," +
" char_to_bigger_char CHAR(3)," +
" char_to_smaller_char CHAR(3)" +
") " +
"PARTITIONED BY (id BIGINT) " +
rowFormat.map(s -> format("ROW FORMAT %s ", s)).orElse("") +
Expand Down
2 changes: 2 additions & 0 deletions ...tests/src/main/java/io/trino/tests/product/hive/TestHiveCoercionOnUnpartitionedTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ short_decimal_to_bounded_varchar DECIMAL(10,5),
long_decimal_to_bounded_varchar DECIMAL(20,12),
varchar_to_bigger_varchar VARCHAR(3),
varchar_to_smaller_varchar VARCHAR(3),
char_to_bigger_char CHAR(3),
char_to_smaller_char CHAR(3),
id BIGINT)
STORED AS\s""" + fileFormat);
}
Expand Down