Fix logical_decoding_timelines test crashes

In the test_slot_timelines test module, we were abusing passing NULL
values which was received as zeroes in x86, but this breaks in ARM
(buildfarm member hamster) by crashing instead.  Fix the breakage by
marking these functions as STRICT; the InvalidXid value that was
previously implicit in NULL values (on x86 at least) can now be passed
as 0.  Failing to follow the fmgr protocol to check for NULLs beforehand
was causing ARM to fail, as evidenced by segmentation faults in
buildfarm member hamster.

In order to use the new functionality in the test script, use COALESCE
in the right spot to avoid forwarding NULL values.

This was diagnosed from the hamster crash by Craig Ringer, who also
proposed a different patch (checking for NULL values explicitely in the
C function code, and keeping the non-strictness in the C functions).
I decided to go with this approach instead.

diff --git a/src/test/modules/test_slot_timelines/expected/load_extension.out b/src/test/modules/test_slot_timelines/expected/load_extension.out
index 14a414aa7ed5299bebf979d468258ae1e25dde94..7c2ad9d7c194d7998a26cbaa51bcadcfaf167f9f 100644 (file)
--- a/src/test/modules/test_slot_timelines/expected/load_extension.out
+++ b/src/test/modules/test_slot_timelines/expected/load_extension.out
@@ -5,7 +5,7 @@ SELECT test_slot_timelines_create_logical_slot('test_slot', 'test_decoding');
  
 (1 row)
 
-SELECT test_slot_timelines_advance_logical_slot('test_slot', txid_current(), txid_current(), pg_current_xlog_location(), pg_current_xlog_location());
+SELECT test_slot_timelines_advance_logical_slot('test_slot', txid_current()::text::xid, txid_current()::text::xid, pg_current_xlog_location(), pg_current_xlog_location());
  test_slot_timelines_advance_logical_slot 
 ------------------------------------------
  

diff --git a/src/test/modules/test_slot_timelines/sql/load_extension.sql b/src/test/modules/test_slot_timelines/sql/load_extension.sql
index a71127d47162b1e47487d5bde52e5b3ff50906b8..2440355246422db6c67c16440c412bc67164e2b3 100644 (file)
--- a/src/test/modules/test_slot_timelines/sql/load_extension.sql
+++ b/src/test/modules/test_slot_timelines/sql/load_extension.sql
@@ -2,6 +2,6 @@ CREATE EXTENSION test_slot_timelines;
 
 SELECT test_slot_timelines_create_logical_slot('test_slot', 'test_decoding');
 
-SELECT test_slot_timelines_advance_logical_slot('test_slot', txid_current(), txid_current(), pg_current_xlog_location(), pg_current_xlog_location());
+SELECT test_slot_timelines_advance_logical_slot('test_slot', txid_current()::text::xid, txid_current()::text::xid, pg_current_xlog_location(), pg_current_xlog_location());
 
 SELECT pg_drop_replication_slot('test_slot');

diff --git a/src/test/modules/test_slot_timelines/test_slot_timelines--1.0.sql b/src/test/modules/test_slot_timelines/test_slot_timelines--1.0.sql
index 31d7f8ef1c1dc56f9be1e7a4a9ca85ae27dfaa49..a1886f732f1b313f2dc3c64b508a41b3f749850c 100644 (file)
--- a/src/test/modules/test_slot_timelines/test_slot_timelines--1.0.sql
+++ b/src/test/modules/test_slot_timelines/test_slot_timelines--1.0.sql
@@ -3,14 +3,14 @@
 
 CREATE OR REPLACE FUNCTION test_slot_timelines_create_logical_slot(slot_name text, plugin text)
 RETURNS void
-LANGUAGE c AS 'MODULE_PATHNAME';
+STRICT LANGUAGE c AS 'MODULE_PATHNAME';
 
 COMMENT ON FUNCTION test_slot_timelines_create_logical_slot(text, text)
 IS 'Create a logical slot at a particular lsn and xid. Do not use in production servers, it is not safe. The slot is created with an invalid xmin and lsn.';
 
-CREATE OR REPLACE FUNCTION test_slot_timelines_advance_logical_slot(slot_name text, new_xmin bigint, new_catalog_xmin bigint, new_restart_lsn pg_lsn, new_confirmed_lsn pg_lsn)
+CREATE OR REPLACE FUNCTION test_slot_timelines_advance_logical_slot(slot_name text, new_xmin xid, new_catalog_xmin xid, new_restart_lsn pg_lsn, new_confirmed_lsn pg_lsn)
 RETURNS void
-LANGUAGE c AS 'MODULE_PATHNAME';
+STRICT LANGUAGE c AS 'MODULE_PATHNAME';
 
