Indirect assignment code for array slices is dead code?

Started by Andres Freundalmost 9 years ago3 messages
#1Andres Freund
andres@anarazel.de

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 FieldStore or 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 where the value would be needed, and an array assignment
* can't be within a CASE either. (So saving and restoring the
* caseValue is just paranoia, but let's do it anyway.)
*
* Since fetching the old element might 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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#1)
Re: Indirect assignment code for array slices is dead code?

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#2)
Re: Indirect assignment code for array slices is dead code?

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers