On 1 April 2016 at 12:47, Craig Ringer <cr...@2ndquadrant.com> wrote:
> I'll prep a follow-up patch.
>
>
Done and attached.
Note that I can't use PG_GETARG_TRANSACTIONID directly since it's a macro
defined only in xid.c . It didn't seem worth extracting it and moving it to
postgres.h (where the other non-ADT-specific PG_GETARG_ macros are) or its
own new header just for this, so I've spelled it out each time.
I now remember that that's part of why I used bigint as an argument type.
The other part is that txid_current() returns bigint and there's no cast
from bigint to xid. So the tests would have to CREATE CAST or cast via
'text'. They now do the latter.
We should probably have a cast from bigint to/from xid, but the type is so
little-used that it's not much fuss.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From 5e63a5f8e467c7855ab22fa7e5fbdac94d9c87e7 Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Fri, 1 Apr 2016 13:36:31 +0800
Subject: [PATCH] Fix test_slot_timelines buildfarm failure on 32-bit ARM
The code was reading a NULL 64-bit Datum that is pass-by-value
on 64-bit platforms but pass-by-reference on 32-bit. On 64-bit
this just produces zero, but on 32-bit it'll dereference a NULL
pointer. Since InvalidXLogRecPtr is zero this went unnoticed.
Test for NULL arguments.
In the process, switch to taking 'xid'-typed arguments instead of
bigint. Since txid_current() returns bigint and there's no cast from
bigint to xid this requires a cast through 'text' in our tests.
---
.../expected/load_extension.out | 2 +-
.../test_slot_timelines/sql/load_extension.sql | 2 +-
.../test_slot_timelines--1.0.sql | 4 +--
.../test_slot_timelines/test_slot_timelines.c | 31 +++++++++++++++++++---
4 files changed, 31 insertions(+), 8 deletions(-)
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 14a414a..7c2ad9d 100644
--- 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 a71127d..2440355 100644
--- 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 31d7f8e..50449bf 100644
--- 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
@@ -8,9 +8,9 @@ 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';
-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 74dd1a0..e818705 100644
--- a/src/test/modules/test_slot_timelines/test_slot_timelines.c
+++ b/src/test/modules/test_slot_timelines/test_slot_timelines.c
@@ -85,10 +85,33 @@ 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);
- XLogRecPtr restart_lsn = PG_GETARG_LSN(3);
- XLogRecPtr confirmed_lsn = PG_GETARG_LSN(4);
+ TransactionId new_xmin;
+ TransactionId new_catalog_xmin;
+ XLogRecPtr restart_lsn;
+ XLogRecPtr confirmed_lsn;
+
+ if (PG_ARGISNULL(0))
+ elog(ERROR, "slot name may not be null");
+
+ if (PG_ARGISNULL(1))
+ new_xmin = InvalidTransactionId;
+ else
+ new_xmin = (TransactionId) DatumGetTransactionId(PG_GETARG_DATUM(1));
+
+ if (PG_ARGISNULL(2))
+ new_catalog_xmin = InvalidTransactionId;
+ else
+ new_catalog_xmin = (TransactionId) DatumGetTransactionId(PG_GETARG_DATUM(2));
+
+ if (PG_ARGISNULL(3))
+ restart_lsn = InvalidXLogRecPtr;
+ else
+ restart_lsn = PG_GETARG_LSN(3);
+
+ if (PG_ARGISNULL(4))
+ confirmed_lsn = InvalidXLogRecPtr;
+ else
+ confirmed_lsn = PG_GETARG_LSN(4);
CheckSlotRequirements();
--
2.1.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
