Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 2a41507

Browse files
committed
Revert "Do execGrouping.c via expression eval machinery."
This reverts commit 773aec7. There's an unresolved issue in the reverted commit: It only creates one comparator function, but in for the nodeSubplan.c case we need more (c.f. FindTupleHashEntry vs LookupTupleHashEntry calls in nodeSubplan.c). This isn't too difficult to fix, but it's not entirely trivial either. The fact that the issue only causes breakage on 32bit systems shows that the current test coverage isn't that great. To avoid turning half the buildfarm red till those two issues are addressed, revert.
1 parent 773aec7 commit 2a41507

15 files changed

+366
-498
lines changed

src/backend/executor/execExpr.c

-118
Original file line numberDiff line numberDiff line change
@@ -3193,121 +3193,3 @@ ExecBuildAggTransCall(ExprState *state, AggState *aggstate,
31933193
as->d.agg_strict_trans_check.jumpnull = state->steps_len;
31943194
}
31953195
}
3196-
3197-
/*
3198-
* Build equality expression that can be evaluated using ExecQual(), returning
3199-
* true if the expression context's inner/outer tuple are NOT DISTINCT. I.e
3200-
* two nulls match, a null and a not-null don't match.
3201-
*
3202-
* desc: tuple descriptor of the to-be-compared tuples
3203-
* numCols: the number of attributes to be examined
3204-
* keyColIdx: array of attribute column numbers
3205-
* eqFunctions: array of function oids of the equality functions to use
3206-
* parent: parent executor node
3207-
*/
3208-
ExprState *
3209-
ExecBuildGroupingEqual(TupleDesc desc,
3210-
int numCols,
3211-
AttrNumber *keyColIdx,
3212-
Oid *eqfunctions,
3213-
PlanState *parent)
3214-
{
3215-
ExprState *state = makeNode(ExprState);
3216-
ExprEvalStep scratch = {0};
3217-
int natt;
3218-
int maxatt = -1;
3219-
List *adjust_jumps = NIL;
3220-
ListCell *lc;
3221-
3222-
/*
3223-
* When no columns are actually compared, the result's always true. See
3224-
* special case in ExecQual().
3225-
*/
3226-
if (numCols == 0)
3227-
return NULL;
3228-
3229-
state->expr = NULL;
3230-
state->flags = EEO_FLAG_IS_QUAL;
3231-
state->parent = parent;
3232-
3233-
scratch.resvalue = &state->resvalue;
3234-
scratch.resnull = &state->resnull;
3235-
3236-
/* compute max needed attribute */
3237-
for (natt = 0; natt < numCols; natt++)
3238-
{
3239-
int attno = keyColIdx[natt];
3240-
3241-
if (attno > maxatt)
3242-
maxatt = attno;
3243-
}
3244-
Assert(maxatt >= 0);
3245-
3246-
/* push deform steps */
3247-
scratch.opcode = EEOP_INNER_FETCHSOME;
3248-
scratch.d.fetch.last_var = maxatt;
3249-
ExprEvalPushStep(state, &scratch);
3250-
3251-
scratch.opcode = EEOP_OUTER_FETCHSOME;
3252-
scratch.d.fetch.last_var = maxatt;
3253-
ExprEvalPushStep(state, &scratch);
3254-
3255-
/*
3256-
* Start comparing at the last field (least significant sort key). That's
3257-
* the most likely to be different if we are dealing with sorted input.
3258-
*/
3259-
for (natt = numCols; --natt >= 0;)
3260-
{
3261-
int attno = keyColIdx[natt];
3262-
Form_pg_attribute att = TupleDescAttr(desc, attno - 1);
3263-
Var *larg,
3264-
*rarg;
3265-
List *args;
3266-
3267-
/*
3268-
* Reusing ExecInitFunc() requires creating Vars, but still seems
3269-
* worth it from a code reuse perspective.
3270-
*/
3271-
3272-
/* left arg */
3273-
larg = makeVar(INNER_VAR, attno, att->atttypid,
3274-
att->atttypmod, InvalidOid, 0);
3275-
/* right arg */
3276-
rarg = makeVar(OUTER_VAR, attno, att->atttypid,
3277-
att->atttypmod, InvalidOid, 0);
3278-
args = list_make2(larg, rarg);
3279-
3280-
/* evaluate distinctness */
3281-
ExecInitFunc(&scratch, NULL,
3282-
args, eqfunctions[natt], InvalidOid,
3283-
state);
3284-
scratch.opcode = EEOP_NOT_DISTINCT;
3285-
ExprEvalPushStep(state, &scratch);
3286-
3287-
/* then emit EEOP_QUAL to detect if result is false (or null) */
3288-
scratch.opcode = EEOP_QUAL;
3289-
scratch.d.qualexpr.jumpdone = -1;
3290-
ExprEvalPushStep(state, &scratch);
3291-
adjust_jumps = lappend_int(adjust_jumps,
3292-
state->steps_len - 1);
3293-
}
3294-
3295-
/* adjust jump targets */
3296-
foreach(lc, adjust_jumps)
3297-
{
3298-
ExprEvalStep *as = &state->steps[lfirst_int(lc)];
3299-
3300-
Assert(as->opcode == EEOP_QUAL);
3301-
Assert(as->d.qualexpr.jumpdone == -1);
3302-
as->d.qualexpr.jumpdone = state->steps_len;
3303-
}
3304-
3305-
scratch.resvalue = NULL;
3306-
scratch.resnull = NULL;
3307-
scratch.opcode = EEOP_DONE;
3308-
ExprEvalPushStep(state, &scratch);
3309-
3310-
ExecReadyExpr(state);
3311-
3312-
return state;
3313-
}

src/backend/executor/execExprInterp.c

-29
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,6 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
355355
&&CASE_EEOP_MAKE_READONLY,
356356
&&CASE_EEOP_IOCOERCE,
357357
&&CASE_EEOP_DISTINCT,
358-
&&CASE_EEOP_NOT_DISTINCT,
359358
&&CASE_EEOP_NULLIF,
360359
&&CASE_EEOP_SQLVALUEFUNCTION,
361360
&&CASE_EEOP_CURRENTOFEXPR,
@@ -1199,34 +1198,6 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
11991198
EEO_NEXT();
12001199
}
12011200

1202-
/* see EEOP_DISTINCT for comments, this is just inverted */
1203-
EEO_CASE(EEOP_NOT_DISTINCT)
1204-
{
1205-
FunctionCallInfo fcinfo = op->d.func.fcinfo_data;
1206-
1207-
if (fcinfo->argnull[0] && fcinfo->argnull[1])
1208-
{
1209-
*op->resvalue = BoolGetDatum(true);
1210-
*op->resnull = false;
1211-
}
1212-
else if (fcinfo->argnull[0] || fcinfo->argnull[1])
1213-
{
1214-
*op->resvalue = BoolGetDatum(false);
1215-
*op->resnull = false;
1216-
}
1217-
else
1218-
{
1219-
Datum eqresult;
1220-
1221-
fcinfo->isnull = false;
1222-
eqresult = op->d.func.fn_addr(fcinfo);
1223-
*op->resvalue = eqresult;
1224-
*op->resnull = fcinfo->isnull;
1225-
}
1226-
1227-
EEO_NEXT();
1228-
}
1229-
12301201
EEO_CASE(EEOP_NULLIF)
12311202
{
12321203
/*

0 commit comments

Comments
 (0)