Re: Logical replication row filtering and TOAST

On Tue, Apr 5, 2022 at 3:52 PM Antonin Houska <ah@cybertec.at> wrote:
>
> Antonin Houska <ah@cybertec.at> wrote:
>
> > I spent some time thinking about a special case of evaluation of the row
> > filter and wrote a comment that might be useful (see the attachment). However
> > now I think that it's not perfect if the code really relies on the fact that
> > value of an indexed column cannot be TOASTed due to size restrictions.
> >
> > I could hit two different error messages when trying activate TOAST on an
> > index column (in this case PG was build with 16kB pages), but still I think
> > the code is unnecessarily fragile if it relies on such errors:
> >
> >
> > ERROR:  index row requires 8224 bytes, maximum size is 8191
> >
> > ERROR:  index row size 8048 exceeds btree version 4 maximum 5432 for index "b_pkey"
> > DETAIL:  Index row references tuple (0,3) in relation "b".
> > HINT:  Values larger than 1/3 of a buffer page cannot be indexed.
> >
> >
> > Note that at least in ExtractReplicaIdentity() we do expect that an indexed
> > column value can be TOASTed.
> >
> >       /*
> >        * If the tuple, which by here only contains indexed columns, still has
> >        * toasted columns, force them to be inlined. This is somewhat unlikely
> >        * since there's limits on the size of indexed columns, so we don't
> >        * duplicate toast_flatten_tuple()s functionality in the above loop over
> >        * the indexed columns, even if it would be more efficient.
> >        */
> >       if (HeapTupleHasExternal(key_tuple))
> >       {
> >               HeapTuple       oldtup = key_tuple;
> >
> >               key_tuple = toast_flatten_tuple(oldtup, desc);
> >               heap_freetuple(oldtup);
> >       }
> >
> > Do I miss anything?
>
> Well, I see now that the point might be that, in heap_update(),
> "id_has_external" would be true the indexed value could be TOASTed, so that
> the (flattened) old tuple would be WAL logged:
>

Right.

>
> Nevertheless, a comment in pgoutput_row_filter(), saying that TOASTed values
> are not expected if old_slot is NULL, might be useful.
>

How about something like the attached?

-- 
With Regards,
Amit Kapila.