diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 79b325c7cf..c8382e9381 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -2523,11 +2523,19 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *sbsref, ExprState *state, Datum *resv, bool *resnull) { bool isAssignment = (sbsref->refassgnexpr != NULL); - SubscriptingRefState *sbsrefstate = palloc0(sizeof(SubscriptingRefState)); + int nupper = list_length(sbsref->refupperindexpr); + int nlower = list_length(sbsref->reflowerindexpr); + SubscriptingRefState *sbsrefstate; + char *ptr; List *adjust_jumps = NIL; ListCell *lc; int i; + /* Allocate enough space for per-subscript arrays too */ + sbsrefstate = palloc0(MAXALIGN(sizeof(SubscriptingRefState)) + + (nupper + nlower) * (sizeof(Datum) + + 2 * sizeof(bool))); + /* Fill constant fields of SubscriptingRefState */ sbsrefstate->isassignment = isAssignment; sbsrefstate->refelemtype = sbsref->refelemtype; @@ -2536,6 +2544,22 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *sbsref, &sbsrefstate->refelemlength, &sbsrefstate->refelembyval, &sbsrefstate->refelemalign); + sbsrefstate->numupper = nupper; + sbsrefstate->numlower = nlower; + /* Set up per-subscript arrays */ + ptr = ((char *) sbsrefstate) + MAXALIGN(sizeof(SubscriptingRefState)); + sbsrefstate->upperindex = (Datum *) ptr; + ptr += nupper * sizeof(Datum); + sbsrefstate->lowerindex = (Datum *) ptr; + ptr += nlower * sizeof(Datum); + sbsrefstate->upperprovided = (bool *) ptr; + ptr += nupper * sizeof(bool); + sbsrefstate->lowerprovided = (bool *) ptr; + ptr += nlower * sizeof(bool); + sbsrefstate->upperindexnull = (bool *) ptr; + ptr += nupper * sizeof(bool); + sbsrefstate->lowerindexnull = (bool *) ptr; + /* ptr += nlower * sizeof(bool); */ /* * Evaluate array input. It's safe to do so into resv/resnull, because we @@ -2548,7 +2572,8 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *sbsref, /* * If refexpr yields NULL, and it's a fetch, then result is NULL. We can * implement this with just JUMP_IF_NULL, since we evaluated the array - * into the desired target location. + * into the desired target location. (XXX is it okay to impose these + * semantics on all forms of subscripting?) */ if (!isAssignment) { @@ -2559,19 +2584,6 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *sbsref, state->steps_len - 1); } - /* Verify subscript list lengths are within limit */ - if (list_length(sbsref->refupperindexpr) > MAXDIM) - ereport(ERROR, - (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)", - list_length(sbsref->refupperindexpr), MAXDIM))); - - if (list_length(sbsref->reflowerindexpr) > MAXDIM) - ereport(ERROR, - (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)", - list_length(sbsref->reflowerindexpr), MAXDIM))); - /* Evaluate upper subscripts */ i = 0; foreach(lc, sbsref->refupperindexpr) @@ -2582,28 +2594,18 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *sbsref, if (!e) { sbsrefstate->upperprovided[i] = false; - i++; - continue; + sbsrefstate->upperindexnull[i] = true; + } + else + { + sbsrefstate->upperprovided[i] = true; + /* Each subscript is evaluated into appropriate array entry */ + ExecInitExprRec(e, state, + &sbsrefstate->upperindex[i], + &sbsrefstate->upperindexnull[i]); } - - sbsrefstate->upperprovided[i] = true; - - /* Each subscript is evaluated into subscriptvalue/subscriptnull */ - ExecInitExprRec(e, state, - &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++; } - sbsrefstate->numupper = i; /* Evaluate lower subscripts similarly */ i = 0; @@ -2615,33 +2617,26 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *sbsref, if (!e) { sbsrefstate->lowerprovided[i] = false; - i++; - continue; + sbsrefstate->lowerindexnull[i] = true; + } + else + { + sbsrefstate->lowerprovided[i] = true; + /* Each subscript is evaluated into appropriate array entry */ + ExecInitExprRec(e, state, + &sbsrefstate->lowerindex[i], + &sbsrefstate->lowerindexnull[i]); } - - sbsrefstate->lowerprovided[i] = true; - - /* Each subscript is evaluated into subscriptvalue/subscriptnull */ - ExecInitExprRec(e, state, - &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++; } - sbsrefstate->numlower = i; - /* Should be impossible if parser is sane, but check anyway: */ - if (sbsrefstate->numlower != 0 && - sbsrefstate->numupper != sbsrefstate->numlower) - elog(ERROR, "upper and lower index lists are not same length"); + /* SBSREF_SUBSCRIPTS checks and converts all the subscripts at once */ + scratch->opcode = EEOP_SBSREF_SUBSCRIPTS; + scratch->d.sbsref_subscript.state = sbsrefstate; + scratch->d.sbsref_subscript.jumpdone = -1; /* adjust later */ + ExprEvalPushStep(state, scratch); + adjust_jumps = lappend_int(adjust_jumps, + state->steps_len - 1); if (isAssignment) { @@ -2686,7 +2681,6 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *sbsref, scratch->opcode = EEOP_SBSREF_ASSIGN; scratch->d.sbsref.state = sbsrefstate; ExprEvalPushStep(state, scratch); - } else { @@ -2694,7 +2688,6 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *sbsref, scratch->opcode = EEOP_SBSREF_FETCH; scratch->d.sbsref.state = sbsrefstate; ExprEvalPushStep(state, scratch); - } /* adjust jump targets */ @@ -2702,7 +2695,7 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *sbsref, { ExprEvalStep *as = &state->steps[lfirst_int(lc)]; - if (as->opcode == EEOP_SBSREF_SUBSCRIPT) + if (as->opcode == EEOP_SBSREF_SUBSCRIPTS) { Assert(as->d.sbsref_subscript.jumpdone == -1); as->d.sbsref_subscript.jumpdone = state->steps_len; diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index c09371ad58..1853405026 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -417,7 +417,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) &&CASE_EEOP_FIELDSELECT, &&CASE_EEOP_FIELDSTORE_DEFORM, &&CASE_EEOP_FIELDSTORE_FORM, - &&CASE_EEOP_SBSREF_SUBSCRIPT, + &&CASE_EEOP_SBSREF_SUBSCRIPTS, &&CASE_EEOP_SBSREF_OLD, &&CASE_EEOP_SBSREF_ASSIGN, &&CASE_EEOP_SBSREF_FETCH, @@ -1396,9 +1396,9 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) EEO_NEXT(); } - EEO_CASE(EEOP_SBSREF_SUBSCRIPT) + EEO_CASE(EEOP_SBSREF_SUBSCRIPTS) { - /* Process an array subscript */ + /* Process array subscript(s) */ /* too complex for an inline implementation */ if (ExecEvalSubscriptingRef(state, op)) @@ -3123,42 +3123,87 @@ ExecEvalFieldStoreForm(ExprState *state, ExprEvalStep *op, ExprContext *econtext } /* - * Process a subscript in a SubscriptingRef expression. + * Process the subscripts in a SubscriptingRef expression. * - * If subscript is NULL, throw error in assignment case, or in fetch case + * If any 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 SubscriptingRef sequence). * - * Subscript expression result is in subscriptvalue/subscriptnull. - * On success, integer subscript value has been saved in upperindex[] or - * lowerindex[] for use later. + * We convert all the subscripts to plain integers and save them in the + * sbsrefstate->workspace array. */ bool ExecEvalSubscriptingRef(ExprState *state, ExprEvalStep *op) { - SubscriptingRefState *sbsrefstate = op->d.sbsref_subscript.state; + SubscriptingRefState *sbsrefstate = op->d.sbsref.state; int *indexes; - int off; - /* If any index expr yields NULL, result is NULL or error */ - if (sbsrefstate->subscriptnull) + /* + * Allocate workspace if first time through. This is also a good place to + * enforce the implementation limit on number of array subscripts. + */ + if (sbsrefstate->workspace == NULL) { - if (sbsrefstate->isassignment) + if (sbsrefstate->numupper > MAXDIM) ereport(ERROR, - (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), - errmsg("array subscript in assignment must not be null"))); - *op->resnull = true; - return false; + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)", + sbsrefstate->numupper, MAXDIM))); + + /* Should be impossible if parser is sane, but check anyway: */ + if (sbsrefstate->numlower != 0 && + sbsrefstate->numupper != sbsrefstate->numlower) + elog(ERROR, "upper and lower index lists are not same length"); + + /* + * Workspace always has room for MAXDIM subscripts even if we don't + * have that many. This is necessary because array_get/set_slice may + * scribble on the extra entries. + */ + sbsrefstate->workspace = + MemoryContextAlloc(GetMemoryChunkContext(sbsrefstate), + 2 * MAXDIM * sizeof(int)); } - /* Convert datum to int, save in appropriate place */ - if (op->d.sbsref_subscript.isupper) - indexes = sbsrefstate->upperindex; - else - indexes = sbsrefstate->lowerindex; - off = op->d.sbsref_subscript.off; + /* Process upper subscripts */ + indexes = (int *) sbsrefstate->workspace; + for (int i = 0; i < sbsrefstate->numupper; i++) + { + if (sbsrefstate->upperprovided[i]) + { + /* If any index expr yields NULL, result is NULL or error */ + if (sbsrefstate->upperindexnull[i]) + { + if (sbsrefstate->isassignment) + ereport(ERROR, + (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), + errmsg("array subscript in assignment must not be null"))); + *op->resnull = true; + return false; + } + indexes[i] = DatumGetInt32(sbsrefstate->upperindex[i]); + } + } - indexes[off] = DatumGetInt32(sbsrefstate->subscriptvalue); + /* Likewise for lower subscripts */ + indexes += MAXDIM; + for (int i = 0; i < sbsrefstate->numlower; i++) + { + if (sbsrefstate->lowerprovided[i]) + { + /* If any index expr yields NULL, result is NULL or error */ + if (sbsrefstate->lowerindexnull[i]) + { + if (sbsrefstate->isassignment) + ereport(ERROR, + (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), + errmsg("array subscript in assignment must not be null"))); + *op->resnull = true; + return false; + } + indexes[i] = DatumGetInt32(sbsrefstate->lowerindex[i]); + } + } return true; } @@ -3166,12 +3211,15 @@ ExecEvalSubscriptingRef(ExprState *state, ExprEvalStep *op) /* * Evaluate SubscriptingRef fetch. * - * Source container is in step's result variable. + * Source container is in step's result variable, + * and indexes have already been evaluated into workspace array. */ void ExecEvalSubscriptingRefFetch(ExprState *state, ExprEvalStep *op) { SubscriptingRefState *sbsrefstate = op->d.sbsref.state; + int *upperindex = (int *) sbsrefstate->workspace; + int *lowerindex = upperindex + MAXDIM; /* Should not get here if source container (or any subscript) is null */ Assert(!(*op->resnull)); @@ -3181,7 +3229,7 @@ ExecEvalSubscriptingRefFetch(ExprState *state, ExprEvalStep *op) /* Scalar case */ *op->resvalue = array_get_element(*op->resvalue, sbsrefstate->numupper, - sbsrefstate->upperindex, + upperindex, sbsrefstate->refattrlength, sbsrefstate->refelemlength, sbsrefstate->refelembyval, @@ -3193,8 +3241,8 @@ ExecEvalSubscriptingRefFetch(ExprState *state, ExprEvalStep *op) /* Slice case */ *op->resvalue = array_get_slice(*op->resvalue, sbsrefstate->numupper, - sbsrefstate->upperindex, - sbsrefstate->lowerindex, + upperindex, + lowerindex, sbsrefstate->upperprovided, sbsrefstate->lowerprovided, sbsrefstate->refattrlength, @@ -3214,6 +3262,8 @@ void ExecEvalSubscriptingRefOld(ExprState *state, ExprEvalStep *op) { SubscriptingRefState *sbsrefstate = op->d.sbsref.state; + int *upperindex = (int *) sbsrefstate->workspace; + int *lowerindex = upperindex + MAXDIM; if (*op->resnull) { @@ -3226,7 +3276,7 @@ ExecEvalSubscriptingRefOld(ExprState *state, ExprEvalStep *op) /* Scalar case */ sbsrefstate->prevvalue = array_get_element(*op->resvalue, sbsrefstate->numupper, - sbsrefstate->upperindex, + upperindex, sbsrefstate->refattrlength, sbsrefstate->refelemlength, sbsrefstate->refelembyval, @@ -3239,8 +3289,8 @@ ExecEvalSubscriptingRefOld(ExprState *state, ExprEvalStep *op) /* this is currently unreachable */ sbsrefstate->prevvalue = array_get_slice(*op->resvalue, sbsrefstate->numupper, - sbsrefstate->upperindex, - sbsrefstate->lowerindex, + upperindex, + lowerindex, sbsrefstate->upperprovided, sbsrefstate->lowerprovided, sbsrefstate->refattrlength, @@ -3260,7 +3310,9 @@ ExecEvalSubscriptingRefOld(ExprState *state, ExprEvalStep *op) void ExecEvalSubscriptingRefAssign(ExprState *state, ExprEvalStep *op) { - SubscriptingRefState *sbsrefstate = op->d.sbsref_subscript.state; + SubscriptingRefState *sbsrefstate = op->d.sbsref.state; + int *upperindex = (int *) sbsrefstate->workspace; + int *lowerindex = upperindex + MAXDIM; /* * For an assignment to a fixed-length container type, both the original @@ -3290,7 +3342,7 @@ ExecEvalSubscriptingRefAssign(ExprState *state, ExprEvalStep *op) /* Scalar case */ *op->resvalue = array_set_element(*op->resvalue, sbsrefstate->numupper, - sbsrefstate->upperindex, + upperindex, sbsrefstate->replacevalue, sbsrefstate->replacenull, sbsrefstate->refattrlength, @@ -3303,8 +3355,8 @@ ExecEvalSubscriptingRefAssign(ExprState *state, ExprEvalStep *op) /* Slice case */ *op->resvalue = array_set_slice(*op->resvalue, sbsrefstate->numupper, - sbsrefstate->upperindex, - sbsrefstate->lowerindex, + upperindex, + lowerindex, sbsrefstate->upperprovided, sbsrefstate->lowerprovided, sbsrefstate->replacevalue, diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index f232397cab..bc9b6771e3 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -1746,7 +1746,7 @@ llvm_compile_expr(ExprState *state) LLVMBuildBr(b, opblocks[opno + 1]); break; - case EEOP_SBSREF_SUBSCRIPT: + case EEOP_SBSREF_SUBSCRIPTS: { int jumpdone = op->d.sbsref_subscript.jumpdone; LLVMValueRef v_ret; diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c index a7ea7656c7..4c8a739bc4 100644 --- a/src/backend/utils/adt/arrayfuncs.c +++ b/src/backend/utils/adt/arrayfuncs.c @@ -2044,7 +2044,8 @@ array_get_element_expanded(Datum arraydatum, * array bound. * * NOTE: we assume it is OK to scribble on the provided subscript arrays - * lowerIndx[] and upperIndx[]. These are generally just temporaries. + * lowerIndx[] and upperIndx[]; also, these arrays must be of size MAXDIM + * even when nSubscripts is less. These are generally just temporaries. */ Datum array_get_slice(Datum arraydatum, @@ -2772,7 +2773,8 @@ array_set_element_expanded(Datum arraydatum, * (XXX TODO: allow a corresponding behavior for multidimensional arrays) * * NOTE: we assume it is OK to scribble on the provided index arrays - * lowerIndx[] and upperIndx[]. These are generally just temporaries. + * lowerIndx[] and upperIndx[]; also, these arrays must be of size MAXDIM + * even when nSubscripts is less. These are generally just temporaries. * * NOTE: For assignments, we throw an error for silly subscripts etc, * rather than returning a NULL or empty array as the fetch operations do. diff --git a/src/include/c.h b/src/include/c.h index b21e4074dd..12ea056a35 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -592,13 +592,9 @@ typedef uint32 CommandId; #define InvalidCommandId (~(CommandId)0) /* - * Array indexing support + * Maximum number of array subscripts, for regular varlena arrays */ #define MAXDIM 6 -typedef struct -{ - int indx[MAXDIM]; -} IntArray; /* ---------------- * Variable-length datatypes all share the 'struct varlena' header. diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index abb489e206..b768c30b74 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -185,8 +185,8 @@ typedef enum ExprEvalOp */ EEOP_FIELDSTORE_FORM, - /* Process a container subscript; short-circuit expression to NULL if NULL */ - EEOP_SBSREF_SUBSCRIPT, + /* Process container subscripts; possibly short-circuit result to NULL */ + EEOP_SBSREF_SUBSCRIPTS, /* * Compute old container element/slice when a SubscriptingRef assignment @@ -494,13 +494,11 @@ typedef struct ExprEvalStep int ncolumns; } fieldstore; - /* for EEOP_SBSREF_SUBSCRIPT */ + /* for EEOP_SBSREF_SUBSCRIPTS */ struct { /* too big to have inline */ struct SubscriptingRefState *state; - int off; /* 0-based index of this subscript */ - bool isupper; /* is it upper or lower subscript? */ int jumpdone; /* jump here on null */ } sbsref_subscript; @@ -646,20 +644,21 @@ typedef struct SubscriptingRefState bool refelembyval; /* is the element type pass-by-value? */ char refelemalign; /* typalign of the element type */ - /* numupper and upperprovided[] are filled at compile time */ - /* at runtime, extracted subscript datums get stored in upperindex[] */ + /* workspace for type-specific subscripting code */ + void *workspace; + + /* numupper and upperprovided[] are filled at expression compile time */ + /* at runtime, subscripts are computed in upperindex[]/upperindexnull[] */ int numupper; - bool upperprovided[MAXDIM]; - int upperindex[MAXDIM]; + bool *upperprovided; /* indicates if this position is supplied */ + Datum *upperindex; + bool *upperindexnull; /* similarly for lower indexes, if any */ int numlower; - bool lowerprovided[MAXDIM]; - int lowerindex[MAXDIM]; - - /* subscript expressions get evaluated into here */ - Datum subscriptvalue; - bool subscriptnull; + bool *lowerprovided; + Datum *lowerindex; + bool *lowerindexnull; /* for assignment, new value to assign is evaluated into here */ Datum replacevalue;