public inbox for [email protected]
help / color / mirror / Atom feedFrom: Vishal Prasanna <[email protected]>
To: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: "'Álvaro Herrera'" <[email protected]>
Cc: pgsql-bugs <[email protected]>
Subject: RE: [BUG] Assert failure in ReorderBufferReturnTXN during logical decoding due to leaked specinsert change
Date: Mon, 09 Mar 2026 02:44:03 +0530
Message-ID: <[email protected]> (raw)
In-Reply-To: <OS9PR01MB1214961799DAE95F5CB32EC75F57FA@OS9PR01MB12149.jpnprd01.prod.outlook.com>
References: <[email protected]>
<[email protected]>
<OS9PR01MB12149FD6C66A344831EBD836EF572A@OS9PR01MB12149.jpnprd01.prod.outlook.com>
<[email protected]>
<OS9PR01MB1214961799DAE95F5CB32EC75F57FA@OS9PR01MB12149.jpnprd01.prod.outlook.com>
Hi Hayato,
>> Found a workload that can reproduce the issue across all supported versions, except PG 14.
>>
>> For PG 14, since row_filter is not supported, so we can go with the 'publication does not exist' error instead.
>> Refer: PG14-Test-specinsert-cleanup-in-ReorderBufferProcessTXN-error-path.patch
>>
>> For PG 15 - 18, using the row_filter option we can cause an error in the logical decoder.
>
> Per my understanding, for PG16-, the provided workload cannot cause an assertion
> failure because it misses the Assert() in the ReorderBufferReturnTXN(), right?
> Adding the line is essential, otherwise the test could pass even without the fix.
Right. Added `Assert(txn->size == 0)` in `ReorderBufferReturnTXN()` for PG14, 15, 16 patches.
> 01.
> I think it is better to combine tests actual code patches into one. Because they
> would be done when patches are committed.
combined the fix and test into a single patch. since some changes are version specific
(like add assert check for PG14-16, different function names from PG17 -> 18, test case)
separate patches are provided per version. PG15 and PG16 share the same patch.
> 02.
> This issue can happen even on HEAD, but PG18-Fix-specinsert... cannot be applied
> atop the branch. Can you create it as well?
created a separate patch for master.
> 03. 100_bugs.pl
>
> Other tests start from the comment like "The bug...", but it does not follow.
> Can we update?
>
> 04. 100_bugs.pl
>
> Can we just rotate a log instead of starting new instance? It might be faster.
>
> 05. 100_bugs.pl for PG
>
> ```
> # The publication row filter WHERE ((a / 0) > 0) will trigger a division by zero error.
> ```
>
> I think the comment can be improved like:
>
> Create a publication with the zero-division row filter. It always throws an
> ERROR before publishing changes, when the filter is evaluated.
>
> Please see attached my top-up patch for PG18, it addresses comments 03-05.
Applied your suggestions across all versions. Thanks.
Overall:
PG14: Assert check + fix + test (uses 'pub does not exist' error)
PG15-16: Assert check + fix + test (uses row_filter error)
PG17: fix + test (uses row_filter error)
PG18, master: fix + test (uses row_filter error)
Regards,
Vishal Prasanna
Zoho Corporation
Attachments:
[application/octet-stream] master-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch (4.2K, 2-master-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch)
download | inline diff:
From 3bdb3b1afc0d8c1c647606c954023ad3552d305f Mon Sep 17 00:00:00 2001
From: Vishal Prasanna <[email protected]>
Date: Mon, 9 Mar 2026 01:11:30 +0530
Subject: [PATCH] Fix specinsert leak in ReorderBufferProcessTXN error path
---
.../replication/logical/reorderbuffer.c | 20 ++++-----
src/test/subscription/t/100_bugs.pl | 45 +++++++++++++++++++
2 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 4c230bcc8e4..c20c2635ab3 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2166,8 +2166,7 @@ static void
ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
Snapshot snapshot_now,
CommandId command_id,
- XLogRecPtr last_lsn,
- ReorderBufferChange *specinsert)
+ XLogRecPtr last_lsn)
{
/* Discard the changes that we just streamed */
ReorderBufferTruncateTXN(rb, txn, rbtxn_is_prepared(txn));
@@ -2175,13 +2174,6 @@ ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
/* Free all resources allocated for toast reconstruction */
ReorderBufferToastReset(rb, txn);
- /* Return the spec insert change if it is not NULL */
- if (specinsert != NULL)
- {
- ReorderBufferFreeChange(rb, specinsert, true);
- specinsert = NULL;
- }
-
/*
* For the streaming case, stop the stream and remember the command ID and
* snapshot for the streaming run.
@@ -2765,6 +2757,13 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
CurrentResourceOwner = cowner;
}
+ /* Free the specinsert change before freeing the ReorderBufferTXN */
+ if (specinsert != NULL)
+ {
+ ReorderBufferFreeChange(rb, specinsert, true);
+ specinsert = NULL;
+ }
+
/*
* The error code ERRCODE_TRANSACTION_ROLLBACK indicates a concurrent
* abort of the (sub)transaction we are streaming or preparing. We
@@ -2798,8 +2797,7 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
/* Reset the TXN so that it is allowed to stream remaining data. */
ReorderBufferResetTXN(rb, txn, snapshot_now,
- command_id, prev_lsn,
- specinsert);
+ command_id, prev_lsn);
}
else
{
diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index a23035e23fe..31dc63ae8c4 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -605,4 +605,49 @@ $node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
$node_publisher->stop('fast');
+# https://postgr.es/m/19c7623e882.4080fd5426212.311756747309556767%40zohocorp.com
+
+# The bug was that when an ERROR was raised while processing an INSERT ... ON
+# CONFLICT statement, the decoded change misses to be free'd. This can cause an
+# assertion failure if enabled.
+
+$node_publisher->rotate_logfile();
+$node_publisher->start();
+
+# Create a publication with the zero-division row filter. It always throws an
+# ERROR before publishing changes, when the filter is evaluated.
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE TABLE tab_upsert (a INT PRIMARY KEY, b INT);
+ CREATE PUBLICATION pub_rowfilter_error FOR TABLE tab_upsert WHERE ((a / 0) > 0);
+ SELECT * FROM pg_create_logical_replication_slot('upsert_slot', 'pgoutput');
+ INSERT INTO tab_upsert (a, b) VALUES (1, 1)
+ ON CONFLICT(a) DO UPDATE SET b = excluded.b;
+));
+
+# Decode the changes with a publication whose row filter causes a
+# division by zero error, and verify that the logical decoder doesn't crash.
+($ret, $stdout, $stderr) = $node_publisher->psql(
+ 'postgres', qq(
+ SELECT *
+ FROM pg_logical_slot_peek_binary_changes(
+ 'upsert_slot',
+ NULL,
+ NULL,
+ 'proto_version', '1',
+ 'publication_names', 'pub_rowfilter_error'
+ );
+));
+
+ok( $stderr =~ qr/division by zero/,
+ 'peek logical changes with row filter causing division by zero throws error'
+);
+
+# Clean up
+$node_publisher->safe_psql('postgres', "SELECT pg_drop_replication_slot('upsert_slot')");
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION pub_rowfilter_error");
+$node_publisher->safe_psql('postgres', "DROP TABLE tab_upsert");
+
+$node_publisher->stop('fast');
+
done_testing();
--
2.50.1 (Apple Git-155)
[application/octet-stream] PG14-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch (4.3K, 3-PG14-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch)
download | inline diff:
From b90143addb125dc180f23b65936999402591cc7b Mon Sep 17 00:00:00 2001
From: Vishal Prasanna <[email protected]>
Date: Sun, 8 Mar 2026 23:29:27 +0530
Subject: [PATCH] Fix specinsert leak in ReorderBufferProcessTXN error path
---
.../replication/logical/reorderbuffer.c | 23 +++++-----
src/test/subscription/t/100_bugs.pl | 43 ++++++++++++++++++-
2 files changed, 54 insertions(+), 12 deletions(-)
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index b708877d965..04d877d67ee 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -474,6 +474,9 @@ ReorderBufferReturnTXN(ReorderBuffer *rb, ReorderBufferTXN *txn)
/* Reset the toast hash */
ReorderBufferToastReset(rb, txn);
+ /* All changes must be deallocated */
+ Assert(txn->size == 0);
+
pfree(txn);
}
@@ -2029,8 +2032,7 @@ static void
ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
Snapshot snapshot_now,
CommandId command_id,
- XLogRecPtr last_lsn,
- ReorderBufferChange *specinsert)
+ XLogRecPtr last_lsn)
{
/* Discard the changes that we just streamed */
ReorderBufferTruncateTXN(rb, txn, rbtxn_prepared(txn));
@@ -2038,13 +2040,6 @@ ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
/* Free all resources allocated for toast reconstruction */
ReorderBufferToastReset(rb, txn);
- /* Return the spec insert change if it is not NULL */
- if (specinsert != NULL)
- {
- ReorderBufferReturnChange(rb, specinsert, true);
- specinsert = NULL;
- }
-
/*
* For the streaming case, stop the stream and remember the command ID and
* snapshot for the streaming run.
@@ -2589,6 +2584,13 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
if (using_subtxn)
RollbackAndReleaseCurrentSubTransaction();
+ /* Free the specinsert change before freeing the ReorderBufferTXN */
+ if (specinsert != NULL)
+ {
+ ReorderBufferReturnChange(rb, specinsert, true);
+ specinsert = NULL;
+ }
+
/*
* The error code ERRCODE_TRANSACTION_ROLLBACK indicates a concurrent
* abort of the (sub)transaction we are streaming or preparing. We
@@ -2615,8 +2617,7 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
/* Reset the TXN so that it is allowed to stream remaining data. */
ReorderBufferResetTXN(rb, txn, snapshot_now,
- command_id, prev_lsn,
- specinsert);
+ command_id, prev_lsn);
}
else
{
diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 235227c7727..81ff79fce46 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -6,7 +6,7 @@ use strict;
use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 11;
+use Test::More tests => 12;
# Bug #15114
@@ -392,3 +392,44 @@ like(
$node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
$node_publisher->stop('fast');
+
+# https://postgr.es/m/19c7623e882.4080fd5426212.311756747309556767%40zohocorp.com
+
+# The bug was that when an ERROR was raised while processing an INSERT ... ON
+# CONFLICT statement, the decoded change misses to be free'd. This can cause an
+# assertion failure if enabled.
+
+$node_publisher->rotate_logfile();
+$node_publisher->start();
+
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE TABLE tab_upsert (a INT PRIMARY KEY, b INT);
+ SELECT * FROM pg_create_logical_replication_slot('upsert_slot', 'pgoutput');
+ INSERT INTO tab_upsert (a, b) VALUES (1, 1)
+ ON CONFLICT(a) DO UPDATE SET b = excluded.b;
+));
+
+# Decode the changes without a publication and
+# verify that the logical decoder doesn't crash.
+($ret, $stdout, $stderr) = $node_publisher->psql(
+ 'postgres', qq(
+ SELECT *
+ FROM pg_logical_slot_peek_binary_changes(
+ 'upsert_slot',
+ NULL,
+ NULL,
+ 'proto_version', '1',
+ 'publication_names', 'pub_that_does_not_exist'
+ );
+));
+
+ok( $stderr =~ qr/publication "pub_that_does_not_exist" does not exist/,
+ 'peek logical changes with non-existent publication throws error'
+);
+
+# Clean up
+$node_publisher->safe_psql('postgres', "SELECT pg_drop_replication_slot('upsert_slot')");
+$node_publisher->safe_psql('postgres', "DROP TABLE tab_upsert");
+
+$node_publisher->stop('fast');
--
2.50.1 (Apple Git-155)
[application/octet-stream] PG15-16-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch (4.5K, 4-PG15-16-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch)
download | inline diff:
From 6cfbfa82b8a31c365de29395758558e7a50d3f46 Mon Sep 17 00:00:00 2001
From: Vishal Prasanna <[email protected]>
Date: Sun, 8 Mar 2026 23:49:30 +0530
Subject: [PATCH] Fix specinsert leak in ReorderBufferProcessTXN error path
---
.../replication/logical/reorderbuffer.c | 23 +++++-----
src/test/subscription/t/100_bugs.pl | 45 +++++++++++++++++++
2 files changed, 57 insertions(+), 11 deletions(-)
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 59efa73930f..2fad1c1cf38 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -478,6 +478,9 @@ ReorderBufferReturnTXN(ReorderBuffer *rb, ReorderBufferTXN *txn)
/* Reset the toast hash */
ReorderBufferToastReset(rb, txn);
+ /* All changes must be deallocated */
+ Assert(txn->size == 0);
+
pfree(txn);
}
@@ -2033,8 +2036,7 @@ static void
ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
Snapshot snapshot_now,
CommandId command_id,
- XLogRecPtr last_lsn,
- ReorderBufferChange *specinsert)
+ XLogRecPtr last_lsn)
{
/* Discard the changes that we just streamed */
ReorderBufferTruncateTXN(rb, txn, rbtxn_prepared(txn));
@@ -2042,13 +2044,6 @@ ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
/* Free all resources allocated for toast reconstruction */
ReorderBufferToastReset(rb, txn);
- /* Return the spec insert change if it is not NULL */
- if (specinsert != NULL)
- {
- ReorderBufferReturnChange(rb, specinsert, true);
- specinsert = NULL;
- }
-
/*
* For the streaming case, stop the stream and remember the command ID and
* snapshot for the streaming run.
@@ -2592,6 +2587,13 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
if (using_subtxn)
RollbackAndReleaseCurrentSubTransaction();
+ /* Free the specinsert change before freeing the ReorderBufferTXN */
+ if (specinsert != NULL)
+ {
+ ReorderBufferReturnChange(rb, specinsert, true);
+ specinsert = NULL;
+ }
+
/*
* The error code ERRCODE_TRANSACTION_ROLLBACK indicates a concurrent
* abort of the (sub)transaction we are streaming or preparing. We
@@ -2618,8 +2620,7 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
/* Reset the TXN so that it is allowed to stream remaining data. */
ReorderBufferResetTXN(rb, txn, snapshot_now,
- command_id, prev_lsn,
- specinsert);
+ command_id, prev_lsn);
}
else
{
diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 061eb2a32c1..eee297efa2b 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -448,4 +448,49 @@ $node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
$node_publisher->stop('fast');
+# https://postgr.es/m/19c7623e882.4080fd5426212.311756747309556767%40zohocorp.com
+
+# The bug was that when an ERROR was raised while processing an INSERT ... ON
+# CONFLICT statement, the decoded change misses to be free'd. This can cause an
+# assertion failure if enabled.
+
+$node_publisher->rotate_logfile();
+$node_publisher->start();
+
+# Create a publication with the zero-division row filter. It always throws an
+# ERROR before publishing changes, when the filter is evaluated.
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE TABLE tab_upsert (a INT PRIMARY KEY, b INT);
+ CREATE PUBLICATION pub_rowfilter_error FOR TABLE tab_upsert WHERE ((a / 0) > 0);
+ SELECT * FROM pg_create_logical_replication_slot('upsert_slot', 'pgoutput');
+ INSERT INTO tab_upsert (a, b) VALUES (1, 1)
+ ON CONFLICT(a) DO UPDATE SET b = excluded.b;
+));
+
+# Decode the changes with a publication whose row filter causes a
+# division by zero error, and verify that the logical decoder doesn't crash.
+($ret, $stdout, $stderr) = $node_publisher->psql(
+ 'postgres', qq(
+ SELECT *
+ FROM pg_logical_slot_peek_binary_changes(
+ 'upsert_slot',
+ NULL,
+ NULL,
+ 'proto_version', '1',
+ 'publication_names', 'pub_rowfilter_error'
+ );
+));
+
+ok( $stderr =~ qr/division by zero/,
+ 'peek logical changes with row filter causing division by zero throws error'
+);
+
+# Clean up
+$node_publisher->safe_psql('postgres', "SELECT pg_drop_replication_slot('upsert_slot')");
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION pub_rowfilter_error");
+$node_publisher->safe_psql('postgres', "DROP TABLE tab_upsert");
+
+$node_publisher->stop('fast');
+
done_testing();
--
2.50.1 (Apple Git-155)
[application/octet-stream] PG17-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch (4.3K, 5-PG17-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch)
download | inline diff:
From dd95d7488fc6b6dcb2964504ea5e28d566839060 Mon Sep 17 00:00:00 2001
From: Vishal Prasanna <[email protected]>
Date: Mon, 9 Mar 2026 00:19:29 +0530
Subject: [PATCH] Fix specinsert leak in ReorderBufferProcessTXN error path
---
.../replication/logical/reorderbuffer.c | 20 ++++-----
src/test/subscription/t/100_bugs.pl | 45 +++++++++++++++++++
2 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 4bd1f7af061..5134abe9359 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2081,8 +2081,7 @@ static void
ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
Snapshot snapshot_now,
CommandId command_id,
- XLogRecPtr last_lsn,
- ReorderBufferChange *specinsert)
+ XLogRecPtr last_lsn)
{
/* Discard the changes that we just streamed */
ReorderBufferTruncateTXN(rb, txn, rbtxn_prepared(txn));
@@ -2090,13 +2089,6 @@ ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
/* Free all resources allocated for toast reconstruction */
ReorderBufferToastReset(rb, txn);
- /* Return the spec insert change if it is not NULL */
- if (specinsert != NULL)
- {
- ReorderBufferReturnChange(rb, specinsert, true);
- specinsert = NULL;
- }
-
/*
* For the streaming case, stop the stream and remember the command ID and
* snapshot for the streaming run.
@@ -2663,6 +2655,13 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
if (using_subtxn)
RollbackAndReleaseCurrentSubTransaction();
+ /* Free the specinsert change before freeing the ReorderBufferTXN */
+ if (specinsert != NULL)
+ {
+ ReorderBufferReturnChange(rb, specinsert, true);
+ specinsert = NULL;
+ }
+
/*
* The error code ERRCODE_TRANSACTION_ROLLBACK indicates a concurrent
* abort of the (sub)transaction we are streaming or preparing. We
@@ -2689,8 +2688,7 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
/* Reset the TXN so that it is allowed to stream remaining data. */
ReorderBufferResetTXN(rb, txn, snapshot_now,
- command_id, prev_lsn,
- specinsert);
+ command_id, prev_lsn);
}
else
{
diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 17accd11d93..7d804e5ff9c 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -597,4 +597,49 @@ $node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
$node_publisher->stop('fast');
+# https://postgr.es/m/19c7623e882.4080fd5426212.311756747309556767%40zohocorp.com
+
+# The bug was that when an ERROR was raised while processing an INSERT ... ON
+# CONFLICT statement, the decoded change misses to be free'd. This can cause an
+# assertion failure if enabled.
+
+$node_publisher->rotate_logfile();
+$node_publisher->start();
+
+# Create a publication with the zero-division row filter. It always throws an
+# ERROR before publishing changes, when the filter is evaluated.
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE TABLE tab_upsert (a INT PRIMARY KEY, b INT);
+ CREATE PUBLICATION pub_rowfilter_error FOR TABLE tab_upsert WHERE ((a / 0) > 0);
+ SELECT * FROM pg_create_logical_replication_slot('upsert_slot', 'pgoutput');
+ INSERT INTO tab_upsert (a, b) VALUES (1, 1)
+ ON CONFLICT(a) DO UPDATE SET b = excluded.b;
+));
+
+# Decode the changes with a publication whose row filter causes a
+# division by zero error, and verify that the logical decoder doesn't crash.
+($ret, $stdout, $stderr) = $node_publisher->psql(
+ 'postgres', qq(
+ SELECT *
+ FROM pg_logical_slot_peek_binary_changes(
+ 'upsert_slot',
+ NULL,
+ NULL,
+ 'proto_version', '1',
+ 'publication_names', 'pub_rowfilter_error'
+ );
+));
+
+ok( $stderr =~ qr/division by zero/,
+ 'peek logical changes with row filter causing division by zero throws error'
+);
+
+# Clean up
+$node_publisher->safe_psql('postgres', "SELECT pg_drop_replication_slot('upsert_slot')");
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION pub_rowfilter_error");
+$node_publisher->safe_psql('postgres', "DROP TABLE tab_upsert");
+
+$node_publisher->stop('fast');
+
done_testing();
--
2.50.1 (Apple Git-155)
[application/octet-stream] PG18-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch (4.3K, 6-PG18-Fix-specinsert-leak-in-ReorderBufferProcessTXN-error-path.patch)
download | inline diff:
From 64deae15dc57ac7083fd5d1967f0b8a83e648b56 Mon Sep 17 00:00:00 2001
From: Vishal Prasanna <[email protected]>
Date: Mon, 9 Mar 2026 00:46:12 +0530
Subject: [PATCH] Fix specinsert leak in ReorderBufferProcessTXN error path
---
.../replication/logical/reorderbuffer.c | 20 ++++-----
src/test/subscription/t/100_bugs.pl | 45 +++++++++++++++++++
2 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 11139a910b8..57e1ae2441b 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2164,8 +2164,7 @@ static void
ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
Snapshot snapshot_now,
CommandId command_id,
- XLogRecPtr last_lsn,
- ReorderBufferChange *specinsert)
+ XLogRecPtr last_lsn)
{
/* Discard the changes that we just streamed */
ReorderBufferTruncateTXN(rb, txn, rbtxn_is_prepared(txn));
@@ -2173,13 +2172,6 @@ ReorderBufferResetTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
/* Free all resources allocated for toast reconstruction */
ReorderBufferToastReset(rb, txn);
- /* Return the spec insert change if it is not NULL */
- if (specinsert != NULL)
- {
- ReorderBufferFreeChange(rb, specinsert, true);
- specinsert = NULL;
- }
-
/*
* For the streaming case, stop the stream and remember the command ID and
* snapshot for the streaming run.
@@ -2753,6 +2745,13 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
if (using_subtxn)
RollbackAndReleaseCurrentSubTransaction();
+ /* Free the specinsert change before freeing the ReorderBufferTXN */
+ if (specinsert != NULL)
+ {
+ ReorderBufferFreeChange(rb, specinsert, true);
+ specinsert = NULL;
+ }
+
/*
* The error code ERRCODE_TRANSACTION_ROLLBACK indicates a concurrent
* abort of the (sub)transaction we are streaming or preparing. We
@@ -2786,8 +2785,7 @@ ReorderBufferProcessTXN(ReorderBuffer *rb, ReorderBufferTXN *txn,
/* Reset the TXN so that it is allowed to stream remaining data. */
ReorderBufferResetTXN(rb, txn, snapshot_now,
- command_id, prev_lsn,
- specinsert);
+ command_id, prev_lsn);
}
else
{
diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 50223054918..df6b2e1296d 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -605,4 +605,49 @@ $node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
$node_publisher->stop('fast');
+# https://postgr.es/m/19c7623e882.4080fd5426212.311756747309556767%40zohocorp.com
+
+# The bug was that when an ERROR was raised while processing an INSERT ... ON
+# CONFLICT statement, the decoded change misses to be free'd. This can cause an
+# assertion failure if enabled.
+
+$node_publisher->rotate_logfile();
+$node_publisher->start();
+
+# Create a publication with the zero-division row filter. It always throws an
+# ERROR before publishing changes, when the filter is evaluated.
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE TABLE tab_upsert (a INT PRIMARY KEY, b INT);
+ CREATE PUBLICATION pub_rowfilter_error FOR TABLE tab_upsert WHERE ((a / 0) > 0);
+ SELECT * FROM pg_create_logical_replication_slot('upsert_slot', 'pgoutput');
+ INSERT INTO tab_upsert (a, b) VALUES (1, 1)
+ ON CONFLICT(a) DO UPDATE SET b = excluded.b;
+));
+
+# Decode the changes with a publication whose row filter causes a
+# division by zero error, and verify that the logical decoder doesn't crash.
+($ret, $stdout, $stderr) = $node_publisher->psql(
+ 'postgres', qq(
+ SELECT *
+ FROM pg_logical_slot_peek_binary_changes(
+ 'upsert_slot',
+ NULL,
+ NULL,
+ 'proto_version', '1',
+ 'publication_names', 'pub_rowfilter_error'
+ );
+));
+
+ok( $stderr =~ qr/division by zero/,
+ 'peek logical changes with row filter causing division by zero throws error'
+);
+
+# Clean up
+$node_publisher->safe_psql('postgres', "SELECT pg_drop_replication_slot('upsert_slot')");
+$node_publisher->safe_psql('postgres', "DROP PUBLICATION pub_rowfilter_error");
+$node_publisher->safe_psql('postgres', "DROP TABLE tab_upsert");
+
+$node_publisher->stop('fast');
+
done_testing();
--
2.50.1 (Apple Git-155)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: RE: [BUG] Assert failure in ReorderBufferReturnTXN during logical decoding due to leaked specinsert change
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox