Decide what to generate for ExecEvalExpr based on PlanState

  * Skip generating ExecEvalExpr ScanNodes when ProjInfo is a Var list
  * Skip generating SlotGetAttr in AggNodes

src/backend/codegen/codegen_wrapper.cc

@@ -146,9 +146,13 @@ void* ExecEvalExprCodegenEnroll(
     ExecEvalExprFn* ptr_to_chosen_func_ptr,
     ExprState *exprstate,
     ExprContext *econtext,
-    TupleTableSlot* slot) {
+    PlanState* plan_state) {
   ExecEvalExprCodegen* generator = CodegenEnroll<ExecEvalExprCodegen>(
-      regular_func_ptr, ptr_to_chosen_func_ptr, exprstate, econtext, slot);
+      regular_func_ptr,
+      ptr_to_chosen_func_ptr,
+      exprstate,
+      econtext,
+      plan_state);
   return generator;
 }
 

src/backend/codegen/exec_eval_expr_codegen.cc

@@ -54,14 +54,48 @@ ExecEvalExprCodegen::ExecEvalExprCodegen
     ExecEvalExprFn* ptr_to_regular_func_ptr,
     ExprState *exprstate,
     ExprContext *econtext,
-    TupleTableSlot* slot) :
+    PlanState* plan_state) :
     BaseCodegen(kExecEvalExprPrefix,
                 regular_func_ptr, ptr_to_regular_func_ptr),
                 exprstate_(exprstate),
                 econtext_(econtext),
