Dear Ajin,
Here are my comments. I must play with patches to understand more.
01.
```
extern bool ReorderBufferFilterByRelFileLocator(ReorderBuffer *rb, TransactionId xid,
XLogRecPtr lsn, RelFileLocator *rlocator,
ReorderBufferChangeType change_type,
bool has_tuple);
```
Can you explain why "has_tuple is needed? All callers is set to true.
02.
```
static Relation
ReorderBufferGetRelation(ReorderBuffer *rb, RelFileLocator *rlocator,
bool has_tuple)
```
Hmm, the naming is bit confusing for me. This operation is mostly not related with
the reorder buffer. How about "GetPossibleDecodableRelation" or something?
03.
```
if (IsToastRelation(relation))
{
Oid real_reloid = InvalidOid;
char *toast_name = RelationGetRelationName(relation);
/* pg_toast_ len is 9 */
char *start_ch = &toast_name[9];
real_reloid = pg_strtoint32(start_ch);
entry->relid = real_reloid;
}
```
It is bit hacky for me. How about using sscanf like attached?
04.
IIUC, toast tables always require the filter_change() call twice, is it right?
I understood like below:
1. ReorderBufferFilterByRelFileLocator() tries to filter the change at outside the
transaction. The OID indicates the pg_toast_xxx table.
2. pgoutput_filter_change() cannot find the table from the hash. It returns false
with cache_valid=false.
3. ReorderBufferFilterByRelFileLocator() starts a transaction and get its relation.
4. The function recognizes the relation seems toast and get parent oid.
5. The function tries to filter the change in the transaction, with the parent oid.
6. pgoutput_filter_change()->get_rel_sync_entry() enters the parent relation to the
hash and return determine the filtable or not.
7. After sometime, the same table is modified. But the toast table is not stored in
the hash so that whole 1-6 steps are required.
I feel this may affect the perfomance when many toast is modified. How about skiping
the check for toasted ones? ISTM IsToastNamespace() can be used for the decision.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
