Looking at the Assert inside tts_virtual_copyslot(), it does:
Assert(srcdesc->natts <= dstslot->tts_tupleDescriptor->natts);
So, that seems to indicate that it's ok for the src slot to have fewer
attributes than the destination. The code then calls
tts_virtual_clear(dstslot), then slot_getallattrs(srcslot); then does
the following loop:
for (int natt = 0; natt < srcdesc->natts; natt++)
{
dstslot->tts_values[natt] = srcslot->tts_values[natt];
dstslot->tts_isnull[natt] = srcslot->tts_isnull[natt];
}
Seems ok so far. If the srcslot has fewer attributes then that'll
leave the extra dstslot array elements untouched.
Where it gets weird is inside tts_virtual_materialize(). In that
function, we materialize *all* of the dstslot attributes, even the
extra ones that were left alone in the for loop shown above. Why do
we need to materialize all of those attributes? We only need to
materialize up to srcslot->natts.
Per the following code, only up to the srcdesc->natts would be
accessible anyway:
dstslot->tts_nvalid = srcdesc->natts;
Virtual slots don't need any further deforming and
tts_virtual_getsomeattrs() is coded in a way that we'll find out if
anything tries to deform a virtual slot.
I changed the Assert in tts_virtual_copyslot() to check the natts
match in each of the slots and all of the regression tests still pass,
so it seems we have no tests where there's an attribute number
mismatch...
I wondered if there are any other cases that try to handle mismatching
attribute numbers. On a quick scan of git grep -E
"^\s*Assert\(.*natts.*\);" I don't see any other Asserts that allow
mismatching attribute numbers.
I think if we are going to support copying slots where the source and
destination don't have the same number of attributes then the
following comment should explain what's allowed and what's not
allowed:
/*
* Copy the contents of the source slot into the destination slot's own
* context. Invoked using callback of the destination slot.
*/
void (*copyslot) (TupleTableSlot *dstslot, TupleTableSlot *srcslot);
I also tried adding the following to ExecCopySlot() to see if there is
any other slot copying going on with other slot types where the natts
don't match. All tests pass still.
Assert(srcslot->tts_tupleDescriptor->natts ==
dstslot->tts_tupleDescriptor->natts);
Is the Assert() in tts_virtual_copyslot() wrong?
David
