+ /* + * Unchanged toasted replica identity columns are only detoasted in the + * old tuple, copy this over to the new tuple. + */ + if (att->attlen == -1 && + VARATT_IS_EXTERNAL_ONDISK(tmp_new_slot->tts_values[i]) && + !VARATT_IS_EXTERNAL_ONDISK(old_slot->tts_values[i])) + { + if (tmp_new_slot == new_slot) + { + tmp_new_slot = MakeSingleTupleTableSlot(desc, &TTSOpsVirtual); + ExecClearTuple(tmp_new_slot); + ExecCopySlot(tmp_new_slot, new_slot); + } + + tmp_new_slot->tts_values[i] = old_slot->tts_values[i]; + tmp_new_slot->tts_isnull[i] = old_slot->tts_isnull[i]; + } + }
What is the need to assign new_slot to tmp_new_slot at the beginning of this part of the code? Can't we do this when we found some attribute that needs to be copied from the old tuple?
The other part which is not clear to me by looking at this code and comments is how do we ensure that we cover all cases where the new tuple doesn't have values?
IMHO, the only part we are trying to handle is when the toasted attribute is not modified in the new tuple. And if we notice the update WAL the new tuple is written as it is in the WAL which is getting inserted into the heap page. That means if it is external it can only be in
VARATT_IS_EXTERNAL_ONDISK format. So I don't think we need to worry about any intermediate format which we use for the in-memory tuples. Sometimes in reorder buffer we do use the INDIRECT format as well which internally can store ON DISK format but we don't need to worry about that case either because that is only true when we have the complete toast tuple as part of the WAL and we recreate the tuple in memory in reorder buffer, so even if it can by ON DISK format inside INDIRECT format but we have complete tuple.