Assign TupleTableSlot->tts_tableOid duplicated in tale AM.

From: Wenchao Zhang <zwcpostgres(at)gmail(dot)com>
To: andres <andres(at)anarazel(dot)de>
Cc: pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, dilipbalaut <dilipbalaut(at)gmail(dot)com>, pg <pg(at)bowt(dot)ie>
Subject: Assign TupleTableSlot->tts_tableOid duplicated in tale AM.
Date: 2022-09-21 06:51:02
Message-ID: CAEyNz8rNMNz5+YoPXq50fmFaEJtiL3YySmL9Dk0YJETne8fxtQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi hackers,

Recently, we discover that the field of tts_tableOid of TupleTableSlot is
assigned duplicated in table AM's interface which is not necessary. For
example, in table_scan_getnextslot,

```
static inline bool
table_scan_getnextslot(TableScanDesc sscan, ScanDirection direction,
TupleTableSlot *slot)
{
slot->tts_tableOid = RelationGetRelid(sscan->rs_rd);

/*
* We don't expect direct calls to table_scan_getnextslot with valid
* CheckXidAlive for catalog or regular tables. See detailed
comments in
* xact.c where these variables are declared.
*/
if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan))
elog(ERROR, "unexpected table_scan_getnextslot call during
logical decoding");

return sscan->rs_rd->rd_tableam->scan_getnextslot(sscan, direction,
slot);
}
```

we can see that it assigns tts_tableOid, while calling
sscan->rs_rd->rd_tableam->scan_getnextslot which implemented by
heap_getnextslot also assigns tts_tableOid in the call of
ExecStoreBufferHeapTuple.

```
TupleTableSlot *
ExecStoreBufferHeapTuple(HeapTuple tuple,
TupleTableSlot *slot,
Buffer buffer)
{
/*
* sanity checks
*/
Assert(tuple != NULL);
Assert(slot != NULL);
Assert(slot->tts_tupleDescriptor != NULL);
Assert(BufferIsValid(buffer));

if (unlikely(!TTS_IS_BUFFERTUPLE(slot)))
elog(ERROR, "trying to store an on-disk heap tuple into
wrong type of slot");
tts_buffer_heap_store_tuple(slot, tuple, buffer, false);

slot->tts_tableOid = tuple->t_tableOid;

return slot;
}
```

We can get the two assigned values are same by reading codes. Maybe we
should remove one?

What's more, we find that maybe we assign slot->tts_tableOid in outer
interface like table_scan_getnextslot may be better and more friendly when
we import other pluggable storage formats. It can avoid duplicated
assignments in every implementation of table AM's interfaces.

Regards,
Wenchao

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2022-09-21 06:57:41 Re: Refactor backup related code (was: Is it correct to say, "invalid data in file \"%s\"", BACKUP_LABEL_FILE in do_pg_backup_stop?)
Previous Message Marina Polyakova 2022-09-21 06:50:55 Re: ICU for global collation