Re: [HACKERS] [PATCH] Generic type subscripting

Hi,

On 2018-01-04 16:47:50 +0100, Dmitry Dolgov wrote:
> diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
> index 16f908037c..ee50fda4ef 100644
> --- a/src/backend/executor/execExpr.c
> +++ b/src/backend/executor/execExpr.c
> @@ -64,7 +64,8 @@ static void ExecInitExprSlots(ExprState *state, Node *node);
> static bool get_last_attnums_walker(Node *node, LastAttnumInfo *info);
> static void ExecInitWholeRowVar(ExprEvalStep *scratch, Var *variable,
> ExprState *state);
> -static void ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref,
> +static void ExecInitSubscriptingRef(ExprEvalStep *scratch,
> + SubscriptingRef *sbsref,
> ExprState *state,
> Datum *resv, bool *resnull);
> static bool isAssignmentIndirectionExpr(Expr *expr);
> @@ -857,11 +858,11 @@ ExecInitExprRec(Expr *node, ExprState *state,
> break;
> }
>
> - case T_ArrayRef:
> + case T_SubscriptingRef:
> {
> - ArrayRef *aref = (ArrayRef *) node;
> + SubscriptingRef *sbsref = (SubscriptingRef *) node;
>
> - ExecInitArrayRef(&scratch, aref, state, resv, resnull);
> + ExecInitSubscriptingRef(&scratch, sbsref, state, resv, resnull);
> break;
> }
>
> @@ -1176,7 +1177,7 @@ ExecInitExprRec(Expr *node, ExprState *state,
> /*
> * Use the CaseTestExpr mechanism to pass down the old
> * value of the field being replaced; this is needed in
> - * case the newval is itself a FieldStore or ArrayRef that
> + * case the newval is itself a FieldStore or SubscriptingRef that
> * has to obtain and modify the old value. It's safe to
> * reuse the CASE mechanism because there cannot be a CASE
> * between here and where the value would be needed, and a
> @@ -2401,34 +2402,40 @@ ExecInitWholeRowVar(ExprEvalStep *scratch, Var *variable, ExprState *state)
> }
>
> /*
> - * Prepare evaluation of an ArrayRef expression.
> + * Prepare evaluation of a SubscriptingRef expression.
> */
> static void
> -ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref,
> +ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *sbsref,
> ExprState *state, Datum *resv, bool *resnull)
> {
> - bool isAssignment = (aref->refassgnexpr != NULL);
> - ArrayRefState *arefstate = palloc0(sizeof(ArrayRefState));
> - List *adjust_jumps = NIL;
> - ListCell *lc;
> - int i;
> + bool isAssignment = (sbsref->refassgnexpr != NULL);
> + SubscriptingRefState *sbsrefstate = palloc0(sizeof(SubscriptingRefState));
> + List *adjust_jumps = NIL;
> + ListCell *lc;
> + int i;
> + FmgrInfo *eval_finfo, *nested_finfo;
> +
> + eval_finfo = palloc0(sizeof(FmgrInfo));
> + nested_finfo = palloc0(sizeof(FmgrInfo));
> +
> + fmgr_info(sbsref->refevalfunc, eval_finfo);
> + if (OidIsValid(sbsref->refnestedfunc))
> + {
> + fmgr_info(sbsref->refnestedfunc, nested_finfo);
> + }
>
> - /* Fill constant fields of ArrayRefState */
> - arefstate->isassignment = isAssignment;
> - arefstate->refelemtype = aref->refelemtype;
> - arefstate->refattrlength = get_typlen(aref->refarraytype);
> - get_typlenbyvalalign(aref->refelemtype,
> - &arefstate->refelemlength,
> - &arefstate->refelembyval,
> - &arefstate->refelemalign);
> + /* Fill constant fields of SubscriptingRefState */
> + sbsrefstate->isassignment = isAssignment;
> + sbsrefstate->refelemtype = sbsref->refelemtype;
> + sbsrefstate->refattrlength = get_typlen(sbsref->refcontainertype);
>
> /*
> * Evaluate array input. It's safe to do so into resv/resnull, because we
> * won't use that as target for any of the other subexpressions, and it'll
> - * be overwritten by the final EEOP_ARRAYREF_FETCH/ASSIGN step, which is
> + * be overwritten by the final EEOP_SBSREF_FETCH/ASSIGN step, which is
> * pushed last.
> */
> - ExecInitExprRec(aref->refexpr, state, resv, resnull);
> + ExecInitExprRec(sbsref->refexpr, state, resv, resnull);
>
> /*
> * If refexpr yields NULL, and it's a fetch, then result is NULL. We can
> @@ -2440,92 +2447,95 @@ ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref,
> scratch->opcode = EEOP_JUMP_IF_NULL;
> scratch->d.jump.jumpdone = -1; /* adjust later */
> ExprEvalPushStep(state, scratch);
> +
> adjust_jumps = lappend_int(adjust_jumps,
> state->steps_len - 1);
> }
>
> /* Verify subscript list lengths are within limit */
> - if (list_length(aref->refupperindexpr) > MAXDIM)
> + if (list_length(sbsref->refupperindexpr) > MAX_SUBSCRIPT_DEPTH)
> ereport(ERROR,
> (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
> errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
> - list_length(aref->refupperindexpr), MAXDIM)));
> + list_length(sbsref->refupperindexpr), MAX_SUBSCRIPT_DEPTH)));
>
> - if (list_length(aref->reflowerindexpr) > MAXDIM)
> + if (list_length(sbsref->reflowerindexpr) > MAX_SUBSCRIPT_DEPTH)
> ereport(ERROR,
> (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
> errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
> - list_length(aref->reflowerindexpr), MAXDIM)));
> + list_length(sbsref->reflowerindexpr), MAX_SUBSCRIPT_DEPTH)));
>
> /* Evaluate upper subscripts */
> i = 0;
> - foreach(lc, aref->refupperindexpr)
> + foreach(lc, sbsref->refupperindexpr)
> {
> Expr *e = (Expr *) lfirst(lc);
>
> /* When slicing, individual subscript bounds can be omitted */
> if (!e)
> {
> - arefstate->upperprovided[i] = false;
> + sbsrefstate->upperprovided[i] = false;
> i++;
> continue;
> }
>
> - arefstate->upperprovided[i] = true;
> + sbsrefstate->upperprovided[i] = true;
>
> /* Each subscript is evaluated into subscriptvalue/subscriptnull */
> ExecInitExprRec(e, state,
> - &arefstate->subscriptvalue, &arefstate->subscriptnull);
> -
> - /* ... and then ARRAYREF_SUBSCRIPT saves it into step's workspace */
> - scratch->opcode = EEOP_ARRAYREF_SUBSCRIPT;
> - scratch->d.arrayref_subscript.state = arefstate;
> - scratch->d.arrayref_subscript.off = i;
> - scratch->d.arrayref_subscript.isupper = true;
> - scratch->d.arrayref_subscript.jumpdone = -1; /* adjust later */
> + &sbsrefstate->subscriptvalue, &sbsrefstate->subscriptnull);
> +
> + /* ... and then SBSREF_SUBSCRIPT saves it into step's workspace */
> + scratch->opcode = EEOP_SBSREF_SUBSCRIPT;
> + scratch->d.sbsref_subscript.state = sbsrefstate;
> + scratch->d.sbsref_subscript.off = i;
> + scratch->d.sbsref_subscript.isupper = true;
> + scratch->d.sbsref_subscript.jumpdone = -1; /* adjust later */
> ExprEvalPushStep(state, scratch);
> +
> adjust_jumps = lappend_int(adjust_jumps,
> state->steps_len - 1);
> i++;
> }
> - arefstate->numupper = i;
> + sbsrefstate->numupper = i;
>
> /* Evaluate lower subscripts similarly */
> i = 0;
> - foreach(lc, aref->reflowerindexpr)
> + foreach(lc, sbsref->reflowerindexpr)
> {
> Expr *e = (Expr *) lfirst(lc);
>
> /* When slicing, individual subscript bounds can be omitted */
> if (!e)
> {
> - arefstate->lowerprovided[i] = false;
> + sbsrefstate->lowerprovided[i] = false;
> i++;
> continue;
> }
>
> - arefstate->lowerprovided[i] = true;
> + sbsrefstate->lowerprovided[i] = true;
>
> /* Each subscript is evaluated into subscriptvalue/subscriptnull */
> ExecInitExprRec(e, state,
> - &arefstate->subscriptvalue, &arefstate->subscriptnull);
> -
> - /* ... and then ARRAYREF_SUBSCRIPT saves it into step's workspace */
> - scratch->opcode = EEOP_ARRAYREF_SUBSCRIPT;
> - scratch->d.arrayref_subscript.state = arefstate;
> - scratch->d.arrayref_subscript.off = i;
> - scratch->d.arrayref_subscript.isupper = false;
> - scratch->d.arrayref_subscript.jumpdone = -1; /* adjust later */
> + &sbsrefstate->subscriptvalue, &sbsrefstate->subscriptnull);
> +
> + /* ... and then SBSREF_SUBSCRIPT saves it into step's workspace */
> + scratch->opcode = EEOP_SBSREF_SUBSCRIPT;
> + scratch->d.sbsref_subscript.state = sbsrefstate;
> + scratch->d.sbsref_subscript.off = i;
> + scratch->d.sbsref_subscript.isupper = false;
> + scratch->d.sbsref_subscript.jumpdone = -1; /* adjust later */
> ExprEvalPushStep(state, scratch);
> +
> adjust_jumps = lappend_int(adjust_jumps,
> state->steps_len - 1);
> i++;
> }
> - arefstate->numlower = i;
> + sbsrefstate->numlower = i;
>
> /* Should be impossible if parser is sane, but check anyway: */
> - if (arefstate->numlower != 0 &&
> - arefstate->numupper != arefstate->numlower)
> + if (sbsrefstate->numlower != 0 &&
> + sbsrefstate->numupper != sbsrefstate->numlower)
> elog(ERROR, "upper and lower index lists are not same length");
>
> if (isAssignment)
> @@ -2535,7 +2545,7 @@ ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref,
>
> /*
> * We might have a nested-assignment situation, in which the
> - * refassgnexpr is itself a FieldStore or ArrayRef that needs to
> + * refassgnexpr is itself a FieldStore or SubscriptingRef that needs to
> * obtain and modify the previous value of the array element or slice
> * being replaced. If so, we have to extract that value from the
> * array and pass it down via the CaseTestExpr mechanism. It's safe
> @@ -2547,37 +2557,45 @@ ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref,
> * Since fetching the old element might be a nontrivial expense, do it
> * only if the argument actually needs it.
> */
> - if (isAssignmentIndirectionExpr(aref->refassgnexpr))
> + if (isAssignmentIndirectionExpr(sbsref->refassgnexpr))
> {
> - scratch->opcode = EEOP_ARRAYREF_OLD;
> - scratch->d.arrayref.state = arefstate;
> + scratch->opcode = EEOP_SBSREF_OLD;
> + scratch->d.sbsref.state = sbsrefstate;
> + scratch->d.sbsref.eval_finfo = eval_finfo;
> + scratch->d.sbsref.nested_finfo = nested_finfo;
> ExprEvalPushStep(state, scratch);
> }
>
> - /* ARRAYREF_OLD puts extracted value into prevvalue/prevnull */
> + /* SBSREF_OLD puts extracted value into prevvalue/prevnull */
> save_innermost_caseval = state->innermost_caseval;
> save_innermost_casenull = state->innermost_casenull;
> - state->innermost_caseval = &arefstate->prevvalue;
> - state->innermost_casenull = &arefstate->prevnull;
> + state->innermost_caseval = &sbsrefstate->prevvalue;
> + state->innermost_casenull = &sbsrefstate->prevnull;
>
> /* evaluate replacement value into replacevalue/replacenull */
> - ExecInitExprRec(aref->refassgnexpr, state,
> - &arefstate->replacevalue, &arefstate->replacenull);
> + ExecInitExprRec(sbsref->refassgnexpr, state,
> + &sbsrefstate->replacevalue, &sbsrefstate->replacenull);
>
> state->innermost_caseval = save_innermost_caseval;
> state->innermost_casenull = save_innermost_casenull;
>
> /* and perform the assignment */
> - scratch->opcode = EEOP_ARRAYREF_ASSIGN;
> - scratch->d.arrayref.state = arefstate;
> + scratch->opcode = EEOP_SBSREF_ASSIGN;
> + scratch->d.sbsref.state = sbsrefstate;
> + scratch->d.sbsref.eval_finfo = eval_finfo;
> + scratch->d.sbsref.nested_finfo = nested_finfo;
> ExprEvalPushStep(state, scratch);
> +
> }
> else
> {
> /* array fetch is much simpler */
> - scratch->opcode = EEOP_ARRAYREF_FETCH;
> - scratch->d.arrayref.state = arefstate;
> + scratch->opcode = EEOP_SBSREF_FETCH;
> + scratch->d.sbsref.state = sbsrefstate;
> + scratch->d.sbsref.eval_finfo = eval_finfo;
> + scratch->d.sbsref.nested_finfo = nested_finfo;
> ExprEvalPushStep(state, scratch);
> +
> }
>
> /* adjust jump targets */
> @@ -2585,10 +2603,10 @@ ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref,
> {
> ExprEvalStep *as = &state->steps[lfirst_int(lc)];
>
> - if (as->opcode == EEOP_ARRAYREF_SUBSCRIPT)
> + if (as->opcode == EEOP_SBSREF_SUBSCRIPT)
> {
> - Assert(as->d.arrayref_subscript.jumpdone == -1);
> - as->d.arrayref_subscript.jumpdone = state->steps_len;
> + Assert(as->d.sbsref_subscript.jumpdone == -1);
> + as->d.sbsref_subscript.jumpdone = state->steps_len;
> }
> else
> {
> @@ -2600,8 +2618,8 @@ ExecInitArrayRef(ExprEvalStep *scratch, ArrayRef *aref,
> }
>
> /*
> - * Helper for preparing ArrayRef expressions for evaluation: is expr a nested
> - * FieldStore or ArrayRef that needs the old element value passed down?
> + * Helper for preparing SubscriptingRef expressions for evaluation: is expr a nested
> + * FieldStore or SubscriptingRef that needs the old element value passed down?
> *
> * (We could use this in FieldStore too, but in that case passing the old
> * value is so cheap there's no need.)
> @@ -2624,11 +2642,11 @@ isAssignmentIndirectionExpr(Expr *expr)
> if (fstore->arg && IsA(fstore->arg, CaseTestExpr))
> return true;
> }
> - else if (IsA(expr, ArrayRef))
> + else if (IsA(expr, SubscriptingRef))
> {
> - ArrayRef *arrayRef = (ArrayRef *) expr;
> + SubscriptingRef *sbsRef = (SubscriptingRef *) expr;
>
> - if (arrayRef->refexpr && IsA(arrayRef->refexpr, CaseTestExpr))
> + if (sbsRef->refexpr && IsA(sbsRef->refexpr, CaseTestExpr))
> return true;
> }
> return false;
> diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
> index 2e88417265..0c72e80b58 100644
> --- a/src/backend/executor/execExprInterp.c
> +++ b/src/backend/executor/execExprInterp.c
> @@ -364,10 +364,10 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
> &&CASE_EEOP_FIELDSELECT,
> &&CASE_EEOP_FIELDSTORE_DEFORM,
> &&CASE_EEOP_FIELDSTORE_FORM,
> - &&CASE_EEOP_ARRAYREF_SUBSCRIPT,
> - &&CASE_EEOP_ARRAYREF_OLD,
> - &&CASE_EEOP_ARRAYREF_ASSIGN,
> - &&CASE_EEOP_ARRAYREF_FETCH,
> + &&CASE_EEOP_SBSREF_SUBSCRIPT,
> + &&CASE_EEOP_SBSREF_OLD,
> + &&CASE_EEOP_SBSREF_ASSIGN,
> + &&CASE_EEOP_SBSREF_FETCH,
> &&CASE_EEOP_DOMAIN_TESTVAL,
> &&CASE_EEOP_DOMAIN_NOTNULL,
> &&CASE_EEOP_DOMAIN_CHECK,
> @@ -1367,43 +1367,43 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
> EEO_NEXT();
> }
>
> - EEO_CASE(EEOP_ARRAYREF_SUBSCRIPT)
> + EEO_CASE(EEOP_SBSREF_SUBSCRIPT)
> {
> /* Process an array subscript */
>
> /* too complex for an inline implementation */
> - if (ExecEvalArrayRefSubscript(state, op))
> + if (ExecEvalSubscriptingRef(state, op))
> {
> EEO_NEXT();
> }
> else
> {
> - /* Subscript is null, short-circuit ArrayRef to NULL */
> - EEO_JUMP(op->d.arrayref_subscript.jumpdone);
> + /* Subscript is null, short-circuit SubscriptingRef to NULL */
> + EEO_JUMP(op->d.sbsref_subscript.jumpdone);
> }
> }
>
> - EEO_CASE(EEOP_ARRAYREF_OLD)
> + EEO_CASE(EEOP_SBSREF_OLD)
> {
> /*
> - * Fetch the old value in an arrayref assignment, in case it's
> + * Fetch the old value in an sbsref assignment, in case it's
> * referenced (via a CaseTestExpr) inside the assignment
> * expression.
> */
>
> /* too complex for an inline implementation */
> - ExecEvalArrayRefOld(state, op);
> + ExecEvalSubscriptingRefOld(state, op);
>
> EEO_NEXT();
> }
>
> /*
> - * Perform ArrayRef assignment
> + * Perform SubscriptingRef assignment
> */
> - EEO_CASE(EEOP_ARRAYREF_ASSIGN)
> + EEO_CASE(EEOP_SBSREF_ASSIGN)
> {
> /* too complex for an inline implementation */
> - ExecEvalArrayRefAssign(state, op);
> + ExecEvalSubscriptingRefAssign(state, op);
>
> EEO_NEXT();
> }
> @@ -1411,10 +1411,10 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
> /*
> * Fetch subset of an array.
> */
> - EEO_CASE(EEOP_ARRAYREF_FETCH)
> + EEO_CASE(EEOP_SBSREF_FETCH)
> {
> /* too complex for an inline implementation */
> - ExecEvalArrayRefFetch(state, op);
> + ExecEvalSubscriptingRefFetch(state, op);
>
> EEO_NEXT();
> }
> @@ -2702,197 +2702,115 @@ ExecEvalFieldStoreForm(ExprState *state, ExprEvalStep *op, ExprContext *econtext
> }
>
> /*
> - * Process a subscript in an ArrayRef expression.
> + * Process a subscript in a SubscriptingRef expression.
> *
> * If subscript is NULL, throw error in assignment case, or in fetch case
> * set result to NULL and return false (instructing caller to skip the rest
> - * of the ArrayRef sequence).
> + * of the SubscriptingRef sequence).
> *
> * Subscript expression result is in subscriptvalue/subscriptnull.
> * On success, integer subscript value has been saved in upperindex[] or
> * lowerindex[] for use later.
> */
> bool
> -ExecEvalArrayRefSubscript(ExprState *state, ExprEvalStep *op)
> +ExecEvalSubscriptingRef(ExprState *state, ExprEvalStep *op)
> {
> - ArrayRefState *arefstate = op->d.arrayref_subscript.state;
> - int *indexes;
> - int off;
> + SubscriptingRefState *sbsrefstate = op->d.sbsref_subscript.state;
> + Datum *indexes;
> + int off;
>
> /* If any index expr yields NULL, result is NULL or error */
> - if (arefstate->subscriptnull)
> + if (sbsrefstate->subscriptnull)
> {
> - if (arefstate->isassignment)
> + if (sbsrefstate->isassignment)
> ereport(ERROR,
> (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
> - errmsg("array subscript in assignment must not be null")));
> + errmsg("subscript in assignment must not be null")));
> *op->resnull = true;
> return false;
> }
>
> /* Convert datum to int, save in appropriate place */
> - if (op->d.arrayref_subscript.isupper)
> - indexes = arefstate->upperindex;
> + if (op->d.sbsref_subscript.isupper)
> + indexes = sbsrefstate->upper;
> else
> - indexes = arefstate->lowerindex;
> - off = op->d.arrayref_subscript.off;
> + indexes = sbsrefstate->lower;
> + off = op->d.sbsref_subscript.off;
>
> - indexes[off] = DatumGetInt32(arefstate->subscriptvalue);
> + indexes[off] = sbsrefstate->subscriptvalue;
>
> return true;
> }
>
> /*
> - * Evaluate ArrayRef fetch.
> + * Evaluate SubscriptingRef fetch.
> *
> - * Source array is in step's result variable.
> + * Source container is in step's result variable.
> */
> void
> -ExecEvalArrayRefFetch(ExprState *state, ExprEvalStep *op)
> +ExecEvalSubscriptingRefFetch(ExprState *state, ExprEvalStep *op)
> {
> - ArrayRefState *arefstate = op->d.arrayref.state;
> -
> - /* Should not get here if source array (or any subscript) is null */
> + /* Should not get here if source container (or any subscript) is null */
> Assert(!(*op->resnull));
>
> - if (arefstate->numlower == 0)
> - {
> - /* Scalar case */
> - *op->resvalue = array_get_element(*op->resvalue,
> - arefstate->numupper,
> - arefstate->upperindex,
> - arefstate->refattrlength,
> - arefstate->refelemlength,
> - arefstate->refelembyval,
> - arefstate->refelemalign,
> - op->resnull);
> - }
> - else
> - {
> - /* Slice case */
> - *op->resvalue = array_get_slice(*op->resvalue,
> - arefstate->numupper,
> - arefstate->upperindex,
> - arefstate->lowerindex,
> - arefstate->upperprovided,
> - arefstate->lowerprovided,
> - arefstate->refattrlength,
> - arefstate->refelemlength,
> - arefstate->refelembyval,
> - arefstate->refelemalign);
> - }
> + *op->resvalue = FunctionCall2(op->d.sbsref.eval_finfo,
> + PointerGetDatum(*op->resvalue),
> + PointerGetDatum(op));
> }
>
> /*
> - * Compute old array element/slice value for an ArrayRef assignment
> - * expression. Will only be generated if the new-value subexpression
> - * contains ArrayRef or FieldStore. The value is stored into the
> - * ArrayRefState's prevvalue/prevnull fields.
> + * Compute old container element/slice value for a SubscriptingRef assignment
> + * expression. Will only be generated if the new-value subexpression
> + * contains SubscriptingRef or FieldStore. The value is stored into the
> + * SubscriptingRefState's prevvalue/prevnull fields.
> */
> void
> -ExecEvalArrayRefOld(ExprState *state, ExprEvalStep *op)
> +ExecEvalSubscriptingRefOld(ExprState *state, ExprEvalStep *op)
> {
> - ArrayRefState *arefstate = op->d.arrayref.state;
> + SubscriptingRefState *sbsrefstate = op->d.sbsref.state;
>
> if (*op->resnull)
> {
> - /* whole array is null, so any element or slice is too */
> - arefstate->prevvalue = (Datum) 0;
> - arefstate->prevnull = true;
> - }
> - else if (arefstate->numlower == 0)
> - {
> - /* Scalar case */
> - arefstate->prevvalue = array_get_element(*op->resvalue,
> - arefstate->numupper,
> - arefstate->upperindex,
> - arefstate->refattrlength,
> - arefstate->refelemlength,
> - arefstate->refelembyval,
> - arefstate->refelemalign,
> - &arefstate->prevnull);
> + /* whole container is null, so any element or slice is too */
> + sbsrefstate->prevvalue = (Datum) 0;
> + sbsrefstate->prevnull = true;
> }
> else
> {
> - /* Slice case */
> - /* this is currently unreachable */
> - arefstate->prevvalue = array_get_slice(*op->resvalue,
> - arefstate->numupper,
> - arefstate->upperindex,
> - arefstate->lowerindex,
> - arefstate->upperprovided,
> - arefstate->lowerprovided,
> - arefstate->refattrlength,
> - arefstate->refelemlength,
> - arefstate->refelembyval,
> - arefstate->refelemalign);
> - arefstate->prevnull = false;
> + sbsrefstate->prevvalue = FunctionCall2(op->d.sbsref.nested_finfo,
> + PointerGetDatum(*op->resvalue),
> + PointerGetDatum(op));
> +
> + if (sbsrefstate->numlower != 0)
> + sbsrefstate->prevnull = false;
> +
> }
> }
>
> /*
> - * Evaluate ArrayRef assignment.
> + * Evaluate SubscriptingRef assignment.
> *
> - * Input array (possibly null) is in result area, replacement value is in
> - * ArrayRefState's replacevalue/replacenull.
> + * Input container (possibly null) is in result area, replacement value is in
> + * SubscriptingRefState's replacevalue/replacenull.
> */
> void
> -ExecEvalArrayRefAssign(ExprState *state, ExprEvalStep *op)
> +ExecEvalSubscriptingRefAssign(ExprState *state, ExprEvalStep *op)
> {
> - ArrayRefState *arefstate = op->d.arrayref.state;
> -
> + SubscriptingRefState *sbsrefstate = op->d.sbsref.state;
> /*
> - * For an assignment to a fixed-length array type, both the original array
> - * and the value to be assigned into it must be non-NULL, else we punt and
> - * return the original array.
> + * For an assignment to a fixed-length container type, both the original
> + * container and the value to be assigned into it must be non-NULL, else we
> + * punt and return the original container.
> */
> - if (arefstate->refattrlength > 0) /* fixed-length array? */
> + if (sbsrefstate->refattrlength > 0)
> {
> - if (*op->resnull || arefstate->replacenull)
> + if (*op->resnull || sbsrefstate->replacenull)
> return;
> }
>
> - /*
> - * For assignment to varlena arrays, we handle a NULL original array by
> - * substituting an empty (zero-dimensional) array; insertion of the new
> - * element will result in a singleton array value. It does not matter
> - * whether the new element is NULL.
> - */
> - if (*op->resnull)
> - {
> - *op->resvalue = PointerGetDatum(construct_empty_array(arefstate->refelemtype));
> - *op->resnull = false;
> - }
> -
> - if (arefstate->numlower == 0)
> - {
> - /* Scalar case */
> - *op->resvalue = array_set_element(*op->resvalue,
> - arefstate->numupper,
> - arefstate->upperindex,
> - arefstate->replacevalue,
> - arefstate->replacenull,
> - arefstate->refattrlength,
> - arefstate->refelemlength,
> - arefstate->refelembyval,
> - arefstate->refelemalign);
> - }
> - else
> - {
> - /* Slice case */
> - *op->resvalue = array_set_slice(*op->resvalue,
> - arefstate->numupper,
> - arefstate->upperindex,
> - arefstate->lowerindex,
> - arefstate->upperprovided,
> - arefstate->lowerprovided,
> - arefstate->replacevalue,
> - arefstate->replacenull,
> - arefstate->refattrlength,
> - arefstate->refelemlength,
> - arefstate->refelembyval,
> - arefstate->refelemalign);
> - }
> + *op->resvalue = FunctionCall2(op->d.sbsref.eval_finfo,
> + PointerGetDatum(*op->resvalue),
> + PointerGetDatum(op));
> }

You might not love me for this suggestion, but I'd like to see the
renaming here split from the rest of the patch. There's a lot of diff
that's just more or less automatic changes, making it hard to see the
actual meaningful changes.

- Andres