Extend ExecBuildAggTrans() to support a NULL pointer check.

Optionally push a step to check for a NULL pointer to the pergroup
state.

This will be important for disk-based hash aggregation in combination
with grouping sets. When memory limits are reached, a given tuple may
find its per-group state for some grouping sets but not others. For
the former, it advances the per-group state as normal; for the latter,
it skips evaluation and the calling code will have to spill the tuple
and reprocess it in a later batch.

Add the NULL check as a separate expression step because in some
common cases it's not needed.

Discussion: https://postgr.es/m/20200221202212.ssb2qpmdgrnx52sj%40alap3.anarazel.de

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 91aa386fa61e7cfc84810bc80eb27698aad5f783..8c5ead93d680baef80ecc0fdbffd7d78a99a4f03 100644 (file)
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -79,7 +79,8 @@ static void ExecInitCoerceToDomain(ExprEvalStep *scratch, CoerceToDomain *ctest,
 static void ExecBuildAggTransCall(ExprState *state, AggState *aggstate,
                                                                  ExprEvalStep *scratch,
                                                                  FunctionCallInfo fcinfo, AggStatePerTrans pertrans,
-                                                                 int transno, int setno, int setoff, bool ishash);
+                                                                 int transno, int setno, int setoff, bool ishash,
+                                                                 bool nullcheck);
 
 
 /*
@@ -2924,10 +2925,13 @@ ExecInitCoerceToDomain(ExprEvalStep *scratch, CoerceToDomain *ctest,
  * check for filters, evaluate aggregate input, check that that input is not
  * NULL for a strict transition function, and then finally invoke the
  * transition for each of the concurrently computed grouping sets.
+ *
+ * If nullcheck is true, the generated code will check for a NULL pointer to
+ * the array of AggStatePerGroup, and skip evaluation if so.
  */
 ExprState *
 ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
-                                 bool doSort, bool doHash)
+                                 bool doSort, bool doHash, bool nullcheck)
 {
        ExprState  *state = makeNode(ExprState);
        PlanState  *parent = &aggstate->ss.ps;
@@ -3158,7 +3162,8 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
                        for (int setno = 0; setno < processGroupingSets; setno++)
                        {
                                ExecBuildAggTransCall(state, aggstate, &scratch, trans_fcinfo,
-                                                                         pertrans, transno, setno, setoff, false);
+                                                                         pertrans, transno, setno, setoff, false,
+                                                                         nullcheck);
                                setoff++;
                        }
                }
@@ -3177,7 +3182,8 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
                        for (int setno = 0; setno < numHashes; setno++)
                        {
                                ExecBuildAggTransCall(state, aggstate, &scratch, trans_fcinfo,
-                                                                         pertrans, transno, setno, setoff, true);
+                                                                         pertrans, transno, setno, setoff, true,
+                                                                         nullcheck);
                                setoff++;
                        }
                }
