Skip to content

Commit

Permalink
Disallow creating Hive tables with unsupported partition types
Browse files Browse the repository at this point in the history
  • Loading branch information
@bowdencm @electrum
bowdencm authored and electrum committed Jun 5, 2018
1 parent 8c7b3a6 commit a8023cc
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 3 deletions.
23 changes: 23 additions & 0 deletions presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@
import static com.facebook.presto.hive.HiveUtil.hiveColumnHandles;
import static com.facebook.presto.hive.HiveUtil.schemaTableName;
import static com.facebook.presto.hive.HiveUtil.toPartitionValues;
import static com.facebook.presto.hive.HiveUtil.verifyPartitionTypeSupported;
import static com.facebook.presto.hive.HiveWriteUtils.checkTableIsWritable;
import static com.facebook.presto.hive.HiveWriteUtils.initializeSerializer;
import static com.facebook.presto.hive.HiveWriteUtils.isWritableType;
Expand Down Expand Up @@ -609,6 +610,13 @@ public void createTable(ConnectorSession session, ConnectorTableMetadata tableMe

hiveStorageFormat.validateColumns(columnHandles);

Map<String, HiveColumnHandle> columnHandlesByName = Maps.uniqueIndex(columnHandles, HiveColumnHandle::getName);
List<Column> partitionColumns = partitionedBy.stream()
.map(columnHandlesByName::get)
.map(column -> new Column(column.getName(), column.getHiveType(), column.getComment()))
.collect(toList());
checkPartitionTypesSupported(partitionColumns);

Path targetPath;
boolean external;
String externalLocation = getExternalLocation(tableMetadata.getProperties());
Expand Down Expand Up @@ -681,6 +689,14 @@ private Path getExternalPath(HdfsContext context, String location)
}
}

private void checkPartitionTypesSupported(List<Column> partitionColumns)
{
for (Column partitionColumn : partitionColumns) {
Type partitionType = typeManager.getType(partitionColumn.getType().getTypeSignature());
verifyPartitionTypeSupported(partitionColumn.getName(), partitionType);
}
}

private static Table buildTableObject(
String queryId,
String schemaName,
Expand Down Expand Up @@ -827,6 +843,13 @@ public HiveOutputTableHandle beginCreateTable(ConnectorSession session, Connecto
HiveStorageFormat actualStorageFormat = partitionedBy.isEmpty() ? tableStorageFormat : partitionStorageFormat;
actualStorageFormat.validateColumns(columnHandles);

Map<String, HiveColumnHandle> columnHandlesByName = Maps.uniqueIndex(columnHandles, HiveColumnHandle::getName);
List<Column> partitionColumns = partitionedBy.stream()
.map(columnHandlesByName::get)
.map(column -> new Column(column.getName(), column.getHiveType(), column.getComment()))
.collect(toList());
checkPartitionTypesSupported(partitionColumns);

LocationHandle locationHandle = locationService.forNewTable(metastore, session, schemaName, tableName);
HiveOutputTableHandle result = new HiveOutputTableHandle(
schemaName,
Expand Down
31 changes: 29 additions & 2 deletions presto-hive/src/main/java/com/facebook/presto/hive/HiveUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.facebook.presto.spi.type.VarcharType;
import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.base.VerifyException;
import com.google.common.collect.ImmutableList;
import io.airlift.compress.lzo.LzoCodec;
import io.airlift.compress.lzo.LzopCodec;
Expand Down Expand Up @@ -110,6 +111,7 @@
import static com.facebook.presto.spi.type.SmallintType.SMALLINT;
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.Varchars.isVarcharType;
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList;
Expand Down Expand Up @@ -395,8 +397,33 @@ public static boolean isHiveNull(byte[] bytes)
return bytes.length == 2 && bytes[0] == '\\' && bytes[1] == 'N';
}

public static void verifyPartitionTypeSupported(String partitionName, Type type)
{
if (!isValidPartitionType(type)) {
throw new PrestoException(NOT_SUPPORTED, format("Unsupported type [%s] for partition: %s", type, partitionName));
}
}

private static boolean isValidPartitionType(Type type)
{
return type instanceof DecimalType ||
BOOLEAN.equals(type) ||
TINYINT.equals(type) ||
SMALLINT.equals(type) ||
INTEGER.equals(type) ||
BIGINT.equals(type) ||
REAL.equals(type) ||
DOUBLE.equals(type) ||
DATE.equals(type) ||
TIMESTAMP.equals(type) ||
isVarcharType(type) ||
isCharType(type);
}

public static NullableValue parsePartitionValue(String partitionName, String value, Type type, DateTimeZone timeZone)
{
verifyPartitionTypeSupported(partitionName, type);

boolean isNull = HIVE_DEFAULT_DYNAMIC_PARTITION.equals(value);

if (type instanceof DecimalType) {
Expand Down Expand Up @@ -502,7 +529,7 @@ public static NullableValue parsePartitionValue(String partitionName, String val
return NullableValue.of(DOUBLE, doublePartitionKey(value, partitionName));
}

if (type instanceof VarcharType) {
if (isVarcharType(type)) {
if (isNull) {
return NullableValue.asNull(type);
}
Expand All @@ -516,7 +543,7 @@ public static NullableValue parsePartitionValue(String partitionName, String val
return NullableValue.of(type, charPartitionKey(value, partitionName, type));
}

throw new PrestoException(NOT_SUPPORTED, format("Unsupported Type [%s] for partition: %s", type, partitionName));
throw new VerifyException(format("Unhandled type [%s] for partition: %s", type, partitionName));
}

public static boolean isPrestoView(Table table)
Expand Down
21 changes: 20 additions & 1 deletion presto-hive/src/test/java/com/facebook/presto/hive/TestHiveIntegrationSmokeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,25 @@ public void testCreateTableNonExistentPartitionColumns()
"WITH (partitioned_by = ARRAY['dragonfruit'])");
}

@Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "Unsupported type .* for partition: .*")
public void testCreateTableUnsupportedPartitionType()
{
assertUpdate("" +
"CREATE TABLE test_create_table_unsupported_partition_type " +
"(foo bigint, bar ARRAY(varchar)) " +
"WITH (partitioned_by = ARRAY['bar'])");
}

@Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "Unsupported type .* for partition: a")
public void testCreateTableUnsupportedPartitionTypeAs()
{
assertUpdate("" +
"CREATE TABLE test_create_table_unsupported_partition_type_as " +
"WITH (partitioned_by = ARRAY['a']) " +
"AS " +
"SELECT 123 x, ARRAY ['foo'] a");
}

@Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "Unsupported Hive type: varchar\\(65536\\)\\. Supported VARCHAR types: VARCHAR\\(<=65535\\), VARCHAR\\.")
public void testCreateTableNonSupportedVarcharColumn()
{
Expand Down Expand Up @@ -1774,7 +1793,7 @@ public void testShowCreateTable()
" format = 'ORC',\n" +
" orc_bloom_filter_columns = ARRAY['c1','c2'],\n" +
" orc_bloom_filter_fpp = 7E-1,\n" +
" partitioned_by = ARRAY['c4','c5'],\n" +
" partitioned_by = ARRAY['c5'],\n" +
" sorted_by = ARRAY['c1','c 2 DESC']\n" +
")",
getSession().getCatalog().get(),
Expand Down

0 comments on commit a8023cc

Please sign in to comment.