On Fri, 27 Oct 2023 at 22:05, Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > > On Fri, Oct 27, 2023 at 8:48 AM David Rowley <dgrowle...@gmail.com> wrote: > > I was uncertain if the old behaviour of when srcslot contains fewer > > attributes than dstslot was intended or not. What happens there is > > that we'd leave the additional old dstslot tts_values in place and > > only overwrite up to srcslot->natts but then we'd go on and > > materialize all the dstslot attributes. I think this might not be > > needed as we do dstslot->tts_nvalid = srcdesc->natts. I suspect we may > > be ok just to materialize the srcslot attributes and ignore the > > previous additional dstslot attributes. Changing it to that would > > make the attached patch more simple. > > We seem to use both tt_nvalid and tts_tupleDescriptor->natts. I forgot > what's the difference. If we do what you say, we might end up trying > to access unmaterialized values beyond tts_nvalid. Better to > investigate whether such a hazard exists.
The TupleDesc's natts is the number of attributes in the tuple descriptor. tts_nvalid is the greatest attribute number that's been deformed in the tuple slot. For slot types other than virtual slots, we'll call slot_getsomeattrs() to deform more attributes from the tuple. The reason the code in question looks suspicious to me is that we do "dstslot->tts_nvalid = srcdesc->natts;" and there's no way to deform more attributes in a virtual slot. Note that tts_virtual_getsomeattrs() unconditionally does elog(ERROR, "getsomeattrs is not required to be called on a virtual tuple table slot");. We shouldn't ever be accessing tts_values elements above what tts_nvalid is set to, so either we should be setting dstslot->tts_nvalid = to the dstdesc->natts so that we can access tts_values elements above srcdesc->natts or we're needlessly materialising too many attributes in tts_virtual_copyslot(). David David