-                slot_(slot) {
+                plan_state_(plan_state) {
 }
 
+void ExecEvalExprCodegen::PrepareSlotGetAttr(
+    gpcodegen::GpCodegenUtils* codegen_utils, ExprTreeGeneratorInfo* gen_info) {
+  assert(nullptr != plan_state_);
+  switch (nodeTag(plan_state_)) {
+    case T_SeqScanState:
+    case T_TableScanState:
+      // Generate dependent slot_getattr() implementation for the given slot
+      if (gen_info->max_attr > 0) {
+        TupleTableSlot* slot = reinterpret_cast<ScanState*>(plan_state_)
+            ->ss_ScanTupleSlot;
+        assert(nullptr != slot);
+        std::string slot_getattr_func_name = "slot_getattr_"
+            + std::to_string(reinterpret_cast<uint64_t>(slot)) + "_"
+            + std::to_string(gen_info->max_attr);
+        bool ok = SlotGetAttrCodegen::GenerateSlotGetAttr(
+            codegen_utils, slot_getattr_func_name, slot, gen_info->max_attr,
+            &gen_info->llvm_slot_getattr_func);
+        if (!ok) {
+          elog(DEBUG1, "slot_getattr generation for ExecEvalExpr failed!");
+        }
+      }
+      break;
+    case T_AggState:
+      // For now, we assume that tuples for the Aggs are already going to be
+      // deformed in which case, we can avoid generating and calling the
+      // generated slot_getattr(). This may not be true always, but calling the
+      // regular slot_getattr() will still preserve correctness.
+      gen_info->llvm_slot_getattr_func = nullptr;
+      break;
+    default:
+      elog(DEBUG1,
+          "Attempting to generate ExecEvalExpr for an unsupported operator!");
+  }
+}
 
 bool ExecEvalExprCodegen::GenerateExecEvalExpr(
     gpcodegen::GpCodegenUtils* codegen_utils) {
@@ -104,24 +138,8 @@ bool ExecEvalExprCodegen::GenerateExecEvalExpr(
     return false;
   }
 
-  // Generate dependant slot_getattr() implementation for the given slot
-  if (gen_info.max_attr > 0) {
-    assert(nullptr != slot_);
-
-    std::string slot_getattr_func_name = "slot_getattr_" +
-        std::to_string(reinterpret_cast<uint64_t>(slot_)) + "_" +
-        std::to_string(gen_info.max_attr);
-
-    bool ok = SlotGetAttrCodegen::GenerateSlotGetAttr(
-        codegen_utils,
-        slot_getattr_func_name,
-        slot_,
-        gen_info.max_attr,
-        &gen_info.llvm_slot_getattr_func);
-    if (!ok) {
-      elog(DEBUG1, "slot_getattr generation for ExecEvalExpr failed!");
-    }
-  }
+  // Prepare dependent slot_getattr() generation
+  PrepareSlotGetAttr(codegen_utils, &gen_info);
 
   // In case the generation above failed either or it was not needed,
   // we revert to use the external slot_getattr()

src/backend/codegen/include/codegen/exec_eval_expr_codegen.h

@@ -15,6 +15,7 @@
 
 #include "codegen/codegen_wrapper.h"
 #include "codegen/base_codegen.h"
+#include "codegen/expr_tree_generator.h"
 
 namespace gpcodegen {
 
@@ -42,7 +43,7 @@ class ExecEvalExprCodegen: public BaseCodegen<ExecEvalExprFn> {
                                ExecEvalExprFn* ptr_to_regular_func_ptr,
                                ExprState *exprstate,
                                ExprContext *econtext,
-                               TupleTableSlot* slot);
+                               PlanState* plan_state);
 
   virtual ~ExecEvalExprCodegen() = default;
 
@@ -70,7 +71,7 @@ class ExecEvalExprCodegen: public BaseCodegen<ExecEvalExprFn> {
  private:
   ExprState *exprstate_;
   ExprContext *econtext_;
-  TupleTableSlot* slot_;
+  PlanState* plan_state_;
 
   static constexpr char kExecEvalExprPrefix[] = "ExecEvalExpr";
 
@@ -81,6 +82,13 @@ class ExecEvalExprCodegen: public BaseCodegen<ExecEvalExprFn> {
    * @return true on successful generation.
    **/
   bool GenerateExecEvalExpr(gpcodegen::GpCodegenUtils* codegen_utils);
+
+  /**
+   * @brief Prepare generation of dependent slot_getattr() if necessary
+   * @return true on successful generation.
+   **/
+  void PrepareSlotGetAttr(gpcodegen::GpCodegenUtils* codegen_utils,
+                          ExprTreeGeneratorInfo* gen_info);
 };
 
 /** @} */

src/backend/executor/execProcnode.c

@@ -163,10 +163,10 @@ static void ExecCdbTraceNode(PlanState *node, bool entry, TupleTableSlot *result
   			        int flags);
 
  static void
- EnrollQualList(PlanState* result, TupleTableSlot* slot);
+ EnrollQualList(PlanState* result);
 
  static void
- EnrollProjInfoTargetList(ProjectionInfo* ProjInfo, TupleTableSlot* slot);
+ EnrollProjInfoTargetList(PlanState* result, ProjectionInfo* ProjInfo);
 
 /*
  * setSubplanSliceId
@@ -346,10 +346,10 @@ ExecInitNode(Plan *node, EState *estate, int eflags)
 			 * Enroll targetlist & quals' expression evaluation functions
 			 * in codegen_manager
 			 */
-			EnrollQualList(result, ((ScanState*) result)->ss_ScanTupleSlot);
+			EnrollQualList(result);
 			if (NULL !=result)
 			{
-			  EnrollProjInfoTargetList(result->ps_ProjInfo, ((ScanState*) result)->ss_ScanTupleSlot);
+			  EnrollProjInfoTargetList(result, result->ps_ProjInfo);
 			}
 			}
 			END_MEMORY_ACCOUNT();
@@ -580,14 +580,14 @@ ExecInitNode(Plan *node, EState *estate, int eflags)
 			 * Enroll targetlist & quals' expression evaluation functions
 			 * in codegen_manager
 			 */
-			EnrollQualList(result, ((ScanState*) result)->ss_ScanTupleSlot);
+			EnrollQualList(result);
 			if (NULL != result)
 			{
 			  AggState* aggstate = (AggState*)result;
 			  for (int aggno = 0; aggno < aggstate->numaggs; aggno++)
 			  {
 			    AggStatePerAgg peraggstate = &aggstate->peragg[aggno];
-			    EnrollProjInfoTargetList(peraggstate->evalproj, ((ScanState*) result)->ss_ScanTupleSlot);
+			    EnrollProjInfoTargetList(result, peraggstate->evalproj);
 			  }
 			}
 			}
@@ -781,7 +781,7 @@ ExecInitNode(Plan *node, EState *estate, int eflags)
  * ----------------------------------------------------------------
  */
 void
-EnrollQualList(PlanState* result, TupleTableSlot* slot)
+EnrollQualList(PlanState* result)
 {
 #ifdef USE_CODEGEN
 	if (NULL == result ||
@@ -798,7 +798,8 @@ EnrollQualList(PlanState* result, TupleTableSlot* slot)
 	                              &exprstate->evalfunc,
 	                              exprstate,
 	                              result->ps_ExprContext,
-	                              slot);
+	                              result
+	                              );
 	}
 
 #endif
