Logical replication row filtering and TOAST
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
Attachments:
logical_rep_row_filter_comment.difftext/x-diffDownload+9-0
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
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.
Attachments:
update_toast_comment_1.patchapplication/octet-stream; name=update_toast_comment_1.patchDownload+5-4
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
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