Skip to content

Commit

Permalink
[BugFix] Fix NULL_TYPE assertion
Browse files Browse the repository at this point in the history 
Signed-off-by: liuyehcf <1559500551@qq.com>
  • Loading branch information
@liuyehcf
liuyehcf committed Feb 8, 2023
1 parent 17286d0 commit 3ef7e36
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 25 deletions.
6 changes: 6 additions & 0 deletions fe/fe-core/src/main/java/com/starrocks/analysis/CloneExpr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ public Type getType() {
return getChild(0).getType();
}

@Override
public void setType(Type type) {
super.setType(type);
getChild(0).setType(type);
}

@Override
protected String toSqlImpl() {
return "clone(" + getChild(0).toSqlImpl() + ")";
Expand Down
4 changes: 2 additions & 2 deletions fe/fe-core/src/main/java/com/starrocks/analysis/Expr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -743,8 +743,8 @@ public interface ExprVisitor {
final void treeToThriftHelper(TExpr container, ExprVisitor visitor) {
TExprNode msg = new TExprNode();

Preconditions.checkState(!type.isNull());
Preconditions.checkState(!Objects.equal(Type.ARRAY_NULL, type));
Preconditions.checkState(!type.isNull(), "NULL_TYPE is illegal in thrift stage");
Preconditions.checkState(!Objects.equal(Type.ARRAY_NULL, type), "Array<NULL_TYPE> is illegal in thrift stage");

msg.type = type.toThrift();
msg.num_children = children.size();
Expand Down
71 changes: 48 additions & 23 deletions fe/fe-core/src/main/java/com/starrocks/sql/plan/ScalarOperatorToExpr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;

public class ScalarOperatorToExpr {
Expand Down Expand Up @@ -130,6 +131,23 @@ public static class Formatter extends ScalarOperatorVisitor<Expr, FormatterConte
this.buildExpr = buildExpr;
}

/**
* For now, backend cannot see the TYPE_NULL and other derived types, like array<null>
* So we need to do some hack when transforming ScalarOperator to Expr.
*/
private static void hackTypeNull(Expr expr) {
// For primitive types, this can be any legitimate type, for simplicity, we pick boolean.
if (expr.getType().isNull()) {
expr.setType(Type.BOOLEAN);
return;
}

// For array types, itemType can be any legitimate type, for simplicity, we pick boolean.
if (Objects.equals(Type.ARRAY_NULL, expr.getType())) {
expr.setType(Type.ARRAY_BOOLEAN);
}
}

@Override
public Expr visit(ScalarOperator scalarOperator, FormatterContext context) {
throw new UnsupportedOperationException(
Expand All @@ -141,47 +159,50 @@ public Expr visitVariableReference(ColumnRefOperator node, FormatterContext cont
Expr expr = context.colRefToExpr.get(node);
if (context.projectOperatorMap.containsKey(node) && expr == null) {
expr = buildExpr.build(context.projectOperatorMap.get(node), context);
if (expr.getType().isNull()) {
// NULL_TYPE hack, this can be any legitimate type, for simplicity, we pick boolean.
expr.setType(Type.BOOLEAN);
}
hackTypeNull(expr);
context.colRefToExpr.put(node, expr);
return expr;
}

if (expr.getType().isNull()) {
// NULL_TYPE hack, this can be any legitimate type, for simplicity, we pick boolean.
expr.setType(Type.BOOLEAN);
hackTypeNull(expr);
}
return expr;
}

@Override
public Expr visitSubfield(SubfieldOperator node, FormatterContext context) {
return new SubfieldExpr(buildExpr.build(node.getChild(0), context), node.getType(), node.getFieldNames());
SubfieldExpr expr = new SubfieldExpr(buildExpr.build(node.getChild(0), context), node.getType(),
node.getFieldNames());
hackTypeNull(expr);
return expr;
}

@Override
public Expr visitArray(ArrayOperator node, FormatterContext context) {
// NULL_TYPE hack, itemType can be any legitimate type, for simplicity, we pick boolean.
Type finalType = Type.ARRAY_NULL.equals(node.getType()) ? Type.ARRAY_BOOLEAN : node.getType();
return new ArrayExpr(finalType,
ArrayExpr expr = new ArrayExpr(node.getType(),
node.getChildren().stream().map(e -> buildExpr.build(e, context)).collect(Collectors.toList()));
hackTypeNull(expr);
return expr;
}

@Override
public Expr visitCollectionElement(CollectionElementOperator node, FormatterContext context) {
return new CollectionElementExpr(node.getType(), buildExpr.build(node.getChild(0), context),
buildExpr.build(node.getChild(1), context));
CollectionElementExpr expr =
new CollectionElementExpr(node.getType(), buildExpr.build(node.getChild(0), context),
buildExpr.build(node.getChild(1), context));
hackTypeNull(expr);
return expr;
}

@Override
public Expr visitArraySlice(ArraySliceOperator node, FormatterContext context) {
ArraySliceExpr arraySliceExpr = new ArraySliceExpr(buildExpr.build(node.getChild(0), context),
ArraySliceExpr expr = new ArraySliceExpr(buildExpr.build(node.getChild(0), context),
buildExpr.build(node.getChild(1), context),
buildExpr.build(node.getChild(2), context));
arraySliceExpr.setType(node.getType());
return arraySliceExpr;
expr.setType(node.getType());
hackTypeNull(expr);
return expr;
}

@Override
Expand All @@ -190,12 +211,8 @@ public Expr visitConstant(ConstantOperator literal, FormatterContext context) {
Type type = literal.getType();
if (literal.isNull()) {
NullLiteral nullLiteral = new NullLiteral();
if (literal.getType().isNull()) {
// NULL_TYPE hack, this can be any legitimate type, for simplicity, we pick boolean.
nullLiteral.setType(Type.BOOLEAN);
} else {
nullLiteral.setType(literal.getType());
}
nullLiteral.setType(literal.getType());
hackTypeNull(nullLiteral);
nullLiteral.setOriginType(Type.NULL);
return nullLiteral;
}
Expand Down Expand Up @@ -381,7 +398,8 @@ public Expr visitLikePredicateOperator(LikePredicateOperator predicate, Formatte
expr = new LikePredicate(LikePredicate.Operator.LIKE, child1, child2);
}

expr.setFn(Expr.getBuiltinFunction(expr.getOp().name(), new Type[] {child1.getType(), child2.getType()},
expr.setFn(Expr.getBuiltinFunction(expr.getOp().name(),
new Type[] {child1.getType(), child2.getType()},
Function.CompareMode.IS_NONSTRICT_SUPERTYPE_OF));

expr.setType(Type.BOOLEAN);
Expand Down Expand Up @@ -502,13 +520,16 @@ public Expr visitCall(CallOperator call, FormatterContext context) {
break;
}
callExpr.setType(call.getType());
hackTypeNull(callExpr);
return callExpr;
}

@Override
public Expr visitCastOperator(CastOperator operator, FormatterContext context) {
CastExpr expr = new CastExpr(operator.getType(), buildExpr.build(operator.getChild(0), context));
CastExpr expr =
new CastExpr(operator.getType(), buildExpr.build(operator.getChild(0), context));
expr.setImplicit(context.implicitCast);
hackTypeNull(expr);
return expr;
}

Expand All @@ -534,6 +555,7 @@ public Expr visitCaseWhenOperator(CaseWhenOperator operator, FormatterContext co

CaseExpr result = new CaseExpr(caseExpr, list, elseExpr);
result.setType(operator.getType());
hackTypeNull(result);
return result;
}

Expand All @@ -545,6 +567,7 @@ public Expr visitLambdaFunctionOperator(LambdaFunctionOperator operator, Formatt
for (ColumnRefOperator ref : operator.getRefColumns()) {
SlotRef slot = new SlotRef(new SlotDescriptor(
new SlotId(ref.getId()), ref.getName(), ref.getType(), ref.isNullable()));
hackTypeNull(slot);
context.colRefToExpr.put(ref, slot);
arguments.add(slot);
}
Expand All @@ -556,6 +579,7 @@ public Expr visitLambdaFunctionOperator(LambdaFunctionOperator operator, Formatt
ColumnRefOperator ref = kv.getKey();
SlotRef slot = new SlotRef(new SlotDescriptor(
new SlotId(ref.getId()), ref.getName(), ref.getType(), ref.isNullable()));
hackTypeNull(slot);
commonSubOperatorMap.put(slot, buildExpr.build(kv.getValue(), context));
context.colRefToExpr.put(ref, slot);
}
Expand Down Expand Up @@ -595,6 +619,7 @@ public Expr visitDictMappingOperator(DictMappingOperator operator, FormatterCont
}
Expr result = new DictMappingExpr(dictExpr, callExpr);
result.setType(operator.getType());
hackTypeNull(result);
return result;
}

Expand Down
16 changes: 16 additions & 0 deletions fe/fe-core/src/test/java/com/starrocks/sql/plan/ArrayTypeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,5 +269,21 @@ public void testEmptyArray() throws Exception {
"(9223372036854775807,[[9223372036854775807]],[[9223372036854775807]]);\n";
getFragmentPlan(sql);
}
{
String sql = "select array_append([],null)";
getThriftPlan(sql);
}
{
String sql = "select [][1]";
getThriftPlan(sql);
}
{
String sql = "select array_append([], [])";
getThriftPlan(sql);
}
{
String sql = "select array_append([[]], [])";
getThriftPlan(sql);
}
}
}

0 comments on commit 3ef7e36

Please sign in to comment.