Repository: incubator-impala Updated Branches: refs/heads/master ee2a06d82 -> b15d992ab
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exec/topn-node.h ---------------------------------------------------------------------- diff --git a/be/src/exec/topn-node.h b/be/src/exec/topn-node.h index 6c2bcae..5bd7ded 100644 --- a/be/src/exec/topn-node.h +++ b/be/src/exec/topn-node.h @@ -45,6 +45,7 @@ class TopNNode : public ExecNode { virtual Status Init(const TPlanNode& tnode, RuntimeState* state); virtual Status Prepare(RuntimeState* state); + virtual void Codegen(RuntimeState* state); virtual Status Open(RuntimeState* state); virtual Status GetNext(RuntimeState* state, RowBatch* row_batch, bool* eos); virtual Status Reset(RuntimeState* state); @@ -57,9 +58,6 @@ class TopNNode : public ExecNode { friend class TupleLessThan; - /// Creates a codegen'd version of InsertBatch() that is used in Open(). - Status Codegen(RuntimeState* state); - /// Inserts all the rows in 'batch' into the queue. void InsertBatch(RowBatch* batch); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/case-expr.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/case-expr.cc b/be/src/exprs/case-expr.cc index 5771f09..2262c22 100644 --- a/be/src/exprs/case-expr.cc +++ b/be/src/exprs/case-expr.cc @@ -182,7 +182,7 @@ string CaseExpr::DebugString() const { // %"class.impala::TupleRow"* %row) // ret i16 %else_val // } -Status CaseExpr::GetCodegendComputeFn(RuntimeState* state, Function** fn) { +Status CaseExpr::GetCodegendComputeFn(LlvmCodeGen* codegen, Function** fn) { if (ir_compute_fn_ != NULL) { *fn = ir_compute_fn_; return Status::OK(); @@ -191,11 +191,9 @@ Status CaseExpr::GetCodegendComputeFn(RuntimeState* state, Function** fn) { const int num_children = GetNumChildren(); Function* child_fns[num_children]; for (int i = 0; i < num_children; ++i) { - RETURN_IF_ERROR(children()[i]->GetCodegendComputeFn(state, &child_fns[i])); + RETURN_IF_ERROR(children()[i]->GetCodegendComputeFn(codegen, &child_fns[i])); } - LlvmCodeGen* codegen; - RETURN_IF_ERROR(state->GetCodegen(&codegen)); LLVMContext& context = codegen->context(); LlvmCodeGen::LlvmBuilder builder(context); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/case-expr.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/case-expr.h b/be/src/exprs/case-expr.h index fbf6f85..87bab76 100644 --- a/be/src/exprs/case-expr.h +++ b/be/src/exprs/case-expr.h @@ -30,7 +30,7 @@ class TExprNode; class CaseExpr: public Expr { public: - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn); virtual BooleanVal GetBooleanVal(ExprContext* ctx, const TupleRow* row); virtual TinyIntVal GetTinyIntVal(ExprContext* ctx, const TupleRow* row); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/compound-predicates.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/compound-predicates.cc b/be/src/exprs/compound-predicates.cc index 94eb269..531a70c 100644 --- a/be/src/exprs/compound-predicates.cc +++ b/be/src/exprs/compound-predicates.cc @@ -65,8 +65,8 @@ string OrPredicate::DebugString() const { return out.str(); } -// IR codegen for compound and/or predicates. Compound predicate has non trivial -// null handling as well as many branches so this is pretty complicated. The IR +// IR codegen for compound and/or predicates. Compound predicate has non trivial +// null handling as well as many branches so this is pretty complicated. The IR // for x && y is: // // define i16 @CompoundPredicate(%"class.impala::ExprContext"* %context, @@ -86,30 +86,30 @@ string OrPredicate::DebugString() const { // %val2 = trunc i8 %3 to i1 // %tmp_and = and i1 %val, %val2 // br i1 %is_null, label %lhs_null, label %lhs_not_null -// +// // lhs_null: ; preds = %entry // br i1 %is_null1, label %null_block, label %lhs_null_rhs_not_null -// +// // lhs_not_null: ; preds = %entry // br i1 %is_null1, label %lhs_not_null_rhs_null, label %not_null_block -// +// // lhs_null_rhs_not_null: ; preds = %lhs_null // br i1 %val2, label %null_block, label %not_null_block -// +// // lhs_not_null_rhs_null: ; preds = %lhs_not_null // br i1 %val, label %null_block, label %not_null_block -// +// // null_block: ; preds = %lhs_null_rhs_not_null, // %lhs_not_null_rhs_null, %lhs_null // br label %ret -// +// // not_null_block: ; preds = %lhs_null_rhs_not_null, // %lhs_not_null_rhs_null, %lhs_not_null // %4 = phi i1 [ false, %lhs_null_rhs_not_null ], // [ false, %lhs_not_null_rhs_null ], // [ %tmp_and, %lhs_not_null ] // br label %ret -// +// // ret: ; preds = %not_null_block, %null_block // %ret3 = phi i1 [ false, %null_block ], [ %4, %not_null_block ] // %5 = zext i1 %ret3 to i16 @@ -118,21 +118,18 @@ string OrPredicate::DebugString() const { // ret i16 %7 // } Status CompoundPredicate::CodegenComputeFn( - bool and_fn, RuntimeState* state, Function** fn) { + bool and_fn, LlvmCodeGen* codegen, Function** fn) { if (ir_compute_fn_ != NULL) { *fn = ir_compute_fn_; return Status::OK(); } DCHECK_EQ(GetNumChildren(), 2); - Function* lhs_function; - RETURN_IF_ERROR(children()[0]->GetCodegendComputeFn(state, &lhs_function)); + RETURN_IF_ERROR(children()[0]->GetCodegendComputeFn(codegen, &lhs_function)); Function* rhs_function; - RETURN_IF_ERROR(children()[1]->GetCodegendComputeFn(state, &rhs_function)); - - LlvmCodeGen* codegen; - RETURN_IF_ERROR(state->GetCodegen(&codegen)); + RETURN_IF_ERROR(children()[1]->GetCodegendComputeFn(codegen, &rhs_function)); + LLVMContext& context = codegen->context(); LlvmCodeGen::LlvmBuilder builder(context); Value* args[2]; @@ -143,11 +140,11 @@ Status CompoundPredicate::CodegenComputeFn( // Control blocks for aggregating results BasicBlock* lhs_null_block = BasicBlock::Create(context, "lhs_null", function); - BasicBlock* lhs_not_null_block = + BasicBlock* lhs_not_null_block = BasicBlock::Create(context, "lhs_not_null", function); - BasicBlock* lhs_null_rhs_not_null_block = + BasicBlock* lhs_null_rhs_not_null_block = BasicBlock::Create(context, "lhs_null_rhs_not_null", function); - BasicBlock* lhs_not_null_rhs_null_block = + BasicBlock* lhs_not_null_rhs_null_block = BasicBlock::Create(context, "lhs_not_null_rhs_null", function); BasicBlock* null_block = BasicBlock::Create(context, "null_block", function); BasicBlock* not_null_block = BasicBlock::Create(context, "not_null_block", function); @@ -159,7 +156,7 @@ Status CompoundPredicate::CodegenComputeFn( // Call rhs CodegenAnyVal rhs_result = CodegenAnyVal::CreateCallWrapped( codegen, &builder, TYPE_BOOLEAN, rhs_function, args, "rhs_call"); - + Value* lhs_is_null = lhs_result.GetIsNull(); Value* rhs_is_null = rhs_result.GetIsNull(); Value* lhs_value = lhs_result.GetVal(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/compound-predicates.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/compound-predicates.h b/be/src/exprs/compound-predicates.h index 3ba4ad0..0e403ba 100644 --- a/be/src/exprs/compound-predicates.h +++ b/be/src/exprs/compound-predicates.h @@ -34,7 +34,7 @@ class CompoundPredicate: public Predicate { protected: CompoundPredicate(const TExprNode& node) : Predicate(node) { } - Status CodegenComputeFn(bool and_fn, RuntimeState* state, llvm::Function** fn); + Status CodegenComputeFn(bool and_fn, LlvmCodeGen* codegen, llvm::Function** fn); }; /// Expr for evaluating and (&&) operators @@ -42,8 +42,8 @@ class AndPredicate: public CompoundPredicate { public: virtual impala_udf::BooleanVal GetBooleanVal(ExprContext* context, const TupleRow*); - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn) { - return CompoundPredicate::CodegenComputeFn(true, state, fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn) { + return CompoundPredicate::CodegenComputeFn(true, codegen, fn); } protected: @@ -61,8 +61,8 @@ class OrPredicate: public CompoundPredicate { public: virtual impala_udf::BooleanVal GetBooleanVal(ExprContext* context, const TupleRow*); - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn) { - return CompoundPredicate::CodegenComputeFn(false, state, fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn) { + return CompoundPredicate::CodegenComputeFn(false, codegen, fn); } protected: http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/conditional-functions.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/conditional-functions.cc b/be/src/exprs/conditional-functions.cc index eed9b8e..6f8f362 100644 --- a/be/src/exprs/conditional-functions.cc +++ b/be/src/exprs/conditional-functions.cc @@ -25,8 +25,8 @@ using namespace impala; using namespace impala_udf; #define CONDITIONAL_CODEGEN_FN(expr_class) \ - Status expr_class::GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn) { \ - return GetCodegendComputeFnWrapper(state, fn); \ + Status expr_class::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn) { \ + return GetCodegendComputeFnWrapper(codegen, fn); \ } CONDITIONAL_CODEGEN_FN(IsNullExpr); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/conditional-functions.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/conditional-functions.h b/be/src/exprs/conditional-functions.h index 8783c6e..12c9b1e 100644 --- a/be/src/exprs/conditional-functions.h +++ b/be/src/exprs/conditional-functions.h @@ -73,7 +73,7 @@ class IsNullExpr : public Expr { virtual TimestampVal GetTimestampVal(ExprContext* context, const TupleRow* row); virtual DecimalVal GetDecimalVal(ExprContext* context, const TupleRow* row); - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn); virtual std::string DebugString() const { return Expr::DebugString("IsNullExpr"); } protected: @@ -94,7 +94,7 @@ class NullIfExpr : public Expr { virtual TimestampVal GetTimestampVal(ExprContext* context, const TupleRow* row); virtual DecimalVal GetDecimalVal(ExprContext* context, const TupleRow* row); - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn); virtual std::string DebugString() const { return Expr::DebugString("NullIfExpr"); } protected: @@ -115,7 +115,7 @@ class IfExpr : public Expr { virtual TimestampVal GetTimestampVal(ExprContext* context, const TupleRow* row); virtual DecimalVal GetDecimalVal(ExprContext* context, const TupleRow* row); - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn); virtual std::string DebugString() const { return Expr::DebugString("IfExpr"); } protected: @@ -141,7 +141,7 @@ class CoalesceExpr : public Expr { protected: friend class Expr; CoalesceExpr(const TExprNode& node) : Expr(node) { } - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn); }; } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/expr.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/expr.cc b/be/src/exprs/expr.cc index 0e3610d..8d21830 100644 --- a/be/src/exprs/expr.cc +++ b/be/src/exprs/expr.cc @@ -267,6 +267,16 @@ Status Expr::CreateExpr(ObjectPool* pool, const TExprNode& texpr_node, Expr** ex } } +bool Expr::NeedCodegen(const TExpr& texpr) { + for (const TExprNode& texpr_node : texpr.nodes) { + if (texpr_node.node_type == TExprNodeType::FUNCTION_CALL && texpr_node.__isset.fn && + texpr_node.fn.binary_type == TFunctionBinaryType::IR) { + return true; + } + } + return false; +} + struct MemLayoutData { int expr_idx; int byte_size; @@ -655,13 +665,11 @@ int Expr::InlineConstants(const FunctionContext::TypeDesc& return_type, return replaced; } -Status Expr::GetCodegendComputeFnWrapper(RuntimeState* state, Function** fn) { +Status Expr::GetCodegendComputeFnWrapper(LlvmCodeGen* codegen, Function** fn) { if (ir_compute_fn_ != NULL) { *fn = ir_compute_fn_; return Status::OK(); } - LlvmCodeGen* codegen; - RETURN_IF_ERROR(state->GetCodegen(&codegen)); Function* static_getval_fn = GetStaticGetValWrapper(type(), codegen); // Call it passing this as the additional first argument. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/expr.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/expr.h b/be/src/exprs/expr.h index 0cdafeb..99d463d 100644 --- a/be/src/exprs/expr.h +++ b/be/src/exprs/expr.h @@ -173,6 +173,10 @@ class Expr { /// Returns the number of slots added to the vector virtual int GetSlotIds(std::vector<SlotId>* slot_ids) const; + /// Returns true iff the expression 'texpr' contains UDF available only as LLVM IR. In + /// which case, it's impossible to interpret this expression and codegen must be used. + static bool NeedCodegen(const TExpr& texpr); + /// Create expression tree from the list of nodes contained in texpr within 'pool'. /// Returns the root of expression tree in 'expr' and the corresponding ExprContext in /// 'ctx'. @@ -234,7 +238,7 @@ class Expr { // /// The function should evaluate this expr over 'row' and return the result as the /// appropriate type of AnyVal. - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn) = 0; + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn) = 0; /// If this expr is constant, evaluates the expr with no input row argument and returns /// the output. Returns NULL if the argument is not constant. The returned AnyVal* is @@ -389,7 +393,7 @@ class Expr { /// functions that use the IRBuilder. It doesn't provide any performance benefit over /// the interpreted path. /// TODO: this should be replaced with fancier xcompiling infrastructure - Status GetCodegendComputeFnWrapper(RuntimeState* state, llvm::Function** fn); + Status GetCodegendComputeFnWrapper(LlvmCodeGen* codegen, llvm::Function** fn); /// Returns the IR version of the static Get*Val() wrapper function corresponding to /// 'type'. This is used for calling interpreted Get*Val() functions from codegen'd http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/hive-udf-call.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/hive-udf-call.cc b/be/src/exprs/hive-udf-call.cc index c7a3f32..453f876 100644 --- a/be/src/exprs/hive-udf-call.cc +++ b/be/src/exprs/hive-udf-call.cc @@ -277,8 +277,8 @@ void HiveUdfCall::Close(RuntimeState* state, ExprContext* ctx, Expr::Close(state, ctx, scope); } -Status HiveUdfCall::GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn) { - return GetCodegendComputeFnWrapper(state, fn); +Status HiveUdfCall::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn) { + return GetCodegendComputeFnWrapper(codegen, fn); } string HiveUdfCall::DebugString() const { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/hive-udf-call.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/hive-udf-call.h b/be/src/exprs/hive-udf-call.h index 8a70540..74302eb 100644 --- a/be/src/exprs/hive-udf-call.h +++ b/be/src/exprs/hive-udf-call.h @@ -82,7 +82,7 @@ class HiveUdfCall : public Expr { virtual TimestampVal GetTimestampVal(ExprContext* ctx, const TupleRow*); virtual DecimalVal GetDecimalVal(ExprContext* ctx, const TupleRow*); - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn); protected: friend class Expr; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/is-not-empty-predicate.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/is-not-empty-predicate.cc b/be/src/exprs/is-not-empty-predicate.cc index 88aac42..521ebf4 100644 --- a/be/src/exprs/is-not-empty-predicate.cc +++ b/be/src/exprs/is-not-empty-predicate.cc @@ -42,9 +42,9 @@ Status IsNotEmptyPredicate::Prepare(RuntimeState* state, return Status::OK(); } -Status IsNotEmptyPredicate::GetCodegendComputeFn(RuntimeState* state, +Status IsNotEmptyPredicate::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn) { - return GetCodegendComputeFnWrapper(state, fn); + return GetCodegendComputeFnWrapper(codegen, fn); } string IsNotEmptyPredicate::DebugString() const { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/is-not-empty-predicate.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/is-not-empty-predicate.h b/be/src/exprs/is-not-empty-predicate.h index d8e15be..2454a6d 100644 --- a/be/src/exprs/is-not-empty-predicate.h +++ b/be/src/exprs/is-not-empty-predicate.h @@ -31,7 +31,7 @@ class IsNotEmptyPredicate: public Predicate { public: virtual Status Prepare(RuntimeState* state, const RowDescriptor& row_desc, ExprContext* ctx); - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn); virtual BooleanVal GetBooleanVal(ExprContext* context, const TupleRow* row); virtual std::string DebugString() const; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/literal.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/literal.cc b/be/src/exprs/literal.cc index 1d816c3..caf6c14 100644 --- a/be/src/exprs/literal.cc +++ b/be/src/exprs/literal.cc @@ -375,15 +375,13 @@ string Literal::DebugString() const { // entry: // ret { i8, i64 } { i8 0, i64 10 } // } -Status Literal::GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn) { +Status Literal::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn) { if (ir_compute_fn_ != NULL) { *fn = ir_compute_fn_; return Status::OK(); } DCHECK_EQ(GetNumChildren(), 0); - LlvmCodeGen* codegen; - RETURN_IF_ERROR(state->GetCodegen(&codegen)); Value* args[2]; *fn = CreateIrFunctionPrototype(codegen, "Literal", &args); BasicBlock* entry_block = BasicBlock::Create(codegen->context(), "entry", *fn); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/literal.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/literal.h b/be/src/exprs/literal.h index 24bc913..65219e0 100644 --- a/be/src/exprs/literal.h +++ b/be/src/exprs/literal.h @@ -45,7 +45,7 @@ class Literal: public Expr { /// Literal. static Literal* CreateLiteral(const ColumnType& type, const std::string& str); - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn); virtual impala_udf::BooleanVal GetBooleanVal(ExprContext*, const TupleRow*); virtual impala_udf::TinyIntVal GetTinyIntVal(ExprContext*, const TupleRow*); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/null-literal.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/null-literal.cc b/be/src/exprs/null-literal.cc index 54d6247..a8f4241 100644 --- a/be/src/exprs/null-literal.cc +++ b/be/src/exprs/null-literal.cc @@ -91,15 +91,13 @@ CollectionVal NullLiteral::GetCollectionVal(ExprContext* context, const TupleRow // entry: // ret { i8, i64 } { i8 1, i64 0 } // } -Status NullLiteral::GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn) { +Status NullLiteral::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn) { if (ir_compute_fn_ != NULL) { *fn = ir_compute_fn_; return Status::OK(); } DCHECK_EQ(GetNumChildren(), 0); - LlvmCodeGen* codegen; - RETURN_IF_ERROR(state->GetCodegen(&codegen)); Value* args[2]; *fn = CreateIrFunctionPrototype(codegen, "NullLiteral", &args); BasicBlock* entry_block = BasicBlock::Create(codegen->context(), "entry", *fn); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/null-literal.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/null-literal.h b/be/src/exprs/null-literal.h index 9297cf6..0a0f9a1 100644 --- a/be/src/exprs/null-literal.h +++ b/be/src/exprs/null-literal.h @@ -28,7 +28,7 @@ class TExprNode; class NullLiteral: public Expr { public: NullLiteral(PrimitiveType type) : Expr(type) { } - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn); virtual impala_udf::BooleanVal GetBooleanVal(ExprContext*, const TupleRow*); virtual impala_udf::TinyIntVal GetTinyIntVal(ExprContext*, const TupleRow*); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/scalar-fn-call.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/scalar-fn-call.cc b/be/src/exprs/scalar-fn-call.cc index b2fce53..17cbd64 100644 --- a/be/src/exprs/scalar-fn-call.cc +++ b/be/src/exprs/scalar-fn-call.cc @@ -22,6 +22,11 @@ #include <llvm/IR/Attributes.h> #include <llvm/ExecutionEngine/ExecutionEngine.h> +#include <boost/preprocessor/punctuation/comma_if.hpp> +#include <boost/preprocessor/repetition/repeat.hpp> +#include <boost/preprocessor/repetition/enum_params.hpp> +#include <boost/preprocessor/repetition/repeat_from_to.hpp> + #include "codegen/codegen-anyval.h" #include "codegen/llvm-codegen.h" #include "exprs/anyval-util.h" @@ -41,6 +46,9 @@ using namespace impala; using namespace impala_udf; using namespace strings; +// Maximum number of arguments the interpretation path supports. +#define MAX_INTERP_ARGS 20 + ScalarFnCall::ScalarFnCall(const TExprNode& node) : Expr(node), vararg_start_idx_(node.__isset.vararg_start_idx ? @@ -89,33 +97,30 @@ Status ScalarFnCall::Prepare(RuntimeState* state, const RowDescriptor& desc, varargs_buffer_size); // Use the interpreted path and call the builtin without codegen if: - // 1. there are char arguments (as they aren't supported yet) - // OR - // if all of the following conditions are satisfied: - // 2. the codegen object hasn't been created yet. - // 3. the planner doesn't insist on using codegen. - // 4. we're calling a builtin or native UDF with <= 8 non-variadic arguments. - // The templates for UDFs used in the interpretation path support up to 8 - // arguments only. - // - // This saves us the overhead of creating the codegen object when it's not necessary - // (i.e., in plan fragments with no codegen-enabled operators). + // 1. codegen is disabled or + // 2. there are char arguments (as they aren't supported yet) // // TODO: codegen for char arguments - // TODO: remove condition 2 above and put a flag in the RuntimeState to indicate - // if codegen should be enabled for the entire fragment. - bool skip_codegen = false; - if (has_char_arg_or_result) { - skip_codegen = true; - } else if (!state->codegen_created() && !state->ShouldCodegenExpr()) { - skip_codegen = fn_.binary_type != TFunctionBinaryType::IR && NumFixedArgs() <= 8; - } - if (skip_codegen) { - // Builtins with char arguments must still have <= 8 arguments. - // TODO: delete when we have codegen for char arguments - if (has_char_arg_or_result) { - DCHECK(NumFixedArgs() <= 8 && fn_.binary_type == TFunctionBinaryType::BUILTIN); + bool codegen_enabled = state->codegen_enabled(); + if (!codegen_enabled || has_char_arg_or_result) { + if (fn_.binary_type == TFunctionBinaryType::IR) { + // CHAR or VARCHAR are not supported as input arguments or return values for UDFs. + DCHECK(!has_char_arg_or_result && !codegen_enabled); + return Status(Substitute("Cannot interpret LLVM IR UDF '$0': Codegen is needed. " + "Please set DISABLE_CODEGEN to false.", fn_.name.function_name)); } + + // The templates for builtin or native UDFs used in the interpretation path + // support up to 20 arguments only. + if (NumFixedArgs() > MAX_INTERP_ARGS) { + DCHECK_EQ(fn_.binary_type, TFunctionBinaryType::NATIVE); + // CHAR or VARCHAR are not supported as input arguments or return values for UDFs. + DCHECK(!has_char_arg_or_result && !codegen_enabled); + return Status(Substitute("Cannot interpret native UDF '$0': number of arguments is " + "more than $1. Codegen is needed. Please set DISABLE_CODEGEN to false.", + fn_.name.function_name, MAX_INTERP_ARGS)); + } + Status status = LibCache::instance()->GetSoFunctionPtr( fn_.hdfs_location, fn_.scalar_fn.symbol, &scalar_fn_, &cache_entry_); if (!status.ok()) { @@ -127,13 +132,12 @@ Status ScalarFnCall::Prepare(RuntimeState* state, const RowDescriptor& desc, DCHECK_EQ(fn_.binary_type, TFunctionBinaryType::NATIVE); return Status(Substitute("Problem loading UDF '$0':\n$1", fn_.name.function_name, status.GetDetail())); - return status; } } } else { // If we got here, either codegen is enabled or we need codegen to run this function. - LlvmCodeGen* codegen; - RETURN_IF_ERROR(state->GetCodegen(&codegen)); + LlvmCodeGen* codegen = state->codegen(); + DCHECK(codegen != NULL); if (fn_.binary_type == TFunctionBinaryType::IR) { string local_path; @@ -146,7 +150,7 @@ Status ScalarFnCall::Prepare(RuntimeState* state, const RowDescriptor& desc, } llvm::Function* ir_udf_wrapper; - RETURN_IF_ERROR(GetCodegendComputeFn(state, &ir_udf_wrapper)); + RETURN_IF_ERROR(GetCodegendComputeFn(codegen, &ir_udf_wrapper)); // TODO: don't do this for child exprs codegen->AddFunctionToJit(ir_udf_wrapper, &scalar_fn_wrapper_); } @@ -260,7 +264,7 @@ bool ScalarFnCall::IsConstant() const { // i32 4, // i64* inttoptr (i64 89111072 to i64*)) // ret { i8, double } %result -Status ScalarFnCall::GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn) { +Status ScalarFnCall::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn) { if (ir_compute_fn_ != NULL) { *fn = ir_compute_fn_; return Status::OK(); @@ -275,11 +279,8 @@ Status ScalarFnCall::GetCodegendComputeFn(RuntimeState* state, llvm::Function** } } - LlvmCodeGen* codegen; - RETURN_IF_ERROR(state->GetCodegen(&codegen)); - llvm::Function* udf; - RETURN_IF_ERROR(GetUdf(state, &udf)); + RETURN_IF_ERROR(GetUdf(codegen, &udf)); // Create wrapper that computes args and calls UDF stringstream fn_name; @@ -326,10 +327,9 @@ Status ScalarFnCall::GetCodegendComputeFn(RuntimeState* state, llvm::Function** for (int i = 0; i < GetNumChildren(); ++i) { llvm::Function* child_fn = NULL; vector<llvm::Value*> child_fn_args; - if (state->codegen_enabled()) { - // Set 'child_fn' to the codegen'd function, sets child_fn = NULL if codegen fails - children_[i]->GetCodegendComputeFn(state, &child_fn); - } + // Set 'child_fn' to the codegen'd function, sets child_fn = NULL if codegen fails + children_[i]->GetCodegendComputeFn(codegen, &child_fn); + if (child_fn == NULL) { // Set 'child_fn' to the interpreted function child_fn = GetStaticGetValWrapper(children_[i]->type(), codegen); @@ -401,21 +401,16 @@ Status ScalarFnCall::GetCodegendComputeFn(RuntimeState* state, llvm::Function** return Status::OK(); } -Status ScalarFnCall::GetUdf(RuntimeState* state, llvm::Function** udf) { - LlvmCodeGen* codegen; - RETURN_IF_ERROR(state->GetCodegen(&codegen)); - - // from_utc_timestamp and to_utc_timestamp have inline ASM that cannot be JIT'd. +Status ScalarFnCall::GetUdf(LlvmCodeGen* codegen, llvm::Function** udf) { + // from_utc_timestamp() and to_utc_timestamp() have inline ASM that cannot be JIT'd. // TimestampFunctions::AddSub() contains a try/catch which doesn't work in JIT'd - // code. Always use the statically compiled versions of these functions so the - // xcompiled versions are not included in the final module to be JIT'd. - // TODO: fix this + // code. Always use the interpreted version of these functions. + // TODO: fix these built-in functions so we don't need 'broken_builtin' below. bool broken_builtin = fn_.name.function_name == "from_utc_timestamp" || fn_.name.function_name == "to_utc_timestamp" || fn_.scalar_fn.symbol.find("AddSub") != string::npos; if (fn_.binary_type == TFunctionBinaryType::NATIVE || - (fn_.binary_type == TFunctionBinaryType::BUILTIN && - (!state->codegen_enabled() || broken_builtin))) { + (fn_.binary_type == TFunctionBinaryType::BUILTIN && broken_builtin)) { // In this path, we are code that has been statically compiled to assembly. // This can either be a UDF implemented in a .so or a builtin using the UDF // interface with the code in impalad. @@ -478,7 +473,6 @@ Status ScalarFnCall::GetUdf(RuntimeState* state, llvm::Function** udf) { } else if (fn_.binary_type == TFunctionBinaryType::BUILTIN) { // In this path, we're running a builtin with the UDF interface. The IR is // in the llvm module. - DCHECK(state->codegen_enabled()); *udf = codegen->GetFunction(fn_.scalar_fn.symbol); if (*udf == NULL) { // Builtins symbols should exist unless there is a version mismatch. @@ -522,8 +516,8 @@ Status ScalarFnCall::GetFunction(RuntimeState* state, const string& symbol, void &cache_entry_); } else { DCHECK_EQ(fn_.binary_type, TFunctionBinaryType::IR); - LlvmCodeGen* codegen; - RETURN_IF_ERROR(state->GetCodegen(&codegen)); + LlvmCodeGen* codegen = state->codegen(); + DCHECK(codegen != NULL); llvm::Function* ir_fn = codegen->GetFunction(symbol); if (ir_fn == NULL) { stringstream ss; @@ -563,122 +557,56 @@ RETURN_TYPE ScalarFnCall::InterpretEval(ExprContext* context, const TupleRow* ro if (vararg_start_idx_ == -1) { switch (children_.size()) { - case 0: - typedef RETURN_TYPE (*ScalarFn0)(FunctionContext*); - return reinterpret_cast<ScalarFn0>(scalar_fn_)(fn_ctx); - case 1: - typedef RETURN_TYPE (*ScalarFn1)(FunctionContext*, const AnyVal& a1); - return reinterpret_cast<ScalarFn1>(scalar_fn_)(fn_ctx, - *(*input_vals)[0]); - case 2: - typedef RETURN_TYPE (*ScalarFn2)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2); - return reinterpret_cast<ScalarFn2>(scalar_fn_)(fn_ctx, - *(*input_vals)[0], *(*input_vals)[1]); - case 3: - typedef RETURN_TYPE (*ScalarFn3)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2, const AnyVal& a3); - return reinterpret_cast<ScalarFn3>(scalar_fn_)(fn_ctx, - *(*input_vals)[0], *(*input_vals)[1], *(*input_vals)[2]); - case 4: - typedef RETURN_TYPE (*ScalarFn4)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2, const AnyVal& a3, const AnyVal& a4); - return reinterpret_cast<ScalarFn4>(scalar_fn_)(fn_ctx, - *(*input_vals)[0], *(*input_vals)[1], *(*input_vals)[2], *(*input_vals)[3]); - case 5: - typedef RETURN_TYPE (*ScalarFn5)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2, const AnyVal& a3, const AnyVal& a4, const AnyVal& a5); - return reinterpret_cast<ScalarFn5>(scalar_fn_)(fn_ctx, - *(*input_vals)[0], *(*input_vals)[1], *(*input_vals)[2], *(*input_vals)[3], - *(*input_vals)[4]); - case 6: - typedef RETURN_TYPE (*ScalarFn6)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2, const AnyVal& a3, const AnyVal& a4, const AnyVal& a5, - const AnyVal& a6); - return reinterpret_cast<ScalarFn6>(scalar_fn_)(fn_ctx, - *(*input_vals)[0], *(*input_vals)[1], *(*input_vals)[2], *(*input_vals)[3], - *(*input_vals)[4], *(*input_vals)[5]); - case 7: - typedef RETURN_TYPE (*ScalarFn7)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2, const AnyVal& a3, const AnyVal& a4, const AnyVal& a5, - const AnyVal& a6, const AnyVal& a7); - return reinterpret_cast<ScalarFn7>(scalar_fn_)(fn_ctx, - *(*input_vals)[0], *(*input_vals)[1], *(*input_vals)[2], *(*input_vals)[3], - *(*input_vals)[4], *(*input_vals)[5], *(*input_vals)[6]); - case 8: - typedef RETURN_TYPE (*ScalarFn8)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2, const AnyVal& a3, const AnyVal& a4, const AnyVal& a5, - const AnyVal& a6, const AnyVal& a7, const AnyVal& a8); - return reinterpret_cast<ScalarFn8>(scalar_fn_)(fn_ctx, - *(*input_vals)[0], *(*input_vals)[1], *(*input_vals)[2], *(*input_vals)[3], - *(*input_vals)[4], *(*input_vals)[5], *(*input_vals)[6], *(*input_vals)[7]); + +#define ARG_DECL_ONE(z, n, data) BOOST_PP_COMMA_IF(n) const AnyVal& +#define ARG_DECL_LIST(n) \ + FunctionContext* BOOST_PP_COMMA_IF(n) BOOST_PP_REPEAT(n, ARG_DECL_ONE, unused) +#define ARG_ONE(z, n, data) BOOST_PP_COMMA_IF(n) *(*input_vals)[n] +#define ARG_LIST(n) fn_ctx BOOST_PP_COMMA_IF(n) BOOST_PP_REPEAT(n, ARG_ONE, unused) + + // Expands to code snippet like the following for X from 0 to 20: + // case X: + // typedef RETURN_TYPE (*ScalarFnX)(FunctionContext*, const AnyVal& a1, ..., + // const AnyVal& aX); + // return reinterpret_cast<ScalarFnn>(scalar_fn_)(fn_ctx, *(*input_vals)[0], ..., + // *(*input_vals)[X-1]); +#define SCALAR_FN_TYPE(n) BOOST_PP_CAT(ScalarFn, n) +#define INTERP_SCALAR_FN(z, n, unused) \ + case n: \ + typedef RETURN_TYPE (*SCALAR_FN_TYPE(n))(ARG_DECL_LIST(n)); \ + return reinterpret_cast<SCALAR_FN_TYPE(n)>(scalar_fn_)(ARG_LIST(n)); + + // Support up to MAX_INTERP_ARGS arguments in the interpretation path + BOOST_PP_REPEAT_FROM_TO(0, BOOST_PP_ADD(MAX_INTERP_ARGS, 1), + INTERP_SCALAR_FN, unused) + default: - DCHECK(false) << "Interpreted path not implemented. We should have " - << "codegen'd the wrapper"; + DCHECK(false) << "Interpreted path not implemented."; } - } else { + } else { int num_varargs = children_.size() - NumFixedArgs(); const AnyVal* varargs = reinterpret_cast<AnyVal*>(fn_ctx->impl()->varargs_buffer()); switch (NumFixedArgs()) { - case 0: - typedef RETURN_TYPE (*VarargFn0)(FunctionContext*, int num_varargs, - const AnyVal* varargs); - return reinterpret_cast<VarargFn0>(scalar_fn_)(fn_ctx, num_varargs, varargs); - case 1: - typedef RETURN_TYPE (*VarargFn1)(FunctionContext*, const AnyVal& a1, - int num_varargs, const AnyVal* varargs); - return reinterpret_cast<VarargFn1>(scalar_fn_)(fn_ctx, *(*input_vals)[0], - num_varargs, varargs); - case 2: - typedef RETURN_TYPE (*VarargFn2)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2, int num_varargs, const AnyVal* varargs); - return reinterpret_cast<VarargFn2>(scalar_fn_)(fn_ctx, *(*input_vals)[0], - *(*input_vals)[1], num_varargs, varargs); - case 3: - typedef RETURN_TYPE (*VarargFn3)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2, const AnyVal& a3, int num_varargs, const AnyVal* varargs); - return reinterpret_cast<VarargFn3>(scalar_fn_)(fn_ctx, *(*input_vals)[0], - *(*input_vals)[1], *(*input_vals)[2], num_varargs, varargs); - case 4: - typedef RETURN_TYPE (*VarargFn4)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2, const AnyVal& a3, const AnyVal& a4, int num_varargs, - const AnyVal* varargs); - return reinterpret_cast<VarargFn4>(scalar_fn_)(fn_ctx, - *(*input_vals)[0], *(*input_vals)[1], *(*input_vals)[2], *(*input_vals)[3], - num_varargs, varargs); - case 5: - typedef RETURN_TYPE (*VarargFn5)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2, const AnyVal& a3, const AnyVal& a4, const AnyVal& a5, - int num_varargs, const AnyVal* varargs); - return reinterpret_cast<VarargFn5>(scalar_fn_)(fn_ctx, - *(*input_vals)[0], *(*input_vals)[1], *(*input_vals)[2], *(*input_vals)[3], - *(*input_vals)[4], num_varargs, varargs); - case 6: - typedef RETURN_TYPE (*VarargFn6)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2, const AnyVal& a3, const AnyVal& a4, const AnyVal& a5, - const AnyVal& a6, int num_varargs, const AnyVal* varargs); - return reinterpret_cast<VarargFn6>(scalar_fn_)(fn_ctx, - *(*input_vals)[0], *(*input_vals)[1], *(*input_vals)[2], *(*input_vals)[3], - *(*input_vals)[4], *(*input_vals)[5], num_varargs, varargs); - case 7: - typedef RETURN_TYPE (*VarargFn7)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2, const AnyVal& a3, const AnyVal& a4, const AnyVal& a5, - const AnyVal& a6, const AnyVal& a7, int num_varargs, const AnyVal* varargs); - return reinterpret_cast<VarargFn7>(scalar_fn_)(fn_ctx, - *(*input_vals)[0], *(*input_vals)[1], *(*input_vals)[2], *(*input_vals)[3], - *(*input_vals)[4], *(*input_vals)[5], *(*input_vals)[6], num_varargs, varargs); - case 8: - typedef RETURN_TYPE (*VarargFn8)(FunctionContext*, const AnyVal& a1, - const AnyVal& a2, const AnyVal& a3, const AnyVal& a4, const AnyVal& a5, - const AnyVal& a6, const AnyVal& a7, const AnyVal& a8, int num_varargs, - const AnyVal* varargs); - return reinterpret_cast<VarargFn8>(scalar_fn_)(fn_ctx, - *(*input_vals)[0], *(*input_vals)[1], *(*input_vals)[2], *(*input_vals)[3], - *(*input_vals)[4], *(*input_vals)[5], *(*input_vals)[6], *(*input_vals)[7], + + // Expands to code snippet like the following for X from 0 to 20: + // case X: + // typedef RETURN_TYPE (*VarargFnX)(FunctionContext*, const AnyVal& a1, ..., + // const AnyVal& aX, int num_varargs, const AnyVal* varargs); + // return reinterpret_cast<VarargFnX>(scalar_fn_)(fn_ctx, *(*input_vals)[0], ..., + // *(*input_vals)[X-1], num_varargs, varargs); +#define SCALAR_VARARG_FN_TYPE(n) BOOST_PP_CAT(VarargFn, n) +#define INTERP_SCALAR_VARARG_FN(z, n, text) \ + case n: \ + typedef RETURN_TYPE (*SCALAR_VARARG_FN_TYPE(n))(ARG_DECL_LIST(n), int, \ + const AnyVal*); \ + return reinterpret_cast<SCALAR_VARARG_FN_TYPE(n)>(scalar_fn_)(ARG_LIST(n), \ num_varargs, varargs); + + BOOST_PP_REPEAT_FROM_TO(0, BOOST_PP_ADD(MAX_INTERP_ARGS, 1), + INTERP_SCALAR_VARARG_FN, unused) + default: - DCHECK(false) << "Interpreted path not implemented. We should have " - << "codegen'd the wrapper"; + DCHECK(false) << "Interpreted path not implemented."; } } return RETURN_TYPE::null(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/scalar-fn-call.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/scalar-fn-call.h b/be/src/exprs/scalar-fn-call.h index 8cc8a19..e48b9df 100644 --- a/be/src/exprs/scalar-fn-call.h +++ b/be/src/exprs/scalar-fn-call.h @@ -60,7 +60,7 @@ class ScalarFnCall: public Expr { ExprContext* context); virtual Status Open(RuntimeState* state, ExprContext* context, FunctionContext::FunctionStateScope scope = FunctionContext::FRAGMENT_LOCAL); - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn); virtual void Close(RuntimeState* state, ExprContext* context, FunctionContext::FunctionStateScope scope = FunctionContext::FRAGMENT_LOCAL); @@ -107,7 +107,7 @@ class ScalarFnCall: public Expr { } /// Loads the native or IR function from HDFS and puts the result in *udf. - Status GetUdf(RuntimeState* state, llvm::Function** udf); + Status GetUdf(LlvmCodeGen* codegen, llvm::Function** udf); /// Loads the native or IR function 'symbol' from HDFS and puts the result in *fn. /// If the function is loaded from an IR module, it cannot be called until the module http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/slot-ref.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/slot-ref.cc b/be/src/exprs/slot-ref.cc index 69f555b..0a766fa 100644 --- a/be/src/exprs/slot-ref.cc +++ b/be/src/exprs/slot-ref.cc @@ -155,7 +155,7 @@ string SlotRef::DebugString() const { // // TODO: We could generate a typed struct (and not a char*) for Tuple for llvm. We know // the types from the TupleDesc. It will likey make this code simpler to reason about. -Status SlotRef::GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn) { +Status SlotRef::GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn) { if (type_.type == TYPE_CHAR) { *fn = NULL; return Status("Codegen for Char not supported."); @@ -166,9 +166,6 @@ Status SlotRef::GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn) { } DCHECK_EQ(GetNumChildren(), 0); - LlvmCodeGen* codegen; - RETURN_IF_ERROR(state->GetCodegen(&codegen)); - // SlotRefs are based on the slot_id and tuple_idx. Combine them to make a // query-wide unique id. We also need to combine whether the tuple is nullable. For // example, in an outer join the scan node could have the same tuple id and slot id http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/slot-ref.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/slot-ref.h b/be/src/exprs/slot-ref.h index d3d8368..6ca3c89 100644 --- a/be/src/exprs/slot-ref.h +++ b/be/src/exprs/slot-ref.h @@ -43,7 +43,7 @@ class SlotRef : public Expr { virtual int GetSlotIds(std::vector<SlotId>* slot_ids) const; const SlotId& slot_id() const { return slot_id_; } - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn); virtual impala_udf::BooleanVal GetBooleanVal(ExprContext* context, const TupleRow*); virtual impala_udf::TinyIntVal GetTinyIntVal(ExprContext* context, const TupleRow*); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/tuple-is-null-predicate.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/tuple-is-null-predicate.cc b/be/src/exprs/tuple-is-null-predicate.cc index dc54273..dd20e8a 100644 --- a/be/src/exprs/tuple-is-null-predicate.cc +++ b/be/src/exprs/tuple-is-null-predicate.cc @@ -59,9 +59,9 @@ Status TupleIsNullPredicate::Prepare(RuntimeState* state, const RowDescriptor& r return Status::OK(); } -Status TupleIsNullPredicate::GetCodegendComputeFn(RuntimeState* state, - llvm::Function** fn) { - return GetCodegendComputeFnWrapper(state, fn); +Status TupleIsNullPredicate::GetCodegendComputeFn(LlvmCodeGen* codegen, + llvm::Function** fn) { + return GetCodegendComputeFnWrapper(codegen, fn); } string TupleIsNullPredicate::DebugString() const { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/exprs/tuple-is-null-predicate.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/tuple-is-null-predicate.h b/be/src/exprs/tuple-is-null-predicate.h index 8c48c3a..e20a195 100644 --- a/be/src/exprs/tuple-is-null-predicate.h +++ b/be/src/exprs/tuple-is-null-predicate.h @@ -38,7 +38,7 @@ class TupleIsNullPredicate: public Predicate { virtual Status Prepare(RuntimeState* state, const RowDescriptor& row_desc, ExprContext* ctx); - virtual Status GetCodegendComputeFn(RuntimeState* state, llvm::Function** fn); + virtual Status GetCodegendComputeFn(LlvmCodeGen* codegen, llvm::Function** fn); virtual std::string DebugString() const; virtual bool IsConstant() const { return false; } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/runtime/plan-fragment-executor.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/plan-fragment-executor.cc b/be/src/runtime/plan-fragment-executor.cc index 5269fe5..a791e61 100644 --- a/be/src/runtime/plan-fragment-executor.cc +++ b/be/src/runtime/plan-fragment-executor.cc @@ -201,10 +201,14 @@ Status PlanFragmentExecutor::PrepareInternal(const TExecPlanFragmentParams& requ scan_node->SetScanRanges(scan_ranges); } + RuntimeState* state = runtime_state_.get(); RuntimeProfile::Counter* prepare_timer = ADD_TIMER(profile(), "ExecTreePrepareTime"); { SCOPED_TIMER(prepare_timer); - RETURN_IF_ERROR(exec_tree_->Prepare(runtime_state_.get())); + // Until IMPALA-4233 is fixed, we still need to create the codegen object before + // Prepare() as ScalarFnCall::Prepare() may codegen. + if (state->codegen_enabled()) RETURN_IF_ERROR(state->CreateCodegen()); + RETURN_IF_ERROR(exec_tree_->Prepare(state)); } PrintVolumeIds(fragment_instance_ctx.per_node_scan_ranges); @@ -232,6 +236,8 @@ Status PlanFragmentExecutor::PrepareInternal(const TExecPlanFragmentParams& requ ReleaseThreadToken(); } + if (state->codegen_enabled()) exec_tree_->Codegen(state); + // set up profile counters profile()->AddChild(exec_tree_->runtime_profile()); rows_produced_counter_ = @@ -246,12 +252,10 @@ Status PlanFragmentExecutor::PrepareInternal(const TExecPlanFragmentParams& requ } void PlanFragmentExecutor::OptimizeLlvmModule() { - if (!runtime_state_->codegen_created()) return; - LlvmCodeGen* codegen; - Status status = runtime_state_->GetCodegen(&codegen, /* initalize */ false); - DCHECK(status.ok()); + if (!runtime_state_->codegen_enabled()) return; + LlvmCodeGen* codegen = runtime_state_->codegen(); DCHECK(codegen != NULL); - status = codegen->FinalizeModule(); + Status status = codegen->FinalizeModule(); if (!status.ok()) { stringstream ss; ss << "Error with codegen for this query: " << status.GetDetail(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/runtime/runtime-state.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/runtime-state.cc b/be/src/runtime/runtime-state.cc index a05b3ef..00ccea1 100644 --- a/be/src/runtime/runtime-state.cc +++ b/be/src/runtime/runtime-state.cc @@ -73,7 +73,6 @@ RuntimeState::RuntimeState( fragment_params_(fragment_params), now_(new TimestampValue( query_ctx().now_string.c_str(), query_ctx().now_string.size())), - codegen_expr_(false), profile_(obj_pool_.get(), "Fragment " + PrintId(fragment_ctx().fragment_instance_id)), is_cancelled_(false), root_node_id_(-1) { @@ -86,7 +85,6 @@ RuntimeState::RuntimeState(const TQueryCtx& query_ctx) now_(new TimestampValue(query_ctx.now_string.c_str(), query_ctx.now_string.size())), exec_env_(ExecEnv::GetInstance()), - codegen_expr_(false), profile_(obj_pool_.get(), "<unnamed>"), is_cancelled_(false), root_node_id_(-1) { @@ -286,12 +284,6 @@ Status RuntimeState::CheckQueryState() { return GetQueryStatus(); } -Status RuntimeState::GetCodegen(LlvmCodeGen** codegen, bool initialize) { - if (codegen_.get() == NULL && initialize) RETURN_IF_ERROR(CreateCodegen()); - *codegen = codegen_.get(); - return Status::OK(); -} - void RuntimeState::AcquireReaderContext(DiskIoRequestContext* reader_context) { boost::lock_guard<SpinLock> l(reader_contexts_lock_); reader_contexts_.push_back(reader_context); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/runtime/runtime-state.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/runtime-state.h b/be/src/runtime/runtime-state.h index 3496d9c..0d612db 100644 --- a/be/src/runtime/runtime-state.h +++ b/be/src/runtime/runtime-state.h @@ -158,9 +158,8 @@ class RuntimeState { /// Returns true if codegen is enabled for this query. bool codegen_enabled() const { return !query_options().disable_codegen; } - /// Returns true if the codegen object has been created. Note that this may return false - /// even when codegen is enabled if nothing has been codegen'd. - bool codegen_created() const { return codegen_.get() != NULL; } + /// Returns the LlvmCodeGen object for this fragment instance. + LlvmCodeGen* codegen() { return codegen_.get(); } /// Takes ownership of a scan node's reader context and plan fragment executor will call /// UnregisterReaderContexts() to unregister it when the fragment is closed. The IO @@ -170,18 +169,6 @@ class RuntimeState { /// Unregisters all reader contexts acquired through AcquireReaderContext(). void UnregisterReaderContexts(); - /// Returns codegen_ in 'codegen'. If 'initialize' is true, codegen_ will be created if - /// it has not been initialized by a previous call already. If 'initialize' is false, - /// 'codegen' will be set to NULL if codegen_ has not been initialized. - Status GetCodegen(LlvmCodeGen** codegen, bool initialize = true); - - /// Returns true if codegen should be used for expr evaluation in this plan fragment. - bool ShouldCodegenExpr() { return codegen_expr_; } - - /// Records that this fragment should use codegen for expr evaluation whenever - /// applicable if codegen is not disabled. - void SetCodegenExpr() { codegen_expr_ = codegen_enabled(); } - BufferedBlockMgr* block_mgr() { DCHECK(block_mgr_.get() != NULL); return block_mgr_.get(); @@ -267,6 +254,9 @@ class RuntimeState { /// execution doesn't continue if the query terminates abnormally. Status CheckQueryState(); + /// Create a codegen object accessible via codegen() if it doesn't exist already. + Status CreateCodegen(); + private: /// Allow TestEnv to set block_mgr manually for testing. friend class TestEnv; @@ -274,10 +264,6 @@ class RuntimeState { /// Set per-fragment state. Status Init(ExecEnv* exec_env); - /// Create a codegen object in codegen_. No-op if it has already been called. This is - /// created on first use. - Status CreateCodegen(); - /// Use a custom block manager for the query for testing purposes. void set_block_mgr(const std::shared_ptr<BufferedBlockMgr>& block_mgr) { block_mgr_ = block_mgr; @@ -306,9 +292,6 @@ class RuntimeState { ExecEnv* exec_env_; boost::scoped_ptr<LlvmCodeGen> codegen_; - /// True if this fragment should force codegen for expr evaluation. - bool codegen_expr_; - /// Thread resource management object for this fragment's execution. The runtime /// state is responsible for returning this pool to the thread mgr. ThreadResourceMgr::ResourcePool* resource_pool_; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/runtime/sorted-run-merger.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/sorted-run-merger.h b/be/src/runtime/sorted-run-merger.h index dade968..e81ca88 100644 --- a/be/src/runtime/sorted-run-merger.h +++ b/be/src/runtime/sorted-run-merger.h @@ -92,7 +92,7 @@ class SortedRunMerger { std::vector<SortedRunWrapper*> min_heap_; /// Row comparator. Returns true if lhs < rhs. - TupleRowComparator comparator_; + const TupleRowComparator& comparator_; /// Descriptor for the rows provided by the input runs. Owned by the exec-node through /// which this merger was created. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/runtime/sorter.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/sorter.cc b/be/src/runtime/sorter.cc index 6757be0..513aba2 100644 --- a/be/src/runtime/sorter.cc +++ b/be/src/runtime/sorter.cc @@ -420,7 +420,7 @@ class Sorter::TupleSorter { const int tuple_size_; /// Tuple comparator with method Less() that returns true if lhs < rhs. - const TupleRowComparator comparator_; + const TupleRowComparator& comparator_; /// Number of times comparator_.Less() can be invoked again before /// comparator_.FreeLocalAllocations() needs to be called. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/runtime/sorter.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/sorter.h b/be/src/runtime/sorter.h index ed23b75..54e2d22 100644 --- a/be/src/runtime/sorter.h +++ b/be/src/runtime/sorter.h @@ -160,7 +160,7 @@ class Sorter { RuntimeState* const state_; /// In memory sorter and less-than comparator. - TupleRowComparator compare_less_than_; + const TupleRowComparator& compare_less_than_; boost::scoped_ptr<TupleSorter> in_mem_tuple_sorter_; /// Block manager object used to allocate, pin and release runs. Not owned by Sorter. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/runtime/tuple.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/tuple.cc b/be/src/runtime/tuple.cc index 656621d..7fa8b2b 100644 --- a/be/src/runtime/tuple.cc +++ b/be/src/runtime/tuple.cc @@ -306,19 +306,17 @@ void Tuple::MaterializeExprs( // ; ----- end CodegenAnyVal::WriteToSlot() ------------------------------------------- // ret void // } -Status Tuple::CodegenMaterializeExprs(RuntimeState* state, bool collect_string_vals, +Status Tuple::CodegenMaterializeExprs(LlvmCodeGen* codegen, bool collect_string_vals, const TupleDescriptor& desc, const vector<ExprContext*>& materialize_expr_ctxs, MemPool* pool, Function** fn) { DCHECK(!collect_string_vals) << "CodegenMaterializeExprs: collect_string_vals NYI"; - LlvmCodeGen* codegen; - RETURN_IF_ERROR(state->GetCodegen(&codegen)); SCOPED_TIMER(codegen->codegen_timer()); LLVMContext& context = codegen->context(); // Codegen each compute function from materialize_expr_ctxs Function* materialize_expr_fns[materialize_expr_ctxs.size()]; for (int i = 0; i < materialize_expr_ctxs.size(); ++i) { - Status status = materialize_expr_ctxs[i]->root()->GetCodegendComputeFn(state, + Status status = materialize_expr_ctxs[i]->root()->GetCodegendComputeFn(codegen, &materialize_expr_fns[i]); if (!status.ok()) { stringstream ss; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/runtime/tuple.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/tuple.h b/be/src/runtime/tuple.h index b95492c..82efc24 100644 --- a/be/src/runtime/tuple.h +++ b/be/src/runtime/tuple.h @@ -167,7 +167,7 @@ class Tuple { /// separate functions for the non-NULL and NULL cases, i.e., the 'pool' argument of the /// generated function is ignored. There are two different MaterializeExprs symbols to /// differentiate these cases when we replace the function calls during codegen. - static Status CodegenMaterializeExprs(RuntimeState* state, bool collect_string_vals, + static Status CodegenMaterializeExprs(LlvmCodeGen* codegen, bool collect_string_vals, const TupleDescriptor& desc, const vector<ExprContext*>& materialize_expr_ctxs, MemPool* pool, llvm::Function** fn); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/service/fe-support.cc ---------------------------------------------------------------------- diff --git a/be/src/service/fe-support.cc b/be/src/service/fe-support.cc index 996770c..31aa488 100644 --- a/be/src/service/fe-support.cc +++ b/be/src/service/fe-support.cc @@ -84,31 +84,45 @@ Java_org_apache_impala_service_FeSupport_NativeEvalConstExprs( DeserializeThriftMsg(env, thrift_expr_batch, &expr_batch); DeserializeThriftMsg(env, thrift_query_ctx_bytes, &query_ctx); - query_ctx.request.query_options.disable_codegen = true; + vector<TExpr>& texprs = expr_batch.exprs; + + // Codegen is almost always disabled in this path. The only exception is when the + // expression contains IR UDF which cannot be interpreted. Enable codegen in this + // case if codegen is not disabled in the query option. Otherwise, we will let it + // fail in ScalarFnCall::Prepare(). + bool need_codegen = false; + for (const TExpr& texpr : texprs) { + if (Expr::NeedCodegen(texpr)) { + need_codegen = true; + break; + } + } + query_ctx.request.query_options.disable_codegen |= !need_codegen; RuntimeState state(query_ctx); + if (!query_ctx.request.query_options.disable_codegen) { + THROW_IF_ERROR_RET( + state.CreateCodegen(), env, JniUtil::internal_exc_class(), result_bytes); + } THROW_IF_ERROR_RET(jni_frame.push(env), env, JniUtil::internal_exc_class(), - result_bytes); + result_bytes); // Exprs can allocate memory so we need to set up the mem trackers before // preparing/running the exprs. state.InitMemTrackers(TUniqueId(), NULL, -1); - vector<TExpr>& texprs = expr_batch.exprs; // Prepare the exprs vector<ExprContext*> expr_ctxs; - for (vector<TExpr>::iterator it = texprs.begin(); it != texprs.end(); it++) { + for (const TExpr& texpr : texprs) { ExprContext* ctx; - THROW_IF_ERROR_RET(Expr::CreateExprTree(&obj_pool, *it, &ctx), env, - JniUtil::internal_exc_class(), result_bytes); + THROW_IF_ERROR_RET(Expr::CreateExprTree(&obj_pool, texpr, &ctx), env, + JniUtil::internal_exc_class(), result_bytes); THROW_IF_ERROR_RET(ctx->Prepare(&state, RowDescriptor(), state.query_mem_tracker()), - env, JniUtil::internal_exc_class(), result_bytes); + env, JniUtil::internal_exc_class(), result_bytes); expr_ctxs.push_back(ctx); } - if (state.codegen_created()) { - // Finalize the module so any UDF functions are jit'd - LlvmCodeGen* codegen = NULL; - state.GetCodegen(&codegen, /* initialize */ false); + if (!query_ctx.request.query_options.disable_codegen) { + LlvmCodeGen* codegen = state.codegen(); DCHECK(codegen != NULL); codegen->EnableOptimizations(false); codegen->FinalizeModule(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/service/query-options.cc ---------------------------------------------------------------------- diff --git a/be/src/service/query-options.cc b/be/src/service/query-options.cc index 0823981..eb1d2f9 100644 --- a/be/src/service/query-options.cc +++ b/be/src/service/query-options.cc @@ -304,6 +304,7 @@ Status impala::SetQueryOption(const string& key, const string& value, query_options->__set_schedule_random_replica( iequals(value, "true") || iequals(value, "1")); break; + // TODO: remove this query option (IMPALA-4319). case TImpalaQueryOptions::SCAN_NODE_CODEGEN_THRESHOLD: query_options->__set_scan_node_codegen_threshold(atol(value.c_str())); break; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/testutil/test-udfs.cc ---------------------------------------------------------------------- diff --git a/be/src/testutil/test-udfs.cc b/be/src/testutil/test-udfs.cc index fa86a4a..efdfa0a 100644 --- a/be/src/testutil/test-udfs.cc +++ b/be/src/testutil/test-udfs.cc @@ -333,3 +333,32 @@ IntVal EightArgs(FunctionContext* context, const IntVal& v1, const IntVal& v2, const IntVal& v7, const IntVal& v8) { return IntVal(v1.val + v2.val + v3.val + v4.val + v5.val + v6.val + v7.val + v8.val); } + +IntVal NineArgs(FunctionContext* context, const IntVal& v1, const IntVal& v2, + const IntVal& v3, const IntVal& v4, const IntVal& v5, const IntVal& v6, + const IntVal& v7, const IntVal& v8, const IntVal& v9) { + return IntVal(v1.val + v2.val + v3.val + v4.val + v5.val + v6.val + v7.val + v8.val + + v9.val); +} + +IntVal TwentyArgs(FunctionContext* context, const IntVal& v1, const IntVal& v2, + const IntVal& v3, const IntVal& v4, const IntVal& v5, const IntVal& v6, + const IntVal& v7, const IntVal& v8, const IntVal& v9, const IntVal& v10, + const IntVal& v11, const IntVal& v12, const IntVal& v13, const IntVal& v14, + const IntVal& v15, const IntVal& v16, const IntVal& v17, const IntVal& v18, + const IntVal& v19, const IntVal& v20) { + return IntVal(v1.val + v2.val + v3.val + v4.val + v5.val + v6.val + v7.val + v8.val + + v9.val + v10.val + v11.val + v12.val + v13.val + v14.val + v15.val + v16.val + + v17.val + v18.val + v19.val + v20.val); +} + +IntVal TwentyOneArgs(FunctionContext* context, const IntVal& v1, const IntVal& v2, + const IntVal& v3, const IntVal& v4, const IntVal& v5, const IntVal& v6, + const IntVal& v7, const IntVal& v8, const IntVal& v9, const IntVal& v10, + const IntVal& v11, const IntVal& v12, const IntVal& v13, const IntVal& v14, + const IntVal& v15, const IntVal& v16, const IntVal& v17, const IntVal& v18, + const IntVal& v19, const IntVal& v20, const IntVal& v21) { + return IntVal(v1.val + v2.val + v3.val + v4.val + v5.val + v6.val + v7.val + v8.val + + v9.val + v10.val + v11.val + v12.val + v13.val + v14.val + v15.val + v16.val + + v17.val + v18.val + v19.val + v20.val + v21.val); +} http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/util/tuple-row-compare.cc ---------------------------------------------------------------------- diff --git a/be/src/util/tuple-row-compare.cc b/be/src/util/tuple-row-compare.cc index 6f68b9e..69892ae 100644 --- a/be/src/util/tuple-row-compare.cc +++ b/be/src/util/tuple-row-compare.cc @@ -51,10 +51,9 @@ int TupleRowComparator::CompareInterpreted( Status TupleRowComparator::Codegen(RuntimeState* state) { Function* fn; - RETURN_IF_ERROR(CodegenCompare(state, &fn)); - LlvmCodeGen* codegen; - bool got_codegen = state->GetCodegen(&codegen).ok(); - DCHECK(got_codegen); + LlvmCodeGen* codegen = state->codegen(); + DCHECK(codegen != NULL); + RETURN_IF_ERROR(CodegenCompare(codegen, &fn)); codegend_compare_fn_ = state->obj_pool()->Add(new CompareFn); codegen->AddFunctionToJit(fn, reinterpret_cast<void**>(codegend_compare_fn_)); return Status::OK(); @@ -175,9 +174,7 @@ Status TupleRowComparator::Codegen(RuntimeState* state) { // next_key2: ; preds = %rhs_non_null12, %next_key // ret i32 0 // } -Status TupleRowComparator::CodegenCompare(RuntimeState* state, Function** fn) { - LlvmCodeGen* codegen; - RETURN_IF_ERROR(state->GetCodegen(&codegen)); +Status TupleRowComparator::CodegenCompare(LlvmCodeGen* codegen, Function** fn) { SCOPED_TIMER(codegen->codegen_timer()); LLVMContext& context = codegen->context(); @@ -188,7 +185,7 @@ Status TupleRowComparator::CodegenCompare(RuntimeState* state, Function** fn) { Function* key_fns[key_expr_ctxs_lhs_.size()]; for (int i = 0; i < key_expr_ctxs_lhs_.size(); ++i) { Status status = - key_expr_ctxs_lhs_[i]->root()->GetCodegendComputeFn(state, &key_fns[i]); + key_expr_ctxs_lhs_[i]->root()->GetCodegendComputeFn(codegen, &key_fns[i]); if (!status.ok()) { return Status(Substitute("Could not codegen TupleRowComparator::Compare(): $0", status.GetDetail())); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/be/src/util/tuple-row-compare.h ---------------------------------------------------------------------- diff --git a/be/src/util/tuple-row-compare.h b/be/src/util/tuple-row-compare.h index a1d8d72..e2beed1 100644 --- a/be/src/util/tuple-row-compare.h +++ b/be/src/util/tuple-row-compare.h @@ -127,7 +127,7 @@ class TupleRowComparator { /// Codegen Compare(). Returns a non-OK status if codegen is unsuccessful. /// TODO: inline this at codegen'd callsites instead of indirectly calling via function /// pointer. - Status CodegenCompare(RuntimeState* state, llvm::Function** fn); + Status CodegenCompare(LlvmCodeGen* codegen, llvm::Function** fn); /// References to ExprContexts managed by SortExecExprs. The lhs ExprContexts must /// be created and prepared before the TupleRowCompator is constructed, but the rhs http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/common/thrift/PlanNodes.thrift ---------------------------------------------------------------------- diff --git a/common/thrift/PlanNodes.thrift b/common/thrift/PlanNodes.thrift index 49fcfbb..6f863b2 100644 --- a/common/thrift/PlanNodes.thrift +++ b/common/thrift/PlanNodes.thrift @@ -196,17 +196,14 @@ struct THdfsScanNode { // Option to control tie breaks during scan scheduling. 4: optional bool random_replica - // Option to control whether codegen should be used for conjuncts evaluation. - 5: optional bool codegen_conjuncts - // Number of header lines to skip at the beginning of each file of this table. Only set // for hdfs text files. - 6: optional i32 skip_header_line_count + 5: optional i32 skip_header_line_count // Indicates whether the MT scan node implementation should be used. // If this is true then the MT_DOP query option must be > 0. // TODO: Remove this option when the MT scan node supports all file formats. - 7: optional bool use_mt_scan_node + 6: optional bool use_mt_scan_node } struct TDataSourceScanNode { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java b/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java index 3d52aa4..95d86e3 100644 --- a/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java +++ b/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java @@ -105,9 +105,6 @@ public class HdfsScanNode extends ScanNode { // Total number of bytes from partitions_ private long totalBytes_ = 0; - // True if this scan node should use codegen for evaluting conjuncts. - private boolean codegenConjuncts_; - // True if this scan node should use the MT implementation in the backend. private boolean useMtScanNode_; @@ -188,9 +185,6 @@ public class HdfsScanNode extends ScanNode { // TODO: do we need this? assignedConjuncts_ = analyzer.getAssignedConjuncts(); - - // Decide whether codegen should be used for evaluating conjuncts. - checkForCodegen(analyzer); } /** @@ -505,39 +499,6 @@ public class HdfsScanNode extends ScanNode { " clusterNodes=" + cluster.numNodes()); } - /** - * Approximate the cost of evaluating all conjuncts bound by this node by - * aggregating total number of nodes in expression trees of all conjuncts. - */ - private int computeConjunctsCost() { - int cost = 0; - for (Expr expr: getConjuncts()) { - cost += expr.numNodes(); - } - for (List<Expr> exprs: collectionConjuncts_.values()) { - for (Expr expr: exprs) { - cost += expr.numNodes(); - } - } - return cost; - } - - /** - * Scan node is not a codegen-enabled operator. Decide whether to use codegen for - * conjuncts evaluation by estimating the cost of interpretation. - */ - private void checkForCodegen(Analyzer analyzer) { - long conjunctsCost = computeConjunctsCost(); - long inputCardinality = getInputCardinality(); - long threshold = - analyzer.getQueryCtx().getRequest().query_options.scan_node_codegen_threshold; - if (inputCardinality == -1) { - codegenConjuncts_ = conjunctsCost > 0; - } else { - codegenConjuncts_ = inputCardinality * conjunctsCost > threshold; - } - } - @Override protected void toThrift(TPlanNode msg) { msg.hdfs_scan_node = new THdfsScanNode(desc_.getId().asInt()); @@ -546,7 +507,6 @@ public class HdfsScanNode extends ScanNode { } msg.hdfs_scan_node.setRandom_replica(randomReplica_); msg.node_type = TPlanNodeType.HDFS_SCAN_NODE; - msg.hdfs_scan_node.setCodegen_conjuncts(codegenConjuncts_); if (!collectionConjuncts_.isEmpty()) { Map<Integer, List<TExpr>> tcollectionConjuncts = Maps.newLinkedHashMap(); for (Map.Entry<TupleDescriptor, List<Expr>> entry: http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/testdata/workloads/functional-query/queries/QueryTest/udf-errors.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-query/queries/QueryTest/udf-errors.test b/testdata/workloads/functional-query/queries/QueryTest/udf-errors.test index f3ff3f9..948d584 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/udf-errors.test +++ b/testdata/workloads/functional-query/queries/QueryTest/udf-errors.test @@ -24,12 +24,60 @@ symbol='FnDoesNotExist'; Could not load binary: $FILESYSTEM_PREFIX/test-warehouse/not-a-real-file.so ==== ---- QUERY +# This test is run with codegen disabled. Interpretation only handles up to 20 arguments. +create function if not exists udf_test_errors.twenty_args(int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int) returns int +location '$FILESYSTEM_PREFIX/test-warehouse/libTestUdfs.so' +symbol='TwentyArgs'; +---- RESULTS +==== +---- QUERY +# Verifies that interpretation can support up to 20 arguments +select udf_test_errors.twenty_args(1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20); +---- TYPES +INT +---- RESULTS +210 +==== +---- QUERY +# This test is run with codegen disabled. Interpretation only handles up to 20 arguments. +create function if not exists udf_test_errors.twenty_one_args(int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int, int, int) returns int +location '$FILESYSTEM_PREFIX/test-warehouse/libTestUdfs.so' +symbol='TwentyOneArgs'; +---- RESULTS +==== +---- QUERY +# Verifies that interpretation fails with more than 20 arguments. +select udf_test_errors.twenty_one_args(1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21); +---- CATCH +Cannot interpret native UDF 'twenty_one_args': number of arguments is more than 20. Codegen is needed. Please set DISABLE_CODEGEN to false. +==== +---- QUERY +# This test is run with codegen disabled. IR UDF will fail. +create function if not exists udf_test_errors.nine_args_ir(int, int, int, int, int, int, + int, int, int) returns int +location '$FILESYSTEM_PREFIX/test-warehouse/test-udfs.ll' +symbol='NineArgs'; +---- RESULTS +==== +---- QUERY +select udf_test_errors.nine_args_ir(1,2,3,4,5,6,7,8,9); +---- CATCH +Cannot interpret LLVM IR UDF 'nine_args_ir': Codegen is needed. Please set DISABLE_CODEGEN to false. +==== +---- QUERY drop database udf_test_errors; ---- CATCH Cannot drop non-empty database: udf_test_errors ==== ---- QUERY drop function udf_test_errors.hive_pi(); +drop function udf_test_errors.twenty_args(int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int); +drop function udf_test_errors.twenty_one_args(int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int); +drop function udf_test_errors.nine_args_ir(int, int, int, int, int, int, int, int, int); drop database udf_test_errors; ---- RESULTS ==== http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/testdata/workloads/functional-query/queries/QueryTest/udf.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-query/queries/QueryTest/udf.test b/testdata/workloads/functional-query/queries/QueryTest/udf.test index 645c321..5cbbecb 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/udf.test +++ b/testdata/workloads/functional-query/queries/QueryTest/udf.test @@ -529,3 +529,17 @@ INT ---- RESULTS 36 ==== +---- QUERY +select twenty_args(1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20); +---- TYPES +INT +---- RESULTS +210 +==== +---- QUERY +select twenty_one_args(1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21); +---- TYPES +INT +---- RESULTS +231 +==== http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b15d992a/tests/query_test/test_udfs.py ---------------------------------------------------------------------- diff --git a/tests/query_test/test_udfs.py b/tests/query_test/test_udfs.py index 02bdf4f..2658351 100644 --- a/tests/query_test/test_udfs.py +++ b/tests/query_test/test_udfs.py @@ -73,6 +73,11 @@ class TestUdfs(ImpalaTestSuite): self.run_test_case('QueryTest/udf-init-close', vector, use_db=database) def test_udf_errors(self, vector): + # Disable codegen to force interpretation path to be taken. + # Aim to exercise two failure cases: + # 1. too many arguments + # 2. IR UDF + vector.get_value('exec_option')['disable_codegen'] = 1 self.run_test_case('QueryTest/udf-errors', vector) def test_udf_invalid_symbol(self, vector): @@ -371,6 +376,11 @@ drop function if exists {database}.five_args(int, int, int, int, int); drop function if exists {database}.six_args(int, int, int, int, int, int); drop function if exists {database}.seven_args(int, int, int, int, int, int, int); drop function if exists {database}.eight_args(int, int, int, int, int, int, int, int); +drop function if exists {database}.nine_args(int, int, int, int, int, int, int, int, int); +drop function if exists {database}.twenty_args(int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int); +drop function if exists {database}.twenty_one_args(int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int, int, int); create database if not exists {database}; @@ -493,4 +503,12 @@ location '{location}' symbol='SevenArgs'; create function {database}.eight_args(int, int, int, int, int, int, int, int) returns int location '{location}' symbol='EightArgs'; + +create function {database}.twenty_args(int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int) returns int +location '{location}' symbol='TwentyArgs'; + +create function {database}.twenty_one_args(int, int, int, int, int, int, int, int, int, int, + int, int, int, int, int, int, int, int, int, int, int) returns int +location '{location}' symbol='TwentyOneArgs'; """
