From: Antonin Houska <ah@cybertec.at>
To: pgsql-hackers@lists.postgresql.org
Subject: Logical replication row filtering and TOAST
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Date: Tue, 05 Apr 2022 11:50:55 +0200
Message-ID: <84638.1649152255@antos>
Archived-At: <https://www.postgresql.org/message-id/84638.1649152255%40antos>
Precedence: bulk

--=-=-=
Content-Type: text/plain

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


--=-=-=
Content-Type: text/x-diff
Content-Disposition: attachment;
 filename=logical_rep_row_filter_comment.diff

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);
 

--=-=-=--