Skip to content

Commit c954d49

Browse files
jeff-davisjeff-davis
committed
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
1 parent 3ed2005 commit c954d49

File tree

6 files changed

+101
-7
lines changed

6 files changed

+101
-7
lines changed

‎src/backend/executor/execExpr.c

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,8 @@ static void ExecInitCoerceToDomain(ExprEvalStep *scratch, CoerceToDomain *ctest,
7979
static void ExecBuildAggTransCall(ExprState *state, AggState *aggstate,
8080
ExprEvalStep *scratch,
8181
FunctionCallInfo fcinfo, AggStatePerTrans pertrans,
82-
int transno, int setno, int setoff, bool ishash);
82+
int transno, int setno, int setoff, bool ishash,
83+
bool nullcheck);
8384

8485

8586
/*
@@ -2924,10 +2925,13 @@ ExecInitCoerceToDomain(ExprEvalStep *scratch, CoerceToDomain *ctest,
29242925
* check for filters, evaluate aggregate input, check that that input is not
29252926
* NULL for a strict transition function, and then finally invoke the
29262927
* transition for each of the concurrently computed grouping sets.
2928+
*
2929+
* If nullcheck is true, the generated code will check for a NULL pointer to
2930+
* the array of AggStatePerGroup, and skip evaluation if so.
29272931
*/
29282932
ExprState *
29292933
ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
2930-
bool doSort, bool doHash)
2934+
bool doSort, bool doHash, bool nullcheck)
29312935
{
29322936
ExprState *state = makeNode(ExprState);
29332937
PlanState *parent = &aggstate->ss.ps;
@@ -3158,7 +3162,8 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
31583162
for (int setno = 0; setno < processGroupingSets; setno++)
31593163
{
31603164
ExecBuildAggTransCall(state, aggstate, &scratch, trans_fcinfo,
3161-
pertrans, transno, setno, setoff, false);
3165+
pertrans, transno, setno, setoff, false,
3166+
nullcheck);
31623167
setoff++;
31633168
}
31643169
}
@@ -3177,7 +3182,8 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
31773182
for (int setno = 0; setno < numHashes; setno++)
31783183
{
31793184
ExecBuildAggTransCall(state, aggstate, &scratch, trans_fcinfo,
3180-
pertrans, transno, setno, setoff, true);
3185+
pertrans, transno, setno, setoff, true,
3186+
nullcheck);
31813187
setoff++;
31823188
}
31833189
}
@@ -3227,15 +3233,28 @@ static void
32273233
ExecBuildAggTransCall(ExprState *state, AggState *aggstate,
32283234
ExprEvalStep *scratch,
32293235
FunctionCallInfo fcinfo, AggStatePerTrans pertrans,
3230-
int transno, int setno, int setoff, bool ishash)
3236+
int transno, int setno, int setoff, bool ishash,
3237+
bool nullcheck)
32313238
{
32323239
ExprContext *aggcontext;
3240+
int adjust_jumpnull = -1;
32333241

32343242
if (ishash)
32353243
aggcontext = aggstate->hashcontext;
32363244
else
32373245
aggcontext = aggstate->aggcontexts[setno];
32383246

3247+
/* add check for NULL pointer? */
3248+
if (nullcheck)
3249+
{
3250+
scratch->opcode = EEOP_AGG_PLAIN_PERGROUP_NULLCHECK;
3251+
scratch->d.agg_plain_pergroup_nullcheck.setoff = setoff;
3252+
/* adjust later */
3253+
scratch->d.agg_plain_pergroup_nullcheck.jumpnull = -1;
3254+
ExprEvalPushStep(state, scratch);
3255+
adjust_jumpnull = state->steps_len - 1;
3256+
}
3257+
32393258
/*
32403259
* Determine appropriate transition implementation.
32413260
*
@@ -3303,6 +3322,16 @@ ExecBuildAggTransCall(ExprState *state, AggState *aggstate,
33033322
scratch->d.agg_trans.transno = transno;
33043323
scratch->d.agg_trans.aggcontext = aggcontext;
33053324
ExprEvalPushStep(state, scratch);
3325+
3326+
/* fix up jumpnull */
3327+
if (adjust_jumpnull != -1)
3328+
{
3329+
ExprEvalStep *as = &state->steps[adjust_jumpnull];
3330+
3331+
Assert(as->opcode == EEOP_AGG_PLAIN_PERGROUP_NULLCHECK);
3332+
Assert(as->d.agg_plain_pergroup_nullcheck.jumpnull == -1);
3333+
as->d.agg_plain_pergroup_nullcheck.jumpnull = state->steps_len;
3334+
}
33063335
}
33073336

