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
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 20d0b1e1253..2be5aaa18c4 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -1246,6 +1246,15 @@ pgoutput_row_filter(Relation relation, TupleTableSlot *old_slot,
*/
if (!new_slot || !old_slot)
{
+ /*
+ * For UPDATE, we should only get here if the replica identity is !=
+ * FULL and thus all the identity columns are index columns, i.e.
+ * never TOASTed. Therefore, we don't need to care of the unchanged
+ * toasted replica identity columns like we do below.
+ */
+ Assert(relation->rd_rel->relreplident != REPLICA_IDENTITY_FULL ||
+ map_changetype_pubaction[*action] != PUBACTION_UPDATE);
+
ecxt->ecxt_scantuple = new_slot ? new_slot : old_slot;
result = pgoutput_row_filter_exec_expr(filter_exprstate, ecxt);
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
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 20d0b1e125..9d33630464 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -1237,10 +1237,11 @@ pgoutput_row_filter(Relation relation, TupleTableSlot *old_slot,
* For inserts, we only have the new tuple.
*
* For updates, we can have only a new tuple when none of the replica
- * identity columns changed but we still need to evaluate the row filter
- * for new tuple as the existing values of those columns might not match
- * the filter. Also, users can use constant expressions in the row filter,
- * so we anyway need to evaluate it for the new tuple.
+ * identity columns changed and none of those columns have external data
+ * but we still need to evaluate the row filter for the new tuple as the
+ * existing values of those columns might not match the filter. Also, users
+ * can use constant expressions in the row filter, so we anyway need to
+ * evaluate it for the new tuple.
*
* For deletes, we only have the old tuple.
*/
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