Skip to content
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.

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

[Enhancement] Support Trino try expression #55144

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions be/src/exprs/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -102,6 +102,7 @@ set(EXPR_FILES
dictionary_get_expr.cpp
ngram.cpp
match_expr.cpp
try_expr.cpp
)

if(STARROCKS_JIT_ENABLE)
4 changes: 4 additions & 0 deletions be/src/exprs/expr.cpp
Original file line number Diff line number Diff line change
@@ -72,6 +72,7 @@
#include "exprs/match_expr.h"
#include "exprs/placeholder_ref.h"
#include "exprs/subfield_expr.h"
#include "exprs/try_expr.h"
#include "gutil/casts.h"
#include "gutil/strings/substitute.h"
#include "runtime/runtime_state.h"
@@ -467,6 +468,9 @@ Status Expr::create_vectorized_expr(starrocks::ObjectPool* pool, const starrocks
case TExprNodeType::MATCH_EXPR:
*expr = pool->add(new MatchExpr(texpr_node));
break;
case TExprNodeType::TRY_EXPR:
*expr = pool->add(TryExprFactory::from_thrift(texpr_node));
break;
case TExprNodeType::ARRAY_SLICE_EXPR:
case TExprNodeType::AGG_EXPR:
case TExprNodeType::TABLE_FUNCTION_EXPR:
49 changes: 49 additions & 0 deletions be/src/exprs/try_expr.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2021-present StarRocks, Inc. All rights reserved.
//
// 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
//
// https://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.

#include "exprs/try_expr.h"

#include "column/chunk.h"
#include "column/column_helper.h"
#include "column/const_column.h"
#include "column/fixed_length_column.h"
#include "common/object_pool.h"

namespace starrocks {

class TryExpr final : public Expr {
public:
explicit TryExpr(const TExprNode& node) : Expr(node) {}

TryExpr(const TryExpr&) = default;
TryExpr(TryExpr&&) = default;

StatusOr<ColumnPtr> evaluate_checked(ExprContext* context, Chunk* chunk) override {
auto&& status = _children[0]->evaluate_checked(context, chunk);
if (LIKELY(status.ok())) {
return status;
}
return ColumnHelper::create_const_null_column(chunk->num_rows());
}

Expr* clone(ObjectPool* pool) const override { return pool->add(new TryExpr(*this)); }
};

Expr* TryExprFactory::from_thrift(const TExprNode& node) {
DCHECK_EQ(TExprNodeType::TRY_EXPR, node.node_type);
DCHECK_EQ(node.type.types.size(), 1);
return new TryExpr(node);
}

} // namespace starrocks
26 changes: 26 additions & 0 deletions be/src/exprs/try_expr.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2021-present StarRocks, Inc. All rights reserved.
//
// 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
//
// https://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.

#pragma once

#include "exprs/expr.h"

namespace starrocks {

class TryExprFactory {
public:
static Expr* from_thrift(const TExprNode& node);
};

} // namespace starrocks
92 changes: 92 additions & 0 deletions fe/fe-core/src/main/java/com/starrocks/analysis/TryExpr.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Copyright 2021-present StarRocks, Inc. All rights reserved.
//
// 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
//
// https://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.starrocks.analysis;

import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.starrocks.catalog.Type;
import com.starrocks.common.AnalysisException;
import com.starrocks.sql.ast.AstVisitor;
import com.starrocks.sql.parser.NodePosition;
import com.starrocks.thrift.TExprNode;
import com.starrocks.thrift.TExprNodeType;
import org.apache.log4j.LogManager;
import org.apache.log4j.Logger;

import java.util.List;
import java.util.Objects;

public class TryExpr extends Expr {
private static final Logger LOG = LogManager.getLogger(TryExpr.class);

public TryExpr(Expr child) {
this(child, NodePosition.ZERO);
}

public TryExpr(Expr child, NodePosition pos) {
super(pos);
this.addChild(child);
setType(child.getType());
}

@Override
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 "TRY(" + getChild(0).toSqlImpl() + ")";
}

@Override
protected String explainImpl() {
return "try(" + getChild(0).explain() + ")";
}

@Override
protected void toThrift(TExprNode msg) {
msg.node_type = TExprNodeType.TRY_EXPR;
msg.setOpcode(opcode);
msg.setOutput_column(outputColumn);
if (getChild(0).getType().isComplexType()) {
msg.setChild_type_desc(getChild(0).getType().toThrift());
} else {
msg.setChild_type(getChild(0).getType().getPrimitiveType().toThrift());
}
}

@Override
public boolean isNullable() {
return true;
}

@Override
public Expr clone() {
return new TryExpr(getChild(0).clone(), pos);
}

@Override
public <R, C> R accept(AstVisitor<R, C> visitor, C context) {
return visitor.visitTryExpr(this, context);
}
}
Copy link

Choose a reason for hiding this comment

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

The most risky bug in this code is:
The clone method calls the TryExpr constructor with this, but there's no constructor for TryExpr that accepts a TryExpr as an argument, which will result in a compilation error.

You can modify the code like this:

@Override
public Expr clone() {
    return new TryExpr(getChild(0).clone(), pos);
}

This change correctly clones the child expression and also provides the current NodePosition.

