Re: Something seems weird inside tts_virtual_copyslot()

[David Rowley]

On Fri, 1 Dec 2023 at 13:14, Andres Freund <and...@anarazel.de> wrote:
> So I think adding an assert to ExecCopySlot(), perhaps with a comment saying
> that the restriction could be lifted with a bit of work, would be fine.

Thanks for looking at this again.

How about the attached?  I wrote the comment you mentioned and also
removed the Assert from tts_virtual_copyslot().

I also noted in the copyslot callback declaration that implementers
can assume the number of attributes in the source and destination
slots match.

David

diff --git a/src/backend/executor/execTuples.c 
b/src/backend/executor/execTuples.c
index 2c2712ceac..5a2db83987 100644
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -253,8 +253,6 @@ tts_virtual_copyslot(TupleTableSlot *dstslot, 
TupleTableSlot *srcslot)
 {
        TupleDesc       srcdesc = srcslot->tts_tupleDescriptor;
 
-       Assert(srcdesc->natts <= dstslot->tts_tupleDescriptor->natts);
-
        tts_virtual_clear(dstslot);
 
        slot_getallattrs(srcslot);
diff --git a/src/include/executor/tuptable.h b/src/include/executor/tuptable.h
index 4210d6d838..ce96111865 100644
--- a/src/include/executor/tuptable.h
+++ b/src/include/executor/tuptable.h
@@ -174,7 +174,8 @@ struct TupleTableSlotOps
 
        /*
         * Copy the contents of the source slot into the destination slot's own
-        * context. Invoked using callback of the destination slot.
+        * context. Invoked using callback of the destination slot.  'dstslot' 
and
+        * 'srcslot' can be assumed to have the same number of attributes.
         */
        void            (*copyslot) (TupleTableSlot *dstslot, TupleTableSlot 
*srcslot);
 
@@ -477,12 +478,19 @@ ExecCopySlotMinimalTuple(TupleTableSlot *slot)
  *
  * If a source's system attributes are supposed to be accessed in the target
  * slot, the target slot and source slot types need to match.
+ *
+ * Currently, source and target slot's TupleDesc must have the same number of
+ * attributes.  Future work could see this relaxed to allow the source to
+ * contain additional attributes and have the code here only copy over the
+ * leading attributes.
  */
 static inline TupleTableSlot *
 ExecCopySlot(TupleTableSlot *dstslot, TupleTableSlot *srcslot)
 {
        Assert(!TTS_EMPTY(srcslot));
        Assert(srcslot != dstslot);
+       Assert(dstslot->tts_tupleDescriptor->natts ==
+                  srcslot->tts_tupleDescriptor->natts);
 
        dstslot->tts_ops->copyslot(dstslot, srcslot);