This is an automated email from the ASF dual-hosted git repository.

yiguolei pushed a commit to branch dev-1.1.2
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/dev-1.1.2 by this push:
     new d066af5e2f [enhancement](VSlotRef) enhance column_id check in execute 
function during runtime (#11862)
d066af5e2f is described below

commit d066af5e2fc4f0943f574c77fa1157c329dfd42e
Author: AlexYue <yj976240...@qq.com>
AuthorDate: Thu Aug 18 09:12:26 2022 +0800

    [enhancement](VSlotRef) enhance column_id check in execute function during 
runtime (#11862)
    
    The column id check in VSlotRef::execute function before is too strict for 
fuzzy test to continuously produce random query. Temporarily loosen the check 
logic.
    Moreover, there exists some careless call to VExpr::get_const_col, it might 
return a nullptr but not every function call checks if it's valid. It's an 
underlying problem.
---
 .../vec/exprs/table_function/vexplode_bitmap.cpp   |  4 +++-
 .../exprs/table_function/vexplode_json_array.cpp   |  3 ++-
 .../vec/exprs/table_function/vexplode_numbers.cpp  |  4 +++-
 be/src/vec/exprs/table_function/vexplode_split.cpp |  6 +++--
 be/src/vec/exprs/vcase_expr.cpp                    |  3 ++-
 be/src/vec/exprs/vcast_expr.cpp                    |  2 +-
 be/src/vec/exprs/vectorized_fn_call.cpp            |  3 ++-
 be/src/vec/exprs/vin_predicate.cpp                 |  3 ++-
 be/src/vec/exprs/vslot_ref.cpp                     | 26 ++++++++++++----------
 9 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/be/src/vec/exprs/table_function/vexplode_bitmap.cpp 
b/be/src/vec/exprs/table_function/vexplode_bitmap.cpp
index 0dec849980..f015ac7330 100644
--- a/be/src/vec/exprs/table_function/vexplode_bitmap.cpp
+++ b/be/src/vec/exprs/table_function/vexplode_bitmap.cpp
@@ -17,6 +17,7 @@
 
 #include "vec/exprs/table_function/vexplode_bitmap.h"
 
+#include "common/status.h"
 #include "util/bitmap_value.h"
 #include "vec/exprs/vexpr.h"
 
@@ -32,7 +33,8 @@ Status 
VExplodeBitmapTableFunction::process_init(vectorized::Block* block) {
             << _vexpr_context->root()->children().size();
 
     int value_column_idx = -1;
-    _vexpr_context->root()->children()[0]->execute(_vexpr_context, block, 
&value_column_idx);
+    
RETURN_IF_ERROR(_vexpr_context->root()->children()[0]->execute(_vexpr_context, 
block,
+                                                                   
&value_column_idx));
     _value_column = block->get_by_position(value_column_idx).column;
 
     return Status::OK();
diff --git a/be/src/vec/exprs/table_function/vexplode_json_array.cpp 
b/be/src/vec/exprs/table_function/vexplode_json_array.cpp
index aedcc53643..8ee1815117 100644
--- a/be/src/vec/exprs/table_function/vexplode_json_array.cpp
+++ b/be/src/vec/exprs/table_function/vexplode_json_array.cpp
@@ -32,7 +32,8 @@ Status 
VExplodeJsonArrayTableFunction::process_init(vectorized::Block* block) {
             << _vexpr_context->root()->children().size();
 
     int text_column_idx = -1;
-    _vexpr_context->root()->children()[0]->execute(_vexpr_context, block, 
&text_column_idx);
+    
RETURN_IF_ERROR(_vexpr_context->root()->children()[0]->execute(_vexpr_context, 
block,
+                                                                   
&text_column_idx));
     _text_column = block->get_by_position(text_column_idx).column;
 
     return Status::OK();
diff --git a/be/src/vec/exprs/table_function/vexplode_numbers.cpp 
b/be/src/vec/exprs/table_function/vexplode_numbers.cpp
index 540c7ac5df..a76f759630 100644
--- a/be/src/vec/exprs/table_function/vexplode_numbers.cpp
+++ b/be/src/vec/exprs/table_function/vexplode_numbers.cpp
@@ -17,6 +17,7 @@
 
 #include "vec/exprs/table_function/vexplode_numbers.h"
 
+#include "common/status.h"
 #include "vec/exprs/vexpr.h"
 
 namespace doris::vectorized {
@@ -31,7 +32,8 @@ Status 
VExplodeNumbersTableFunction::process_init(vectorized::Block* block) {
             << _vexpr_context->root()->children().size();
 
     int value_column_idx = -1;
-    _vexpr_context->root()->children()[0]->execute(_vexpr_context, block, 
&value_column_idx);
+    
RETURN_IF_ERROR(_vexpr_context->root()->children()[0]->execute(_vexpr_context, 
block,
+                                                                   
&value_column_idx));
     _value_column = block->get_by_position(value_column_idx).column;
 
     return Status::OK();
diff --git a/be/src/vec/exprs/table_function/vexplode_split.cpp 
b/be/src/vec/exprs/table_function/vexplode_split.cpp
index 1e124f5781..fbd37d4d3f 100644
--- a/be/src/vec/exprs/table_function/vexplode_split.cpp
+++ b/be/src/vec/exprs/table_function/vexplode_split.cpp
@@ -40,8 +40,10 @@ Status 
VExplodeSplitTableFunction::process_init(vectorized::Block* block) {
     int text_column_idx = -1;
     int delimiter_column_idx = -1;
 
-    _vexpr_context->root()->children()[0]->execute(_vexpr_context, block, 
&text_column_idx);
-    _vexpr_context->root()->children()[1]->execute(_vexpr_context, block, 
&delimiter_column_idx);
+    
RETURN_IF_ERROR(_vexpr_context->root()->children()[0]->execute(_vexpr_context, 
block,
+                                                                   
&text_column_idx));
+    
RETURN_IF_ERROR(_vexpr_context->root()->children()[1]->execute(_vexpr_context, 
block,
+                                                                   
&delimiter_column_idx));
 
     _text_column = block->get_by_position(text_column_idx).column;
     _delimiter_column = block->get_by_position(delimiter_column_idx).column;
diff --git a/be/src/vec/exprs/vcase_expr.cpp b/be/src/vec/exprs/vcase_expr.cpp
index 2683e33bba..13c98b152c 100644
--- a/be/src/vec/exprs/vcase_expr.cpp
+++ b/be/src/vec/exprs/vcase_expr.cpp
@@ -17,6 +17,7 @@
 
 #include "vec/exprs/vcase_expr.h"
 
+#include "common/status.h"
 #include "vec/columns/column_nullable.h"
 
 namespace doris::vectorized {
@@ -94,7 +95,7 @@ Status VCaseExpr::execute(VExprContext* context, Block* 
block, int* result_colum
 
     for (int i = 0; i < _children.size(); i++) {
         int column_id = -1;
-        _children[i]->execute(context, block, &column_id);
+        RETURN_IF_ERROR(_children[i]->execute(context, block, &column_id));
         arguments[i] = column_id;
 
         block->replace_by_position_if_const(column_id);
diff --git a/be/src/vec/exprs/vcast_expr.cpp b/be/src/vec/exprs/vcast_expr.cpp
index f910dfd466..d0286bdce6 100644
--- a/be/src/vec/exprs/vcast_expr.cpp
+++ b/be/src/vec/exprs/vcast_expr.cpp
@@ -75,7 +75,7 @@ doris::Status VCastExpr::execute(VExprContext* context, 
doris::vectorized::Block
                                  int* result_column_id) {
     // for each child call execute
     int column_id = 0;
-    _children[0]->execute(context, block, &column_id);
+    RETURN_IF_ERROR(_children[0]->execute(context, block, &column_id));
 
     size_t const_param_id = VExpr::insert_param(
             block, {_cast_param, _cast_param_data_type, 
_target_data_type_name}, block->rows());
diff --git a/be/src/vec/exprs/vectorized_fn_call.cpp 
b/be/src/vec/exprs/vectorized_fn_call.cpp
index 6f01a126d6..d0850c9cf2 100644
--- a/be/src/vec/exprs/vectorized_fn_call.cpp
+++ b/be/src/vec/exprs/vectorized_fn_call.cpp
@@ -19,6 +19,7 @@
 
 #include <string_view>
 
+#include "common/status.h"
 #include "exprs/anyval_util.h"
 #include "fmt/format.h"
 #include "fmt/ranges.h"
@@ -79,7 +80,7 @@ doris::Status VectorizedFnCall::execute(VExprContext* 
context, doris::vectorized
     doris::vectorized::ColumnNumbers arguments(_children.size());
     for (int i = 0; i < _children.size(); ++i) {
         int column_id = -1;
-        _children[i]->execute(context, block, &column_id);
+        RETURN_IF_ERROR(_children[i]->execute(context, block, &column_id));
         arguments[i] = column_id;
     }
     // call function
diff --git a/be/src/vec/exprs/vin_predicate.cpp 
b/be/src/vec/exprs/vin_predicate.cpp
index f8c096d8ee..3a84087bd7 100644
--- a/be/src/vec/exprs/vin_predicate.cpp
+++ b/be/src/vec/exprs/vin_predicate.cpp
@@ -19,6 +19,7 @@
 
 #include <string_view>
 
+#include "common/status.h"
 #include "exprs/create_predicate_function.h"
 
 #include "vec/columns/column_set.h"
@@ -89,7 +90,7 @@ Status VInPredicate::execute(VExprContext* context, Block* 
block, int* result_co
     doris::vectorized::ColumnNumbers arguments(_children.size());
     for (int i = 0; i < _children.size(); ++i) {
         int column_id = -1;
-        _children[i]->execute(context, block, &column_id);
+        RETURN_IF_ERROR(_children[i]->execute(context, block, &column_id));
         arguments[i] = column_id;
     }
     // call function
diff --git a/be/src/vec/exprs/vslot_ref.cpp b/be/src/vec/exprs/vslot_ref.cpp
index 8cbc56cb37..81b845cb97 100644
--- a/be/src/vec/exprs/vslot_ref.cpp
+++ b/be/src/vec/exprs/vslot_ref.cpp
@@ -26,16 +26,13 @@ namespace doris::vectorized {
 using doris::Status;
 using doris::SlotDescriptor;
 VSlotRef::VSlotRef(const doris::TExprNode& node)
-        : VExpr(node),
-          _slot_id(node.slot_ref.slot_id),
-          _column_id(-1),
-          _column_name(nullptr) {
-            if (node.__isset.is_nullable) {
-              _is_nullable = node.is_nullable;
-            } else {
-              _is_nullable = true;
-            }
-        }
+        : VExpr(node), _slot_id(node.slot_ref.slot_id), _column_id(-1), 
_column_name(nullptr) {
+    if (node.__isset.is_nullable) {
+        _is_nullable = node.is_nullable;
+    } else {
+        _is_nullable = true;
+    }
+}
 
 VSlotRef::VSlotRef(const SlotDescriptor* desc)
         : VExpr(desc->type(), true, desc->is_nullable()),
@@ -60,7 +57,11 @@ Status VSlotRef::prepare(doris::RuntimeState* state, const 
doris::RowDescriptor&
 }
 
 Status VSlotRef::execute(VExprContext* context, Block* block, int* 
result_column_id) {
-    CHECK_GE(_column_id, 0) << ", " << debug_string() << ", " << 
get_stack_trace();
+    // comment DCHECK temporarily to make fuzzy test run smoothly
+    // DCHECK_GE(_column_id, 0);
+    if (_column_id < 0) {
+        return Status::InternalError("invalid column id {}", _column_id);
+    }
     *result_column_id = _column_id;
     return Status::OK();
 }
@@ -70,7 +71,8 @@ const std::string& VSlotRef::expr_name() const {
 }
 std::string VSlotRef::debug_string() const {
     std::stringstream out;
-    out << "SlotRef(slot_id=" << _slot_id << VExpr::debug_string() << ") 
column id: " << _column_id << ", name: " << *_column_name << ", is nulable: " 
<< _is_nullable;
+    out << "SlotRef(slot_id=" << _slot_id << VExpr::debug_string() << ") 
column id: " << _column_id
+        << ", name: " << *_column_name << ", is nulable: " << _is_nullable;
     return out.str();
 }
 } // namespace doris::vectorized


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to