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

Reply via email to