Original file line number Diff line number Diff line change
@@ -53,6 +53,7 @@
import com.starrocks.analysis.Subquery;
import com.starrocks.analysis.TableName;
import com.starrocks.analysis.TimestampArithmeticExpr;
import com.starrocks.analysis.TryExpr;
import com.starrocks.analysis.TypeDef;
import com.starrocks.analysis.VarBinaryLiteral;
import com.starrocks.analysis.VariableExpr;
@@ -868,7 +869,8 @@ protected ParseNode visitDereferenceExpression(DereferenceExpression node, Parse

@Override
protected ParseNode visitTryExpression(TryExpression node, ParseTreeContext context) {
return visit(node.getInnerExpression(), context);
List<Expr> children = visit(node, context, Expr.class);
return new TryExpr(children.get(0));
}

@Override
Original file line number Diff line number Diff line change
@@ -43,6 +43,7 @@
import com.starrocks.analysis.SlotRef;
import com.starrocks.analysis.Subquery;
import com.starrocks.analysis.TimestampArithmeticExpr;
import com.starrocks.analysis.TryExpr;
import com.starrocks.analysis.VariableExpr;
import com.starrocks.catalog.AggregateFunction;
import com.starrocks.qe.ConnectContext;
@@ -355,6 +356,11 @@ public Boolean visitCloneExpr(CloneExpr node, Void context) {
return visit(node.getChild(0));
}

@Override
public Boolean visitTryExpr(TryExpr node, Void context) {
return visit(node.getChild(0));
}

@Override
public Boolean visitDictQueryExpr(DictQueryExpr node, Void context) {
return node.getChildren().stream().allMatch(this::visit);
Original file line number Diff line number Diff line change
@@ -60,6 +60,7 @@
import com.starrocks.analysis.Subquery;
import com.starrocks.analysis.TableName;
import com.starrocks.analysis.TimestampArithmeticExpr;
import com.starrocks.analysis.TryExpr;
import com.starrocks.analysis.UserVariableExpr;
import com.starrocks.analysis.VariableExpr;
import com.starrocks.authorization.AuthorizationMgr;
@@ -1522,6 +1523,11 @@ public Void visitCloneExpr(CloneExpr node, Scope context) {
return null;
}

@Override
public Void visitTryExpr(TryExpr node, Scope context) {
return null;
}

@Override
public Void visitDictQueryExpr(DictQueryExpr node, Scope context) {
if (RunMode.isSharedDataMode()) {
Original file line number Diff line number Diff line change
@@ -49,6 +49,7 @@
import com.starrocks.analysis.SubfieldExpr;
import com.starrocks.analysis.Subquery;
import com.starrocks.analysis.TimestampArithmeticExpr;
import com.starrocks.analysis.TryExpr;
import com.starrocks.analysis.UserVariableExpr;
import com.starrocks.analysis.UserVariableHint;
import com.starrocks.analysis.VariableExpr;
@@ -1467,6 +1468,10 @@ default R visitTimestampArithmeticExpr(TimestampArithmeticExpr node, C context)
return visitExpression(node, context);
}

default R visitTryExpr(TryExpr node, C context) {
return visitExpression(node, context);
}

default R visitCloneExpr(CloneExpr node, C context) {
return visitExpression(node, context);
}
Original file line number Diff line number Diff line change
@@ -134,6 +134,7 @@ public enum OperatorType {
MULTI_IN,
DICTIONARY_GET,
MATCH_EXPR,
TRY,

/**
* PATTERN
Original file line number Diff line number Diff line change
@@ -115,6 +115,10 @@ public R visitCloneOperator(CloneOperator operator, C context) {
return visit(operator, context);
}

public R visitTryOperator(TryOperator operator, C context) {
return visit(operator, context);
}

public R visitSubqueryOperator(SubqueryOperator operator, C context) {
return visit(operator, context);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// Copyright 2021-present StarRocks, Inc. All rights reserved.
//
// 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
//
// https://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.starrocks.sql.optimizer.operator.scalar;

import com.google.common.collect.Lists;
import com.starrocks.sql.optimizer.base.ColumnRefSet;
import com.starrocks.sql.optimizer.operator.OperatorType;

import java.util.List;
import java.util.Objects;

public class TryOperator extends ScalarOperator {
private List<ScalarOperator> arguments;

public TryOperator(ScalarOperator argument) {
super(OperatorType.TRY, argument.getType());
arguments = Lists.newArrayList(argument);
setType(argument.getType());
}

@Override
public boolean isNullable() {
return true;
}

@Override
public List<ScalarOperator> getChildren() {
return arguments;
}

@Override
public ScalarOperator getChild(int index) {
return arguments.get(0);
}

@Override
public void setChild(int index, ScalarOperator child) {
arguments.set(0, child);
}

@Override
public String toString() {
return "Try(" + arguments.get(0) + ")";
}

@Override
public int hashCode() {
return Objects.hash(arguments);
}

@Override
public boolean equals(Object other) {
if (this == other) {
return true;
}
if (other == null || getClass() != other.getClass()) {
return false;
}
TryOperator that = (TryOperator) other;
return Objects.equals(arguments, that.arguments);
}

@Override
public <R, C> R accept(ScalarOperatorVisitor<R, C> visitor, C context) {
return visitor.visitTryOperator(this, context);
}

@Override
public ColumnRefSet getUsedColumns() {
return arguments.get(0).getUsedColumns();
}

@Override
public ScalarOperator clone() {
TryOperator operator = (TryOperator) super.clone();
// Deep copy here
List<ScalarOperator> newArguments = Lists.newArrayList();
this.arguments.forEach(p -> newArguments.add(p.clone()));
operator.arguments = newArguments;
return operator;
}
}

Copy link

Choose a reason for hiding this comment

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

The most risky bug in this code is:
The equals method only checks for reference equality, which may not be the intended behavior and could lead to incorrect logical comparison results.

You can modify the code like this:

@Override
public boolean equals(Object other) {
    if (this == other) return true;
    if (other == null || getClass() != other.getClass()) return false;
    TryOperator that = (TryOperator) other;
    return Objects.equals(arguments, that.arguments);
}

Loading