Skip to content

Commit

Permalink
[BugFix]Only when unsupported column is referred, exception will be t…
Browse files Browse the repository at this point in the history
…hrown. (StarRocks#8881)

Table binarytest has two columns, col_int and col_binary, and only col_int is unsupported.
The goal is that, only when unsupported column is referred, exception will be thrown.
  • Loading branch information
adzfolc authored Jul 28, 2022
1 parent 98ddfbc commit b3c4d10
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,9 @@ private void validate(Map<String, String> properties, org.apache.hadoop.hive.met
if (hiveColumn == null) {
throw new DdlException("column [" + column.getName() + "] not exists in hive");
}
if (!HiveMetaStoreTableUtils.validateColumnType(hiveColumn.getType(), column.getType())) {
// Only internal catalog like hive external table need to validate column type
if (HiveMetaStoreTableUtils.isInternalCatalog(resourceName) &&
!HiveMetaStoreTableUtils.validateColumnType(hiveColumn.getType(), column.getType())) {
throw new DdlException("can not convert hive column type [" + hiveColumn.getType() + "] to " +
"starrocks type [" + column.getPrimitiveType() + "]");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ public enum PrimitiveType {
JSON("JSON", 16, TPrimitiveType.JSON),

// Unsupported scalar types.
BINARY("BINARY", -1, TPrimitiveType.BINARY);
BINARY("BINARY", -1, TPrimitiveType.BINARY),

// If external table column type is unsupported, it will be converted to UNKNOWN_TYPE
UNKNOWN_TYPE("UNKNOWN_TYPE", 0, TPrimitiveType.INVALID_TYPE);

private static final int DATE_INDEX_LEN = 3;
private static final int DATETIME_INDEX_LEN = 8;
Expand Down Expand Up @@ -352,6 +355,7 @@ public int getTypeSize() {
switch (this) {
case INVALID_TYPE:
case BINARY:
case UNKNOWN_TYPE:
break;
case NULL_TYPE:
case BOOLEAN:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,10 @@ public static ScalarType createHllType() {
return type;
}

public static ScalarType createUnknownType() {
return new ScalarType(PrimitiveType.UNKNOWN_TYPE);
}

// A common type for two decimal v3 types means that if t2 = getCommonTypeForDecimalV3(t0, t1),
// two invariants following is always holds:
// 1. t2's integer part is sufficient to hold both t0 and t1's counterparts: i.e.
Expand Down Expand Up @@ -614,7 +618,9 @@ public boolean isWildcardChar() {

@Override
public boolean isSupported() {
return type != PrimitiveType.BINARY;
// BINARY and UNKNOWN_TYPE is unsupported
return (type != PrimitiveType.BINARY) &&
(type != PrimitiveType.UNKNOWN_TYPE);
}

@Override
Expand Down
58 changes: 32 additions & 26 deletions fe/fe-core/src/main/java/com/starrocks/catalog/Type.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.apache.logging.log4j.Logger;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

Expand All @@ -57,6 +58,12 @@ public abstract class Type implements Cloneable {
// performance tests.
public static int MAX_NESTING_DEPTH = 15;

// DECIMAL, NULL, and INVALID_TYPE are handled separately.
private static final List<PrimitiveType> skipCompareTypes = Arrays.asList(
PrimitiveType.INVALID_TYPE, PrimitiveType.NULL_TYPE, PrimitiveType.DECIMALV2,
PrimitiveType.DECIMAL32, PrimitiveType.DECIMAL64, PrimitiveType.DECIMAL128,
PrimitiveType.TIME, PrimitiveType.JSON);

// Static constant types for scalar types that don't require additional information.
public static final ScalarType INVALID = new ScalarType(PrimitiveType.INVALID_TYPE);
public static final ScalarType NULL = new ScalarType(PrimitiveType.NULL_TYPE);
Expand Down Expand Up @@ -103,6 +110,7 @@ public abstract class Type implements Cloneable {
public static final ScalarType BITMAP = new ScalarType(PrimitiveType.BITMAP);
public static final ScalarType PERCENTILE = new ScalarType(PrimitiveType.PERCENTILE);
public static final ScalarType JSON = new ScalarType(PrimitiveType.JSON);
public static final ScalarType UNKNOWN_TYPE = ScalarType.createUnknownType();

public static final PseudoType ANY_ELEMENT = PseudoType.ANY_ELEMENT;
public static final PseudoType ANY_ARRAY = PseudoType.ANY_ARRAY;
Expand Down Expand Up @@ -151,6 +159,7 @@ public abstract class Type implements Cloneable {
.add(DECIMAL64)
.add(DECIMAL128)
.add(JSON)
.add(UNKNOWN_TYPE)
.build();

protected static final ImmutableList<Type> SUPPORT_SCALAR_TYPE_LIST =
Expand Down Expand Up @@ -214,6 +223,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[BOOLEAN.ordinal()][BITMAP.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[BOOLEAN.ordinal()][PERCENTILE.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[BOOLEAN.ordinal()][JSON.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[BOOLEAN.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// TINYINT
compatibilityMatrix[TINYINT.ordinal()][SMALLINT.ordinal()] = PrimitiveType.SMALLINT;
Expand All @@ -236,6 +246,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[TINYINT.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[TINYINT.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[TINYINT.ordinal()][JSON.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[TINYINT.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// SMALLINT
compatibilityMatrix[SMALLINT.ordinal()][INT.ordinal()] = PrimitiveType.INT;
Expand All @@ -257,6 +268,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[SMALLINT.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[SMALLINT.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[SMALLINT.ordinal()][JSON.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[SMALLINT.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// INT
compatibilityMatrix[INT.ordinal()][BIGINT.ordinal()] = PrimitiveType.BIGINT;
Expand All @@ -276,6 +288,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[INT.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[INT.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[INT.ordinal()][JSON.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[INT.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

compatibilityMatrix[BIGINT.ordinal()][LARGEINT.ordinal()] = PrimitiveType.LARGEINT;
compatibilityMatrix[BIGINT.ordinal()][FLOAT.ordinal()] = PrimitiveType.DOUBLE;
Expand All @@ -295,6 +308,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[BIGINT.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[BIGINT.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[BIGINT.ordinal()][JSON.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[BIGINT.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// LARGEINT
compatibilityMatrix[LARGEINT.ordinal()][FLOAT.ordinal()] = PrimitiveType.DOUBLE;
Expand All @@ -312,6 +326,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[LARGEINT.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.DECIMAL64;
compatibilityMatrix[LARGEINT.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.DECIMAL128;
compatibilityMatrix[LARGEINT.ordinal()][JSON.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[LARGEINT.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// FLOAT
compatibilityMatrix[FLOAT.ordinal()][DOUBLE.ordinal()] = PrimitiveType.DOUBLE;
Expand All @@ -328,6 +343,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[FLOAT.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[FLOAT.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[FLOAT.ordinal()][JSON.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[FLOAT.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// DOUBLE
compatibilityMatrix[DOUBLE.ordinal()][DATE.ordinal()] = PrimitiveType.INVALID_TYPE;
Expand All @@ -342,6 +358,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[DOUBLE.ordinal()][DECIMAL32.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[DOUBLE.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[DOUBLE.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[DOUBLE.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// DATE
compatibilityMatrix[DATE.ordinal()][DATETIME.ordinal()] = PrimitiveType.DATETIME;
Expand All @@ -355,6 +372,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[DATE.ordinal()][DECIMAL32.ordinal()] = PrimitiveType.DECIMAL32;
compatibilityMatrix[DATE.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.DECIMAL64;
compatibilityMatrix[DATE.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.DECIMAL128;
compatibilityMatrix[DATE.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// DATETIME
compatibilityMatrix[DATETIME.ordinal()][CHAR.ordinal()] = PrimitiveType.INVALID_TYPE;
Expand All @@ -367,6 +385,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[DATETIME.ordinal()][DECIMAL32.ordinal()] = PrimitiveType.DECIMAL32;
compatibilityMatrix[DATETIME.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.DECIMAL64;
compatibilityMatrix[DATETIME.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.DECIMAL128;
compatibilityMatrix[DATETIME.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// We can convert some but not all string values to timestamps.
// CHAR
Expand All @@ -379,6 +398,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[CHAR.ordinal()][DECIMAL32.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[CHAR.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[CHAR.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[CHAR.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// VARCHAR
compatibilityMatrix[VARCHAR.ordinal()][DECIMALV2.ordinal()] = PrimitiveType.INVALID_TYPE;
Expand All @@ -389,6 +409,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[VARCHAR.ordinal()][DECIMAL32.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[VARCHAR.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[VARCHAR.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[VARCHAR.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// DECIMALV2
compatibilityMatrix[DECIMALV2.ordinal()][HLL.ordinal()] = PrimitiveType.INVALID_TYPE;
Expand All @@ -398,6 +419,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[DECIMALV2.ordinal()][DECIMAL32.ordinal()] = PrimitiveType.DECIMAL32;
compatibilityMatrix[DECIMALV2.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.DECIMAL64;
compatibilityMatrix[DECIMALV2.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.DECIMAL128;
compatibilityMatrix[DECIMALV2.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// DECIMAL32
compatibilityMatrix[DECIMAL32.ordinal()][HLL.ordinal()] = PrimitiveType.INVALID_TYPE;
Expand All @@ -407,6 +429,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[DECIMAL32.ordinal()][DECIMAL32.ordinal()] = PrimitiveType.DECIMAL32;
compatibilityMatrix[DECIMAL32.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.DECIMAL64;
compatibilityMatrix[DECIMAL32.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.DECIMAL128;
compatibilityMatrix[DECIMAL32.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// DECIMAL64
compatibilityMatrix[DECIMAL64.ordinal()][HLL.ordinal()] = PrimitiveType.INVALID_TYPE;
Expand All @@ -416,6 +439,7 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[DECIMAL64.ordinal()][DECIMAL32.ordinal()] = PrimitiveType.DECIMAL32;
compatibilityMatrix[DECIMAL64.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.DECIMAL64;
compatibilityMatrix[DECIMAL64.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.DECIMAL128;
compatibilityMatrix[DECIMAL64.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// DECIMAL128
compatibilityMatrix[DECIMAL128.ordinal()][HLL.ordinal()] = PrimitiveType.INVALID_TYPE;
Expand All @@ -425,16 +449,21 @@ public abstract class Type implements Cloneable {
compatibilityMatrix[DECIMAL128.ordinal()][DECIMAL32.ordinal()] = PrimitiveType.DECIMAL32;
compatibilityMatrix[DECIMAL128.ordinal()][DECIMAL64.ordinal()] = PrimitiveType.DECIMAL64;
compatibilityMatrix[DECIMAL128.ordinal()][DECIMAL128.ordinal()] = PrimitiveType.DECIMAL128;
compatibilityMatrix[DECIMAL128.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// HLL
compatibilityMatrix[HLL.ordinal()][TIME.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[HLL.ordinal()][BITMAP.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[HLL.ordinal()][PERCENTILE.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[HLL.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// BITMAP
compatibilityMatrix[BITMAP.ordinal()][TIME.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[BITMAP.ordinal()][PERCENTILE.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[BITMAP.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

compatibilityMatrix[PERCENTILE.ordinal()][BITMAP.ordinal()] = PrimitiveType.INVALID_TYPE;
compatibilityMatrix[PERCENTILE.ordinal()][UNKNOWN_TYPE.ordinal()] = PrimitiveType.INVALID_TYPE;

// JSON
for (PrimitiveType type : PrimitiveType.JSON_COMPATIBLE_TYPE) {
Expand All @@ -457,34 +486,11 @@ public abstract class Type implements Cloneable {

// Check all of the necessary entries that should be filled.
// ignore binary
for (int i = 0; i < PrimitiveType.values().length - 1; ++i) {
for (int j = i; j < PrimitiveType.values().length - 1; ++j) {
for (int i = 0; i < PrimitiveType.values().length - 2; ++i) {
for (int j = i; j < PrimitiveType.values().length - 2; ++j) {
PrimitiveType t1 = PrimitiveType.values()[i];
PrimitiveType t2 = PrimitiveType.values()[j];
// DECIMAL, NULL, and INVALID_TYPE are handled separately.
if (t1 == PrimitiveType.INVALID_TYPE ||
t2 == PrimitiveType.INVALID_TYPE) {
continue;
}
if (t1 == PrimitiveType.NULL_TYPE || t2 == PrimitiveType.NULL_TYPE) {
continue;
}
if (t1 == PrimitiveType.DECIMALV2 || t2 == PrimitiveType.DECIMALV2) {
continue;
}
if (t1 == PrimitiveType.DECIMAL32 || t2 == PrimitiveType.DECIMAL32) {
continue;
}
if (t1 == PrimitiveType.DECIMAL64 || t2 == PrimitiveType.DECIMAL64) {
continue;
}
if (t1 == PrimitiveType.DECIMAL128 || t2 == PrimitiveType.DECIMAL128) {
continue;
}
if (t1 == PrimitiveType.TIME || t2 == PrimitiveType.TIME) {
continue;
}
if (t1 == PrimitiveType.JSON || t2 == PrimitiveType.JSON) {
if (skipCompareTypes.contains(t1) || skipCompareTypes.contains(t2)) {
continue;
}
Preconditions.checkNotNull(compatibilityMatrix[i][j]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ public static boolean validateColumnType(String hiveType, Type type) {
return validateColumnType(hiveType.substring(hiveType.indexOf('<') + 1, hiveType.length() - 1),
((ArrayType) type).getItemType());
default:
return false;
// for BINARY and other types, we transfer it to UNKNOWN_TYPE
return primitiveType == PrimitiveType.UNKNOWN_TYPE;
}
}

Expand Down Expand Up @@ -205,7 +206,8 @@ public static Type convertColumnType(String hiveType) throws DdlException {
return type;
}
default:
throw new DdlException("hive table column type [" + typeUpperCase + "] transform failed.");
primitiveType = PrimitiveType.UNKNOWN_TYPE;
break;
}

if (primitiveType != PrimitiveType.DECIMAL32) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import com.google.common.collect.Maps;
import com.starrocks.analysis.SlotRef;
import com.starrocks.catalog.PrimitiveType;
import com.starrocks.sql.ast.CTERelation;

import java.util.List;
Expand Down Expand Up @@ -53,7 +54,12 @@ private Optional<ResolvedField> resolveField(SlotRef expression, int fieldIndexO
if (matchFields.size() > 1) {
throw new SemanticException("Column '%s' is ambiguous", expression.getColumnName());
} else if (matchFields.size() == 1) {
return Optional.of(asResolvedField(matchFields.get(0), fieldIndexOffset));
if (matchFields.get(0).getType().getPrimitiveType().equals(PrimitiveType.UNKNOWN_TYPE)) {
throw new SemanticException("External Table Column " + matchFields.get(0).getName()
+ " convert failed, and column type is known!");
} else {
return Optional.of(asResolvedField(matchFields.get(0), fieldIndexOffset));
}
} else {
if (parent != null
//Correlated subqueries currently only support accessing properties in the first level outer layer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.starrocks.analysis.SelectListItem;
import com.starrocks.analysis.SlotRef;
import com.starrocks.catalog.FunctionSet;
import com.starrocks.catalog.PrimitiveType;
import com.starrocks.catalog.Type;
import com.starrocks.common.TreeNode;
import com.starrocks.qe.ConnectContext;
Expand Down Expand Up @@ -191,6 +192,13 @@ private List<Expr> analyzeSelect(SelectList selectList, Relation fromRelation, b
List<Field> fields = (item.getTblName() == null ? scope.getRelationFields().getAllFields()
: scope.getRelationFields().resolveFieldsWithPrefix(item.getTblName()))
.stream().filter(Field::isVisible).collect(Collectors.toList());
List<String> unknownTypeFields = fields.stream()
.filter(field -> field.getType().getPrimitiveType().equals(PrimitiveType.UNKNOWN_TYPE))
.map(Field::getName).collect(Collectors.toList());
if (!unknownTypeFields.isEmpty()) {
throw new SemanticException("External Table Column " + unknownTypeFields
+ " convert failed, and column type is known!");
}
if (fields.isEmpty()) {
if (item.getTblName() != null) {
throw new SemanticException("Table %s not found", item.getTblName());
Expand Down
Loading

0 comments on commit b3c4d10

Please sign in to comment.