Thread: [HACKERS] Indirect assignment code for array slices is dead code?
Hi,
In the context of my expression evaluation patch, I was trying to
increase test coverage of execQual.c. I'm a bit confused about
$subject. ExecEvalArrayRef() has the following codepath:
if (isAssignment) { Datum sourceData; Datum save_datum; bool save_isNull;
/* * We might have a nested-assignment situation, in which the * refassgnexpr is itself a
FieldStoreor ArrayRef 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 econtext's
caseValue. It's safe to * reuse the CASE mechanism because there cannot be a CASE between * here and
wherethe value would be needed, and an array assignment * can't be within a CASE either. (So saving and
restoringthe * caseValue is just paranoia, but let's do it anyway.) * * Since fetching the old
elementmight be a nontrivial expense, do it * only if the argument appears to actually need it. */
save_datum= econtext->caseValue_datum; save_isNull = econtext->caseValue_isNull;
if (isAssignmentIndirectionExpr(astate->refassgnexpr)) if (*isNull) { ...
} else if (lIndex == NULL) { ... } else {
econtext->caseValue_datum = array_get_slice(array_source, i,
upper.indx,lower.indx, upperProvided, lowerProvided,
astate->refattrlength, astate->refelemlength,
astate->refelembyval, astate->refelemalign); econtext->caseValue_isNull
=false; } }
...
That's support for multiple indirect assignments, assigning to array
slices. But I can't figure out how to reach that bit of code.
The !slice code can be reached:
CREATE TABLE arr(d int[]);
CREATE TABLE arr2(arr arr)
INSERT INTO arr2(arr[1].d, arr[2].d) VALUES(ARRAY[1,2],ARRAY[3,4]) RETURNING *
┌───────────────────────────────┐
│ arr │
├───────────────────────────────┤
│ {"(\"{1,2}\")","(\"{3,4}\")"} │
└───────────────────────────────┘
But I don't see how to the slice code can be reached. The indirection
code is only triggered when isAssignmentIndirectionExpr is true, which
requires that refassgnexpr is a FieldStore/ArrayRef containing a
CaseTest.
But that doesn't make sense for slices, because there can't be a
FieldStore directly below a slice (you'd get "subfield "d" is of type
integer[] but expression is of type integer" - makes sense), nor can
there be an ArrayRef inside a slice ArrayRef for assignemnts, because
there are no parens in the LHS, and consecutive []'s are collapsed into
a single ArrayRef.
Am I standing on my own foot here?
Greetings,
Andres Freund
Andres Freund <andres@anarazel.de> writes:
> In the context of my expression evaluation patch, I was trying to
> increase test coverage of execQual.c. I'm a bit confused about
> $subject. ExecEvalArrayRef() has the following codepath:
I think it may indeed be unreachable at present, because we don't support
something like this:
regression=# create table tt (f1 complex[]);
CREATE TABLE
regression=# update tt set f1[2:3].r = array[7,11];
ERROR: cannot assign to field "r" of column "f1" because its type complex[] is not a composite type
LINE 1: update tt set f1[2:3].r = array[7,11]; ^
regression=# update tt set f1[2:3].r = array[7,11];
ERROR: cannot assign to field "r" of column "f1" because its type complex[] is not a composite type
LINE 1: update tt set f1[2:3].r = 42; ^
I would not like to remove it though. This particular bit of the executor
has no business making assumptions about how array and field references
can be nested.
regards, tom lane
Hi, On 2017-03-10 20:59:48 -0500, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > In the context of my expression evaluation patch, I was trying to > > increase test coverage of execQual.c. I'm a bit confused about > > $subject. ExecEvalArrayRef() has the following codepath: > > I think it may indeed be unreachable at present, because we don't support > something like this: > > regression=# create table tt (f1 complex[]); > CREATE TABLE > regression=# update tt set f1[2:3].r = array[7,11]; > ERROR: cannot assign to field "r" of column "f1" because its type complex[] is not a composite type > LINE 1: update tt set f1[2:3].r = array[7,11]; > ^ > > regression=# update tt set f1[2:3].r = array[7,11]; > ERROR: cannot assign to field "r" of column "f1" because its type complex[] is not a composite type > LINE 1: update tt set f1[2:3].r = 42; Yea, that's where I got too. > I would not like to remove it though. This particular bit of the executor > has no business making assumptions about how array and field references > can be nested. Not planning to - I am just trying to make sure I get the corner cases right - if there had been a way to reach that bit of code, I'd have like to understand how, before screwing around. Thanks for looking, Andres