@@ -3227,15 +3233,28 @@ static void
 ExecBuildAggTransCall(ExprState *state, AggState *aggstate,
                                          ExprEvalStep *scratch,
                                          FunctionCallInfo fcinfo, AggStatePerTrans pertrans,
-                                         int transno, int setno, int setoff, bool ishash)
+                                         int transno, int setno, int setoff, bool ishash,
+                                         bool nullcheck)
 {
        ExprContext *aggcontext;
+       int adjust_jumpnull = -1;
 
        if (ishash)
                aggcontext = aggstate->hashcontext;
        else
                aggcontext = aggstate->aggcontexts[setno];
 
+       /* add check for NULL pointer? */
+       if (nullcheck)
+       {
+               scratch->opcode = EEOP_AGG_PLAIN_PERGROUP_NULLCHECK;
+               scratch->d.agg_plain_pergroup_nullcheck.setoff = setoff;
+               /* adjust later */
+               scratch->d.agg_plain_pergroup_nullcheck.jumpnull = -1;
+               ExprEvalPushStep(state, scratch);
+               adjust_jumpnull = state->steps_len - 1;
+       }
+
        /*
         * Determine appropriate transition implementation.
         *
@@ -3303,6 +3322,16 @@ ExecBuildAggTransCall(ExprState *state, AggState *aggstate,
        scratch->d.agg_trans.transno = transno;
        scratch->d.agg_trans.aggcontext = aggcontext;
        ExprEvalPushStep(state, scratch);
+
+       /* fix up jumpnull */
+       if (adjust_jumpnull != -1)
+       {
+               ExprEvalStep *as = &state->steps[adjust_jumpnull];
+
+               Assert(as->opcode == EEOP_AGG_PLAIN_PERGROUP_NULLCHECK);
+               Assert(as->d.agg_plain_pergroup_nullcheck.jumpnull == -1);
+               as->d.agg_plain_pergroup_nullcheck.jumpnull = state->steps_len;
+       }
 }
 
 /*

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index eafd484900206637f47171791d3c52bdb53d3022..113ed1547cbc941010b6fe0b2429ecc79d963f38 100644 (file)
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -435,6 +435,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
                &&CASE_EEOP_AGG_DESERIALIZE,
                &&CASE_EEOP_AGG_STRICT_INPUT_CHECK_ARGS,
                &&CASE_EEOP_AGG_STRICT_INPUT_CHECK_NULLS,
+               &&CASE_EEOP_AGG_PLAIN_PERGROUP_NULLCHECK,
                &&CASE_EEOP_AGG_PLAIN_TRANS_INIT_STRICT_BYVAL,
                &&CASE_EEOP_AGG_PLAIN_TRANS_STRICT_BYVAL,
                &&CASE_EEOP_AGG_PLAIN_TRANS_BYVAL,
@@ -1603,6 +1604,22 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
                        EEO_NEXT();
                }
 
+               /*
+                * Check for a NULL pointer to the per-group states.
+                */
+
+               EEO_CASE(EEOP_AGG_PLAIN_PERGROUP_NULLCHECK)
+               {
+                       AggState   *aggstate = castNode(AggState, state->parent);
+                       AggStatePerGroup pergroup_allaggs = aggstate->all_pergroups
+                               [op->d.agg_plain_pergroup_nullcheck.setoff];
+
+                       if (pergroup_allaggs == NULL)
+                               EEO_JUMP(op->d.agg_plain_pergroup_nullcheck.jumpnull);
+
+                       EEO_NEXT();
+               }
+
                /*
                 * Different types of aggregate transition functions are implemented
                 * as different types of steps, to avoid incurring unnecessary

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 13c21ffe9a37af127c0061527548959cdcf6e4c1..7aebb247d8895ed56acef5d9f0a71d1ca2f19d3e 100644 (file)
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -2928,7 +2928,8 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
                else
                        Assert(false);
 
-               phase->evaltrans = ExecBuildAggTrans(aggstate, phase, dosort, dohash);
+               phase->evaltrans = ExecBuildAggTrans(aggstate, phase, dosort, dohash,
+                                                                                        false);
 
        }
 

diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index dc16b3993275b316bae83612370a9b7d4d10623a..b855e739571097d797c42e18be3bdc964a1b3fdd 100644 (file)
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -2046,6 +2046,45 @@ llvm_compile_expr(ExprState *state)
                                        break;
                                }
 
+                       case EEOP_AGG_PLAIN_PERGROUP_NULLCHECK:
+                               {
+                                       int                              jumpnull;
+                                       LLVMValueRef     v_aggstatep;
+                                       LLVMValueRef     v_allpergroupsp;
+                                       LLVMValueRef     v_pergroup_allaggs;
+                                       LLVMValueRef     v_setoff;
+
+                                       jumpnull = op->d.agg_plain_pergroup_nullcheck.jumpnull;
+
+                                       /*
+                                        * pergroup_allaggs = aggstate->all_pergroups
+                                        * [op->d.agg_plain_pergroup_nullcheck.setoff];
+                                        */
+                                       v_aggstatep = LLVMBuildBitCast(
+                                               b, v_parent, l_ptr(StructAggState), "");
+
+                                       v_allpergroupsp = l_load_struct_gep(
+                                               b, v_aggstatep,
+                                               FIELDNO_AGGSTATE_ALL_PERGROUPS,
+                                               "aggstate.all_pergroups");
+
+                                       v_setoff = l_int32_const(
+                                               op->d.agg_plain_pergroup_nullcheck.setoff);
+
+                                       v_pergroup_allaggs = l_load_gep1(
+                                               b, v_allpergroupsp, v_setoff, "");
+
+                                       LLVMBuildCondBr(
+                                               b,
+                                               LLVMBuildICmp(b, LLVMIntEQ,
+                                                                         LLVMBuildPtrToInt(
+                                                                                 b, v_pergroup_allaggs, TypeSizeT, ""),
+                                                                         l_sizet_const(0), ""),
+                                               opblocks[jumpnull],
+                                               opblocks[opno + 1]);
+                                       break;
+                               }
+
                        case EEOP_AGG_PLAIN_TRANS_INIT_STRICT_BYVAL:
                        case EEOP_AGG_PLAIN_TRANS_STRICT_BYVAL:
                        case EEOP_AGG_PLAIN_TRANS_BYVAL:

diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index 8bbf6621da0e36d933e699e605b265c1dc229ed9..dbe8649a5763f23ef9bbc1493fbe1a5775c39dd2 100644 (file)
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -225,6 +225,7 @@ typedef enum ExprEvalOp
        EEOP_AGG_DESERIALIZE,
        EEOP_AGG_STRICT_INPUT_CHECK_ARGS,
        EEOP_AGG_STRICT_INPUT_CHECK_NULLS,
+       EEOP_AGG_PLAIN_PERGROUP_NULLCHECK,
        EEOP_AGG_PLAIN_TRANS_INIT_STRICT_BYVAL,
        EEOP_AGG_PLAIN_TRANS_STRICT_BYVAL,
        EEOP_AGG_PLAIN_TRANS_BYVAL,
@@ -622,6 +623,13 @@ typedef struct ExprEvalStep
                        int                     jumpnull;
                }                       agg_strict_input_check;
 
+               /* for EEOP_AGG_PLAIN_PERGROUP_NULLCHECK */
+               struct
+               {
+                       int                     setoff;
+                       int                     jumpnull;
+               }                       agg_plain_pergroup_nullcheck;
+
                /* for EEOP_AGG_PLAIN_TRANS_[INIT_][STRICT_]{BYVAL,BYREF} */
                /* for EEOP_AGG_ORDERED_TRANS_{DATUM,TUPLE} */
                struct

diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 81fdfa4add32164660ae09136e63969050234e33..94890512dc881806aa31c4c2dc9670a3d46314bd 100644 (file)
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -255,7 +255,7 @@ extern ExprState *ExecInitQual(List *qual, PlanState *parent);
 extern ExprState *ExecInitCheck(List *qual, PlanState *parent);
 extern List *ExecInitExprList(List *nodes, PlanState *parent);
 extern ExprState *ExecBuildAggTrans(AggState *aggstate, struct AggStatePerPhaseData *phase,
-                                                                       bool doSort, bool doHash);
+                                                                       bool doSort, bool doHash, bool nullcheck);
 extern ExprState *ExecBuildGroupingEqual(TupleDesc ldesc, TupleDesc rdesc,
                                                                                 const TupleTableSlotOps *lops, const TupleTableSlotOps *rops,
                                                                                 int numCols,