Thread
Commits
-
Improve comments for row filtering and toast interaction in logical replication.
- 2d09e44d309f 15.0 landed
-
Logical replication row filtering and TOAST
Antonin Houska <ah@cybertec.at> — 2022-04-05T09:50:55Z
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? -- Antonin Houska Web: https://www.cybertec-postgresql.com -
Re: Logical replication row filtering and TOAST
Antonin Houska <ah@cybertec.at> — 2022-04-05T10:22:16Z
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: old_key_tuple = ExtractReplicaIdentity(relation, &oldtup, bms_overlap(modified_attrs, id_attrs) || id_has_external, &old_key_copied); Nevertheless, a comment in pgoutput_row_filter(), saying that TOASTed values are not expected if old_slot is NULL, might be useful. -- Antonin Houska Web: https://www.cybertec-postgresql.com -
Re: Logical replication row filtering and TOAST
Amit Kapila <amit.kapila16@gmail.com> — 2022-04-05T10:32:05Z
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. -
Re: Logical replication row filtering and TOAST
Antonin Houska <ah@cybertec.at> — 2022-04-05T11:59:30Z
Amit Kapila <amit.kapila16@gmail.com> wrote: > > Antonin Houska <ah@cybertec.at> wrote: > > > > 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? Yes, that'd be sufficient. Thanks. -- Antonin Houska Web: https://www.cybertec-postgresql.com
-
Re: Logical replication row filtering and TOAST
Ajin Cherian <itsajin@gmail.com> — 2022-04-06T01:50:50Z
On Tue, Apr 5, 2022 at 8:32 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > How about something like the attached? > LGTM. regards, Ajin Cherian Fujitsu Australia
-
Re: Logical replication row filtering and TOAST
Amit Kapila <amit.kapila16@gmail.com> — 2022-04-06T04:33:52Z
On Wed, Apr 6, 2022 at 7:21 AM Ajin Cherian <itsajin@gmail.com> wrote: > > On Tue, Apr 5, 2022 at 8:32 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > > How about something like the attached? > > > > LGTM. > Pushed. -- With Regards, Amit Kapila.