@@ -811,7 +812,7 @@ EnrollQualList(PlanState* result, TupleTableSlot* slot)
  * ----------------------------------------------------------------
  */
 void
-EnrollProjInfoTargetList(ProjectionInfo* ProjInfo, TupleTableSlot* slot)
+EnrollProjInfoTargetList(PlanState* result, ProjectionInfo* ProjInfo)
 {
 #ifdef USE_CODEGEN
   if (NULL == ProjInfo ||
@@ -819,6 +820,15 @@ EnrollProjInfoTargetList(ProjectionInfo* ProjInfo, TupleTableSlot* slot)
   {
     return;
   }
+  if (ProjInfo->pi_isVarList) {
+    /*
+     * Skip generating expression evaluation for VAR elements in the target
+     * list since ExecVariableList will take of that
+     * TODO(shardikar) Re-evaluate this condition once we codegen ExecTargetList
+     */
+    return;
+  }
+
   ListCell *l;
   foreach(l, ProjInfo->pi_targetlist)
   {
@@ -831,8 +841,7 @@ EnrollProjInfoTargetList(ProjectionInfo* ProjInfo, TupleTableSlot* slot)
                                 &gstate->arg->evalfunc,
                                 gstate->arg,
                                 ProjInfo->pi_exprContext,
-                                slot);
-
+                                result);
   }
 #endif
 }

src/include/codegen/codegen_wrapper.h

@@ -28,6 +28,7 @@ struct TupleTableSlot;
 struct ProjectionInfo;
 struct ExprContext;
 struct ExprState;
+struct PlanState;
 
 /*
  * Enum used to mimic ExprDoneCond in ExecEvalExpr function pointer.
@@ -171,7 +172,7 @@ ExecEvalExprCodegenEnroll(ExecEvalExprFn regular_func_ptr,
                           ExecEvalExprFn* ptr_to_regular_func_ptr,
                           struct ExprState *exprstate,
                           struct ExprContext *econtext,
-                          struct TupleTableSlot* slot);
+                          struct PlanState* plan_state);
 
 #ifdef __cplusplus
 }  // extern "C"
@@ -232,9 +233,9 @@ ExecEvalExprCodegenEnroll(ExecEvalExprFn regular_func_ptr,
 				regular_func, ptr_to_regular_func_ptr, proj_info, slot); \
 		Assert(proj_info->ExecVariableList_gen_info.ExecVariableList_fn == regular_func); \
 
-#define enroll_ExecEvalExpr_codegen(regular_func, ptr_to_regular_func_ptr, exprstate, econtext, slot) \
+#define enroll_ExecEvalExpr_codegen(regular_func, ptr_to_regular_func_ptr, exprstate, econtext, plan_state) \
 		exprstate->ExecEvalExpr_code_generator = ExecEvalExprCodegenEnroll( \
-        (ExecEvalExprFn)regular_func, (ExecEvalExprFn*)ptr_to_regular_func_ptr, exprstate, econtext, slot); \
+        (ExecEvalExprFn)regular_func, (ExecEvalExprFn*)ptr_to_regular_func_ptr, exprstate, econtext, plan_state); \
         Assert(exprstate->evalfunc == regular_func); \
 
 #endif //USE_CODEGEN

src/test/unit/mock/gpcodegen_mock.c

@@ -103,7 +103,7 @@ ExecEvalExprCodegenEnroll(ExecEvalExprFn regular_func_ptr,
                           ExecEvalExprFn* ptr_to_regular_func_ptr,
                           struct ExprState *exprstate,
                           struct ExprContext *econtext,
-                          struct TupleTableSlot* slot)
+                          struct PlanState* planstate)
 {
   *ptr_to_regular_func_ptr = regular_func_ptr;
    elog(ERROR, "mock implementation of ExecEvalExprCodegenEnroll called");