This is an automated email from the ASF dual-hosted git repository.
lihaopeng pushed a commit to branch vectorized
in repository https://gitbox.apache.org/repos/asf/incubator-doris.git
The following commit(s) were added to refs/heads/vectorized by this push:
new 5229418 [Bug] Fix bug of cast expr nullable and ifnull function
(#7626)
5229418 is described below
commit 5229418da6745cb940f98f113099336cd0c79563
Author: HappenLee <[email protected]>
AuthorDate: Wed Jan 5 23:56:22 2022 -0600
[Bug] Fix bug of cast expr nullable and ifnull function (#7626)
Co-authored-by: lihaopeng <[email protected]>
---
be/src/runtime/descriptors.h | 1 -
be/src/runtime/fold_constant_executor.cpp | 8 ++--
be/src/vec/exec/join/vhash_join_node.cpp | 2 +-
be/src/vec/exec/vunion_node.cpp | 2 +
be/src/vec/functions/function.cpp | 2 +-
.../function_date_or_datetime_computation.h | 4 +-
be/src/vec/functions/function_ifnull.h | 43 ++++++++++++----------
be/src/vec/sink/vtabet_sink.cpp | 1 +
.../java/org/apache/doris/analysis/CastExpr.java | 7 ++++
.../apache/doris/analysis/FunctionCallExpr.java | 2 +-
.../apache/doris/rewrite/FoldConstantsRule.java | 9 ++++-
11 files changed, 51 insertions(+), 30 deletions(-)
diff --git a/be/src/runtime/descriptors.h b/be/src/runtime/descriptors.h
index 97f9712..ad43209 100644
--- a/be/src/runtime/descriptors.h
+++ b/be/src/runtime/descriptors.h
@@ -381,7 +381,6 @@ public:
int get_row_size() const;
int num_materialized_slots() const {
- DCHECK(_num_materialized_slots != 0);
return _num_materialized_slots;
}
diff --git a/be/src/runtime/fold_constant_executor.cpp
b/be/src/runtime/fold_constant_executor.cpp
index 9781c2f..f093c04 100644
--- a/be/src/runtime/fold_constant_executor.cpp
+++ b/be/src/runtime/fold_constant_executor.cpp
@@ -127,9 +127,11 @@ Status FoldConstantExecutor::fold_constant_vexpr(
}
vectorized::Block tmp_block;
+ tmp_block.insert({vectorized::ColumnUInt8::create(1),
+ std::make_shared<vectorized::DataTypeUInt8>(), ""});
int result_column = -1;
// calc vexpr
- ctx->execute(&tmp_block, &result_column);
+ RETURN_IF_ERROR(ctx->execute(&tmp_block, &result_column));
DCHECK(result_column != -1);
PrimitiveType root_type = ctx->root()->type().type;
// covert to thrift type
@@ -139,7 +141,7 @@ Status FoldConstantExecutor::fold_constant_vexpr(
PExprResult expr_result;
string result;
const auto& column_ptr =
tmp_block.get_by_position(result_column).column;
- if (column_ptr->is_nullable() && column_ptr->is_null_at(0)) {
+ if (column_ptr->is_null_at(0)) {
expr_result.set_success(false);
} else {
expr_result.set_success(true);
@@ -194,7 +196,7 @@ Status FoldConstantExecutor::_init(const TQueryGlobals&
query_globals) {
template <typename Context>
Status FoldConstantExecutor::_prepare_and_open(Context* ctx) {
- ctx->prepare(_runtime_state.get(), RowDescriptor(), _mem_tracker);
+ RETURN_IF_ERROR(ctx->prepare(_runtime_state.get(), RowDescriptor(),
_mem_tracker));
return ctx->open(_runtime_state.get());
}
diff --git a/be/src/vec/exec/join/vhash_join_node.cpp
b/be/src/vec/exec/join/vhash_join_node.cpp
index 7606783..4533cae 100644
--- a/be/src/vec/exec/join/vhash_join_node.cpp
+++ b/be/src/vec/exec/join/vhash_join_node.cpp
@@ -133,7 +133,7 @@ struct ProcessRuntimeFilterBuild {
RETURN_IF_ERROR(runtime_filter_slots->init(state,
hash_table_ctx.hash_table.get_size()));
- if (!runtime_filter_slots->empty()) {
+ if (!runtime_filter_slots->empty() &&
!_join_node->_inserted_rows.empty()) {
{
SCOPED_TIMER(_join_node->_push_compute_timer);
runtime_filter_slots->insert(_join_node->_inserted_rows);
diff --git a/be/src/vec/exec/vunion_node.cpp b/be/src/vec/exec/vunion_node.cpp
index 122eafa..1fa4da4 100644
--- a/be/src/vec/exec/vunion_node.cpp
+++ b/be/src/vec/exec/vunion_node.cpp
@@ -181,6 +181,8 @@ Status VUnionNode::get_next_const(RuntimeState* state,
Block* block) {
MutableBlock(Block(VectorizedUtils::create_columns_with_type_and_name(row_desc())));
for (; _const_expr_list_idx < _const_expr_lists.size();
++_const_expr_list_idx) {
Block tmp_block;
+ tmp_block.insert({vectorized::ColumnUInt8::create(1),
+ std::make_shared<vectorized::DataTypeUInt8>(), ""});
int const_expr_lists_size =
_const_expr_lists[_const_expr_list_idx].size();
std::vector<int> result_list(const_expr_lists_size);
for (size_t i = 0; i < const_expr_lists_size; ++i) {
diff --git a/be/src/vec/functions/function.cpp
b/be/src/vec/functions/function.cpp
index d3a910e..9ed3edb 100644
--- a/be/src/vec/functions/function.cpp
+++ b/be/src/vec/functions/function.cpp
@@ -66,7 +66,7 @@ ColumnPtr wrap_in_nullable(const ColumnPtr& src, const Block&
block, const Colum
null_map_column->clone_resized(null_map_column->size());
} else {
MutableColumnPtr mutable_result_null_map_column =
- (*std::move(result_null_map_column)).mutate();
+ (*std::move(result_null_map_column)).assume_mutable();
NullMap& result_null_map =
assert_cast<ColumnUInt8&>(*mutable_result_null_map_column).get_data();
diff --git a/be/src/vec/functions/function_date_or_datetime_computation.h
b/be/src/vec/functions/function_date_or_datetime_computation.h
index 33c7588..607039c 100644
--- a/be/src/vec/functions/function_date_or_datetime_computation.h
+++ b/be/src/vec/functions/function_date_or_datetime_computation.h
@@ -434,11 +434,11 @@ struct CurrentDateImpl {
template<typename FunctionName>
struct CurrentTimeImpl {
- using ReturnType = DataTypeInt64;
+ using ReturnType = DataTypeFloat64;
static constexpr auto name = FunctionName::name;
static Status execute(FunctionContext* context, Block& block, size_t
result,
size_t input_rows_count) {
- auto col_to = ColumnVector<Int64>::create();
+ auto col_to = ColumnVector<Float64>::create();
VecDateTimeValue dtv;
if (dtv.from_unixtime(context->impl()->state()->timestamp_ms() / 1000,
context->impl()->state()->timezone_obj())) {
diff --git a/be/src/vec/functions/function_ifnull.h
b/be/src/vec/functions/function_ifnull.h
index b18de80..26fe18c 100644
--- a/be/src/vec/functions/function_ifnull.h
+++ b/be/src/vec/functions/function_ifnull.h
@@ -42,6 +42,9 @@ public:
bool use_default_implementation_for_constants() const override { return
false; }
DataTypePtr get_return_type_impl(const DataTypes& arguments) const
override {
+ if (!arguments[0]->is_nullable() && arguments[1]->is_nullable()) {
+ return reinterpret_cast<const
DataTypeNullable*>(arguments[1].get())->get_nested_type();
+ }
return arguments[1];
}
@@ -55,42 +58,42 @@ public:
block.get_by_position(result).column =
block.get_by_position(arguments[1]).column;
return Status::OK();
}
- const ColumnWithTypeAndName new_result_column {
- block.get_by_position(result),
+
+ ColumnWithTypeAndName null_column_arg0 {
+ nullptr, std::make_shared<DataTypeUInt8>(),""
};
- const ColumnWithTypeAndName new_column {
- col_left,
+ ColumnWithTypeAndName nested_column_arg0 {
+ nullptr, col_left.type, ""
};
+
/// implement isnull(col_left) logic
if (auto* nullable =
check_and_get_column<ColumnNullable>(*col_left.column)) {
- block.get_by_position(arguments[0]).column =
nullable->get_null_map_column_ptr();
+ null_column_arg0.column = nullable->get_null_map_column_ptr();
+ nested_column_arg0.column = nullable->get_nested_column_ptr();
+ nested_column_arg0.type = reinterpret_cast<const
DataTypeNullable*>(
+ nested_column_arg0.type.get())->get_nested_type();
} else {
block.get_by_position(result).column = col_left.column;
return Status::OK();
}
const ColumnsWithTypeAndName if_columns
{
- block.get_by_position(arguments[0]),
+ null_column_arg0,
block.get_by_position(arguments[1]),
- new_column,
+ nested_column_arg0
};
Block temporary_block(
- {block.get_by_position(arguments[0]),
- block.get_by_position(arguments[1]),
- new_column,
- new_result_column
+ {
+ null_column_arg0,
+ block.get_by_position(arguments[1]),
+ nested_column_arg0,
+ block.get_by_position(result),
});
- auto func_if = SimpleFunctionFactory::instance().get_function("if",
if_columns, make_nullable(new_result_column.type));
+
+ auto func_if = SimpleFunctionFactory::instance().get_function("if",
if_columns, block.get_by_position(result).type);
func_if->execute(context, temporary_block, {0, 1, 2}, 3,
input_rows_count);
- /// need to handle nullable type and not nullable type differently,
- /// because `IF` function always return nullable type, but result type
is not always
- if (block.get_by_position(result).type->is_nullable()) {
- block.get_by_position(result).column =
temporary_block.get_by_position(3).column;
- } else {
- auto cols =
check_and_get_column<ColumnNullable>(temporary_block.get_by_position(3).column.get());
- block.replace_by_position(result,
std::move(cols->get_nested_column_ptr()));
- }
+ block.get_by_position(result).column =
temporary_block.get_by_position(3).column;
return Status::OK();
}
};
diff --git a/be/src/vec/sink/vtabet_sink.cpp b/be/src/vec/sink/vtabet_sink.cpp
index 685f017..3b09cae 100644
--- a/be/src/vec/sink/vtabet_sink.cpp
+++ b/be/src/vec/sink/vtabet_sink.cpp
@@ -129,6 +129,7 @@ int VOlapTableSink::_validate_data(doris::RuntimeState*
state, doris::vectorized
for (int i = 0; i < _output_tuple_desc->slots().size(); ++i) {
SlotDescriptor* desc = _output_tuple_desc->slots()[i];
+ block->get_by_position(i).column =
block->get_by_position(i).column->convert_to_full_column_if_const();
const auto& column = block->get_by_position(i).column;
if (desc->is_nullable() && desc->type() == TYPE_OBJECT) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
index f071e0f..2a629f9 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
@@ -451,4 +451,11 @@ public class CastExpr extends Expr {
}
return -1;
}
+
+ @Override
+ public boolean isNullable() {
+ return children.get(0).isNullable() ||
+ (children.get(0).getType().isStringType() &&
!getType().isStringType()) ||
+ (!children.get(0).getType().isDateType() &&
getType().isDateType());
+ }
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
index f6a350a..6fa7c69 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
@@ -1053,7 +1053,7 @@ public class FunctionCallExpr extends Expr {
// TODO: we can't correctly determine const-ness before analyzing
'fn_'. We should
// rework logic so that we do not call this function on unanalyzed
exprs.
// Aggregate functions are never constant.
- if (fn instanceof AggregateFunction) return false;
+ if (fn instanceof AggregateFunction || fn == null) return false;
final String fnName = this.fnName.getFunction();
// Non-deterministic functions are never constant.
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/rewrite/FoldConstantsRule.java
b/fe/fe-core/src/main/java/org/apache/doris/rewrite/FoldConstantsRule.java
index 8eaac29..628740a 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/rewrite/FoldConstantsRule.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/rewrite/FoldConstantsRule.java
@@ -27,6 +27,7 @@ import org.apache.doris.analysis.LiteralExpr;
import org.apache.doris.analysis.NullLiteral;
import org.apache.doris.analysis.SysVariableDesc;
import org.apache.doris.catalog.Catalog;
+import org.apache.doris.catalog.Function;
import org.apache.doris.catalog.PrimitiveType;
import org.apache.doris.catalog.Type;
import org.apache.doris.common.AnalysisException;
@@ -194,6 +195,12 @@ public class FoldConstantsRule implements ExprRewriteRule {
Analyzer analyzer, Map<String, Expr> sysVarMap,
Map<String, Expr> infoFnMap)
throws AnalysisException {
if (expr.isConstant()) {
+ if (VectorizedUtil.isVectorized()) {
+ Function fn = expr.getFn();
+ if (fn != null &&
(fn.functionName().equalsIgnoreCase("curtime") ||
+ fn.functionName().equalsIgnoreCase("current_time")))
+ return;
+ }
// Do not constant fold cast(null as dataType) because we cannot
preserve the
// cast-to-types and that can lead to query failures, e.g., CTAS
if (expr instanceof CastExpr) {
@@ -216,7 +223,7 @@ public class FoldConstantsRule implements ExprRewriteRule {
getInfoFnExpr(expr, infoFnMap);
return;
}
- constExprMap.put(expr.getId().toString(),expr.treeToThrift());
+ constExprMap.put(expr.getId().toString(), expr.treeToThrift());
oriConstMap.put(expr.getId().toString(), expr);
} else {
recursiveGetChildrenConstExpr(expr, constExprMap, oriConstMap,
analyzer, sysVarMap, infoFnMap);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