-COMMENT ON FUNCTION test_slot_timelines_advance_logical_slot(text, bigint, bigint, pg_lsn, pg_lsn)
+COMMENT ON FUNCTION test_slot_timelines_advance_logical_slot(text, xid, xid, pg_lsn, pg_lsn)
 IS 'Advance a logical slot directly. Do not use this in production servers, it is not safe.';

diff --git a/src/test/modules/test_slot_timelines/test_slot_timelines.c b/src/test/modules/test_slot_timelines/test_slot_timelines.c
index 74dd1a041b0efb2995eafcef17be559dce182355..1f074881d2f3dc6e02367c4eb95bd9446e1b3d36 100644 (file)
--- a/src/test/modules/test_slot_timelines/test_slot_timelines.c
+++ b/src/test/modules/test_slot_timelines/test_slot_timelines.c
@@ -85,8 +85,8 @@ Datum
 test_slot_timelines_advance_logical_slot(PG_FUNCTION_ARGS)
 {
        char       *slotname = text_to_cstring(PG_GETARG_TEXT_P(0));
-       TransactionId new_xmin = (TransactionId) PG_GETARG_INT64(1);
-       TransactionId new_catalog_xmin = (TransactionId) PG_GETARG_INT64(2);
+       TransactionId new_xmin = DatumGetTransactionId(PG_GETARG_DATUM(1));
+       TransactionId new_catalog_xmin = DatumGetTransactionId(PG_GETARG_DATUM(2));
        XLogRecPtr      restart_lsn = PG_GETARG_LSN(3);
        XLogRecPtr      confirmed_lsn = PG_GETARG_LSN(4);
 
@@ -95,7 +95,7 @@ test_slot_timelines_advance_logical_slot(PG_FUNCTION_ARGS)
        ReplicationSlotAcquire(slotname);
 
        if (MyReplicationSlot->data.database != MyDatabaseId)
-               elog(ERROR, "Trying to update a slot on a different database");
+               elog(ERROR, "trying to update a slot on a different database");
 
        MyReplicationSlot->data.xmin = new_xmin;
        MyReplicationSlot->data.catalog_xmin = new_catalog_xmin;

diff --git a/src/test/recovery/t/006_logical_decoding_timelines.pl b/src/test/recovery/t/006_logical_decoding_timelines.pl
index bc20f405d785fb449930e6648595a27450379eed..a3a4b6150ae782496cb144787770fc159e1e6d5b 100644 (file)
--- a/src/test/recovery/t/006_logical_decoding_timelines.pl
+++ b/src/test/recovery/t/006_logical_decoding_timelines.pl
@@ -172,8 +172,13 @@ is($stdout, '', 'No slots exist on the replica');
 # we're just doing it by hand for this test. This is exposing
 # postgres innards to SQL so it's unsafe except for testing.
 $node_master->safe_psql('postgres', 'CREATE EXTENSION test_slot_timelines;');
-my $slotinfo = $node_master->safe_psql('postgres',
-'SELECT slot_name, plugin, xmin, catalog_xmin, restart_lsn, confirmed_flush_lsn FROM pg_replication_slots ORDER BY slot_name'
+
+my $slotinfo = $node_master->safe_psql(
+       'postgres',
+       qq{SELECT slot_name, plugin,
+       COALESCE(xmin, '0'), catalog_xmin,
+       restart_lsn, confirmed_flush_lsn
+       FROM pg_replication_slots ORDER BY slot_name}
 );
 diag "Copying slots to replica";
 open my $fh, '<', \$slotinfo or die $!;
@@ -183,10 +188,7 @@ while (<$fh>)
        chomp $_;
        my ($slot_name, $plugin, $xmin, $catalog_xmin, $restart_lsn,
                $confirmed_flush_lsn)
-         = map {
-               if   ($_ ne '') { "'$_'" }
-               else            { 'NULL' }
-         } split qr/\|/, $_;
+         = map { "'$_'" } split qr/\|/, $_;
 
        print
 "# Copying slot $slot_name,$plugin,$xmin,$catalog_xmin,$restart_lsn,$confirmed_flush_lsn\n";
@@ -208,7 +210,7 @@ is( $stdout,
 
 $stdout = $node_replica->safe_psql(
        'postgres',
-       qq{SELECT slot_name, plugin, xmin, catalog_xmin,
+       qq{SELECT slot_name, plugin, COALESCE(xmin, '0'), catalog_xmin,
                          restart_lsn, confirmed_flush_lsn
                 FROM pg_replication_slots
         ORDER BY slot_name});
@@ -243,6 +245,7 @@ diag "oldest needed xlog seg is $oldest_needed_segment ";
 opendir(my $pg_xlog, $node_master->data_dir . "/pg_xlog") or die $!;
 while (my $seg = readdir $pg_xlog)
 {
+       next if $seg eq '.' or $seg eq '..';
        next unless $seg >= $oldest_needed_segment && $seg =~ /^[0-9]{24}/;
        diag "copying xlog seg $seg";
        copy(