33083337
/*

‎src/backend/executor/execExprInterp.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
435435
&&CASE_EEOP_AGG_DESERIALIZE,
436436
&&CASE_EEOP_AGG_STRICT_INPUT_CHECK_ARGS,
437437
&&CASE_EEOP_AGG_STRICT_INPUT_CHECK_NULLS,
438+
&&CASE_EEOP_AGG_PLAIN_PERGROUP_NULLCHECK,
438439
&&CASE_EEOP_AGG_PLAIN_TRANS_INIT_STRICT_BYVAL,
439440
&&CASE_EEOP_AGG_PLAIN_TRANS_STRICT_BYVAL,
440441
&&CASE_EEOP_AGG_PLAIN_TRANS_BYVAL,
@@ -1603,6 +1604,22 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
16031604
EEO_NEXT();
16041605
}
16051606

1607+
/*
1608+
* Check for a NULL pointer to the per-group states.
1609+
*/
1610+
1611+
EEO_CASE(EEOP_AGG_PLAIN_PERGROUP_NULLCHECK)
1612+
{
1613+
AggState *aggstate = castNode(AggState, state->parent);
1614+
AggStatePerGroup pergroup_allaggs = aggstate->all_pergroups
1615+
[op->d.agg_plain_pergroup_nullcheck.setoff];
1616+
1617+
if (pergroup_allaggs == NULL)
1618+
EEO_JUMP(op->d.agg_plain_pergroup_nullcheck.jumpnull);
1619+
1620+
EEO_NEXT();
1621+
}
1622+
16061623
/*
16071624
* Different types of aggregate transition functions are implemented
16081625
* as different types of steps, to avoid incurring unnecessary

‎src/backend/executor/nodeAgg.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2928,7 +2928,8 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
29282928
else
29292929
Assert(false);
29302930

2931-
phase->evaltrans = ExecBuildAggTrans(aggstate, phase, dosort, dohash);
2931+
phase->evaltrans = ExecBuildAggTrans(aggstate, phase, dosort, dohash,
2932+
false);
29322933

29332934
}
29342935

‎src/backend/jit/llvm/llvmjit_expr.c

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2046,6 +2046,45 @@ llvm_compile_expr(ExprState *state)
20462046
break;
20472047
}
20482048

2049+
case EEOP_AGG_PLAIN_PERGROUP_NULLCHECK:
2050+
{
2051+
int jumpnull;
2052+
LLVMValueRef v_aggstatep;
2053+
LLVMValueRef v_allpergroupsp;
2054+
LLVMValueRef v_pergroup_allaggs;
2055+
LLVMValueRef v_setoff;
2056+
2057+
jumpnull = op->d.agg_plain_pergroup_nullcheck.jumpnull;
2058+
2059+
/*
2060+
* pergroup_allaggs = aggstate->all_pergroups
2061+
* [op->d.agg_plain_pergroup_nullcheck.setoff];
2062+
*/
2063+
v_aggstatep = LLVMBuildBitCast(
2064+
b, v_parent, l_ptr(StructAggState), "");
2065+
2066+
v_allpergroupsp = l_load_struct_gep(
2067+
b, v_aggstatep,
2068+
FIELDNO_AGGSTATE_ALL_PERGROUPS,
2069+
"aggstate.all_pergroups");
2070+
2071+
v_setoff = l_int32_const(
2072+
op->d.agg_plain_pergroup_nullcheck.setoff);
2073+
2074+
v_pergroup_allaggs = l_load_gep1(
2075+
b, v_allpergroupsp, v_setoff, "");
2076+
2077+
LLVMBuildCondBr(
2078+
b,
2079+
LLVMBuildICmp(b, LLVMIntEQ,
2080+
LLVMBuildPtrToInt(
2081+
b, v_pergroup_allaggs, TypeSizeT, ""),
2082+
l_sizet_const(0), ""),
2083+
opblocks[jumpnull],
2084+
opblocks[opno + 1]);
2085+
break;
2086+
}
2087+
20492088
case EEOP_AGG_PLAIN_TRANS_INIT_STRICT_BYVAL:
20502089
case EEOP_AGG_PLAIN_TRANS_STRICT_BYVAL:
20512090
case EEOP_AGG_PLAIN_TRANS_BYVAL:

‎src/include/executor/execExpr.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ typedef enum ExprEvalOp
225225
EEOP_AGG_DESERIALIZE,
226226
EEOP_AGG_STRICT_INPUT_CHECK_ARGS,
227227
EEOP_AGG_STRICT_INPUT_CHECK_NULLS,
228+
EEOP_AGG_PLAIN_PERGROUP_NULLCHECK,
228229
EEOP_AGG_PLAIN_TRANS_INIT_STRICT_BYVAL,
229230
EEOP_AGG_PLAIN_TRANS_STRICT_BYVAL,
230231
EEOP_AGG_PLAIN_TRANS_BYVAL,
@@ -622,6 +623,13 @@ typedef struct ExprEvalStep
622623
int jumpnull;
623624
} agg_strict_input_check;
624625

626+
/* for EEOP_AGG_PLAIN_PERGROUP_NULLCHECK */
627+
struct
628+
{
629+
int setoff;
630+
int jumpnull;
631+
} agg_plain_pergroup_nullcheck;
632+
625633
/* for EEOP_AGG_PLAIN_TRANS_[INIT_][STRICT_]{BYVAL,BYREF} */
626634
/* for EEOP_AGG_ORDERED_TRANS_{DATUM,TUPLE} */
627635
struct

‎src/include/executor/executor.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ extern ExprState *ExecInitQual(List *qual, PlanState *parent);
255255
extern ExprState *ExecInitCheck(List *qual, PlanState *parent);
256256
extern List *ExecInitExprList(List *nodes, PlanState *parent);
257257
extern ExprState *ExecBuildAggTrans(AggState *aggstate, struct AggStatePerPhaseData *phase,
258-
bool doSort, bool doHash);
258+
bool doSort, bool doHash, bool nullcheck);
259259
extern ExprState *ExecBuildGroupingEqual(TupleDesc ldesc, TupleDesc rdesc,
260260
const TupleTableSlotOps *lops, const TupleTableSlotOps *rops,
261261
int numCols,

0 commit comments

Comments
 (0)