typedef struct LogicalDecodingContext
Hi,
During a recent code review, I noticed a lot of 'struct
LogicalDecodingContext' usage.
There are many function prototypes where the params are (for no
apparent reason to me) a mixture of structs and typedef structs.
AFAICT just by pre-declaring the typedef struct
LogicalDecodingContext, all of those 'struct LogicalDecodingContext'
can be culled, resulting in cleaner and more consistent function
signatures.
The PG Docs were similarly modified.
PSA patch for this. It passes make check-world.
(I recognize this is potentially the tip of an iceberg. If this patch
is deemed OK, I can hunt down similar underuse of typedefs for other
structs)
Thoughts?
------
Kind Regards,
Peter Smith.
Fujitsu Australia
Attachments:
v1-0001-Use-typedef-struct-LogicalDecodingContext.patchapplication/octet-stream; name=v1-0001-Use-typedef-struct-LogicalDecodingContext.patchDownload
From 108a867c425deeeeb5bf10b8813e6fd1b5d9cdac Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Wed, 1 Mar 2023 17:52:38 +1100
Subject: [PATCH v1] Use typedef struct LogicalDecodingContext
Make better use of the pre-declared typed struct so that function prototypes
using LogicalDecodingContext no longer need to say 'struct LogicalDecodingContext'.
---
doc/src/sgml/custom-rmgr.sgml | 2 +-
doc/src/sgml/logicaldecoding.sgml | 42 ++++++++++++------------
src/backend/replication/logical/logical.c | 6 ++--
src/backend/replication/pgoutput/pgoutput.c | 16 ++++-----
src/include/access/xlog_internal.h | 4 +--
src/include/replication/logical.h | 10 +++---
src/include/replication/output_plugin.h | 50 ++++++++++++++---------------
7 files changed, 65 insertions(+), 65 deletions(-)
diff --git a/doc/src/sgml/custom-rmgr.sgml b/doc/src/sgml/custom-rmgr.sgml
index baf86b1..353777a 100644
--- a/doc/src/sgml/custom-rmgr.sgml
+++ b/doc/src/sgml/custom-rmgr.sgml
@@ -52,7 +52,7 @@ typedef struct RmgrData
void (*rm_startup) (void);
void (*rm_cleanup) (void);
void (*rm_mask) (char *pagedata, BlockNumber blkno);
- void (*rm_decode) (struct LogicalDecodingContext *ctx,
+ void (*rm_decode) (LogicalDecodingContext *ctx,
struct XLogRecordBuffer *buf);
} RmgrData;
</programlisting>
diff --git a/doc/src/sgml/logicaldecoding.sgml b/doc/src/sgml/logicaldecoding.sgml
index 4e912b4..8e0ee00 100644
--- a/doc/src/sgml/logicaldecoding.sgml
+++ b/doc/src/sgml/logicaldecoding.sgml
@@ -597,7 +597,7 @@ CREATE TABLE another_catalog_table(data text) WITH (user_catalog_table = true);
a replication slot is created or asked to stream changes, independent
of the number of changes that are ready to be put out.
<programlisting>
-typedef void (*LogicalDecodeStartupCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStartupCB) (LogicalDecodingContext *ctx,
OutputPluginOptions *options,
bool is_init);
</programlisting>
@@ -639,7 +639,7 @@ typedef struct OutputPluginOptions
be used to deallocate resources private to the output plugin. The slot
isn't necessarily being dropped, streaming is just being stopped.
<programlisting>
-typedef void (*LogicalDecodeShutdownCB) (struct LogicalDecodingContext *ctx);
+typedef void (*LogicalDecodeShutdownCB) (LogicalDecodingContext *ctx);
</programlisting>
</para>
</sect3>
@@ -652,7 +652,7 @@ typedef void (*LogicalDecodeShutdownCB) (struct LogicalDecodingContext *ctx);
start of a committed transaction has been decoded. Aborted transactions
and their contents never get decoded.
<programlisting>
-typedef void (*LogicalDecodeBeginCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeBeginCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn);
</programlisting>
The <parameter>txn</parameter> parameter contains meta information about
@@ -671,7 +671,7 @@ typedef void (*LogicalDecodeBeginCB) (struct LogicalDecodingContext *ctx,
rows will have been called before this, if there have been any modified
rows.
<programlisting>
-typedef void (*LogicalDecodeCommitCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeCommitCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr commit_lsn);
</programlisting>
@@ -695,7 +695,7 @@ typedef void (*LogicalDecodeCommitCB) (struct LogicalDecodingContext *ctx,
this very same transaction. In that case, the logical decoding of this
aborted transaction is stopped gracefully.
<programlisting>
-typedef void (*LogicalDecodeChangeCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeChangeCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
Relation relation,
ReorderBufferChange *change);
@@ -726,7 +726,7 @@ typedef void (*LogicalDecodeChangeCB) (struct LogicalDecodingContext *ctx,
The optional <function>truncate_cb</function> callback is called for a
<command>TRUNCATE</command> command.
<programlisting>
-typedef void (*LogicalDecodeTruncateCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeTruncateCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
int nrelations,
Relation relations[],
@@ -750,7 +750,7 @@ typedef void (*LogicalDecodeTruncateCB) (struct LogicalDecodingContext *ctx,
from <parameter>origin_id</parameter> is of interest to the
output plugin.
<programlisting>
-typedef bool (*LogicalDecodeFilterByOriginCB) (struct LogicalDecodingContext *ctx,
+typedef bool (*LogicalDecodeFilterByOriginCB) (LogicalDecodingContext *ctx,
RepOriginId origin_id);
</programlisting>
The <parameter>ctx</parameter> parameter has the same contents
@@ -777,7 +777,7 @@ typedef bool (*LogicalDecodeFilterByOriginCB) (struct LogicalDecodingContext *ct
The optional <function>message_cb</function> callback is called whenever
a logical decoding message has been decoded.
<programlisting>
-typedef void (*LogicalDecodeMessageCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeMessageCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr message_lsn,
bool transactional,
@@ -824,7 +824,7 @@ typedef void (*LogicalDecodeMessageCB) (struct LogicalDecodingContext *ctx,
defined, <literal>false</literal> is assumed (i.e. no filtering, all
transactions using two-phase commit are decoded in two phases as well).
<programlisting>
-typedef bool (*LogicalDecodeFilterPrepareCB) (struct LogicalDecodingContext *ctx,
+typedef bool (*LogicalDecodeFilterPrepareCB) (LogicalDecodingContext *ctx,
TransactionId xid,
const char *gid);
</programlisting>
@@ -855,7 +855,7 @@ typedef bool (*LogicalDecodeFilterPrepareCB) (struct LogicalDecodingContext *ctx
in which case it can either error out or skip the remaining changes of
the transaction.
<programlisting>
-typedef void (*LogicalDecodeBeginPrepareCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeBeginPrepareCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn);
</programlisting>
</para>
@@ -872,7 +872,7 @@ typedef void (*LogicalDecodeBeginPrepareCB) (struct LogicalDecodingContext *ctx,
rows. The <parameter>gid</parameter> field, which is part of the
<parameter>txn</parameter> parameter, can be used in this callback.
<programlisting>
-typedef void (*LogicalDecodePrepareCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodePrepareCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr prepare_lsn);
</programlisting>
@@ -888,7 +888,7 @@ typedef void (*LogicalDecodePrepareCB) (struct LogicalDecodingContext *ctx,
The <parameter>gid</parameter> field, which is part of the
<parameter>txn</parameter> parameter, can be used in this callback.
<programlisting>
-typedef void (*LogicalDecodeCommitPreparedCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeCommitPreparedCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr commit_lsn);
</programlisting>
@@ -910,7 +910,7 @@ typedef void (*LogicalDecodeCommitPreparedCB) (struct LogicalDecodingContext *ct
<parameter>gid</parameter> alone is not sufficient because the downstream
node can have a prepared transaction with same identifier.
<programlisting>
-typedef void (*LogicalDecodeRollbackPreparedCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeRollbackPreparedCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr prepare_end_lsn,
TimestampTz prepare_time);
@@ -924,7 +924,7 @@ typedef void (*LogicalDecodeRollbackPreparedCB) (struct LogicalDecodingContext *
The required <function>stream_start_cb</function> callback is called when
opening a block of streamed changes from an in-progress transaction.
<programlisting>
-typedef void (*LogicalDecodeStreamStartCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamStartCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn);
</programlisting>
</para>
@@ -936,7 +936,7 @@ typedef void (*LogicalDecodeStreamStartCB) (struct LogicalDecodingContext *ctx,
The required <function>stream_stop_cb</function> callback is called when
closing a block of streamed changes from an in-progress transaction.
<programlisting>
-typedef void (*LogicalDecodeStreamStopCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamStopCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn);
</programlisting>
</para>
@@ -948,7 +948,7 @@ typedef void (*LogicalDecodeStreamStopCB) (struct LogicalDecodingContext *ctx,
The required <function>stream_abort_cb</function> callback is called to
abort a previously streamed transaction.
<programlisting>
-typedef void (*LogicalDecodeStreamAbortCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamAbortCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr abort_lsn);
</programlisting>
@@ -963,7 +963,7 @@ typedef void (*LogicalDecodeStreamAbortCB) (struct LogicalDecodingContext *ctx,
callback is required when the output plugin supports both the streaming
of large in-progress transactions and two-phase commits.
<programlisting>
-typedef void (*LogicalDecodeStreamPrepareCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamPrepareCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr prepare_lsn);
</programlisting>
@@ -976,7 +976,7 @@ typedef void (*LogicalDecodeStreamPrepareCB) (struct LogicalDecodingContext *ctx
The required <function>stream_commit_cb</function> callback is called to
commit a previously streamed transaction.
<programlisting>
-typedef void (*LogicalDecodeStreamCommitCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamCommitCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr commit_lsn);
</programlisting>
@@ -992,7 +992,7 @@ typedef void (*LogicalDecodeStreamCommitCB) (struct LogicalDecodingContext *ctx,
The actual changes are not displayed as the transaction can abort at a later
point in time and we don't decode changes for aborted transactions.
<programlisting>
-typedef void (*LogicalDecodeStreamChangeCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamChangeCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
Relation relation,
ReorderBufferChange *change);
@@ -1010,7 +1010,7 @@ typedef void (*LogicalDecodeStreamChangeCB) (struct LogicalDecodingContext *ctx,
can abort at a later point in time and we don't decode changes for aborted
transactions.
<programlisting>
-typedef void (*LogicalDecodeStreamMessageCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamMessageCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr message_lsn,
bool transactional,
@@ -1029,7 +1029,7 @@ typedef void (*LogicalDecodeStreamMessageCB) (struct LogicalDecodingContext *ctx
(demarcated by <function>stream_start_cb</function> and
<function>stream_stop_cb</function> calls).
<programlisting>
-typedef void (*LogicalDecodeStreamTruncateCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamTruncateCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
int nrelations,
Relation relations[],
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index c3ec97a..88de062 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -659,7 +659,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx)
* Prepare a write using the context's output routine.
*/
void
-OutputPluginPrepareWrite(struct LogicalDecodingContext *ctx, bool last_write)
+OutputPluginPrepareWrite(LogicalDecodingContext *ctx, bool last_write)
{
if (!ctx->accept_writes)
elog(ERROR, "writes are only accepted in commit, begin and change callbacks");
@@ -672,7 +672,7 @@ OutputPluginPrepareWrite(struct LogicalDecodingContext *ctx, bool last_write)
* Perform a write using the context's output routine.
*/
void
-OutputPluginWrite(struct LogicalDecodingContext *ctx, bool last_write)
+OutputPluginWrite(LogicalDecodingContext *ctx, bool last_write)
{
if (!ctx->prepared_write)
elog(ERROR, "OutputPluginPrepareWrite needs to be called before OutputPluginWrite");
@@ -685,7 +685,7 @@ OutputPluginWrite(struct LogicalDecodingContext *ctx, bool last_write)
* Update progress tracking (if supported).
*/
void
-OutputPluginUpdateProgress(struct LogicalDecodingContext *ctx,
+OutputPluginUpdateProgress(LogicalDecodingContext *ctx,
bool skipped_xact)
{
if (!ctx->update_progress)
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 0df1acb..f09f643 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -66,14 +66,14 @@ static void pgoutput_rollback_prepared_txn(LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr prepare_end_lsn,
TimestampTz prepare_time);
-static void pgoutput_stream_start(struct LogicalDecodingContext *ctx,
+static void pgoutput_stream_start(LogicalDecodingContext *ctx,
ReorderBufferTXN *txn);
-static void pgoutput_stream_stop(struct LogicalDecodingContext *ctx,
+static void pgoutput_stream_stop(LogicalDecodingContext *ctx,
ReorderBufferTXN *txn);
-static void pgoutput_stream_abort(struct LogicalDecodingContext *ctx,
+static void pgoutput_stream_abort(LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr abort_lsn);
-static void pgoutput_stream_commit(struct LogicalDecodingContext *ctx,
+static void pgoutput_stream_commit(LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr commit_lsn);
static void pgoutput_stream_prepare_txn(LogicalDecodingContext *ctx,
@@ -1809,7 +1809,7 @@ publication_invalidation_cb(Datum arg, int cacheid, uint32 hashvalue)
* START STREAM callback
*/
static void
-pgoutput_stream_start(struct LogicalDecodingContext *ctx,
+pgoutput_stream_start(LogicalDecodingContext *ctx,
ReorderBufferTXN *txn)
{
bool send_replication_origin = txn->origin_id != InvalidRepOriginId;
@@ -1840,7 +1840,7 @@ pgoutput_stream_start(struct LogicalDecodingContext *ctx,
* STOP STREAM callback
*/
static void
-pgoutput_stream_stop(struct LogicalDecodingContext *ctx,
+pgoutput_stream_stop(LogicalDecodingContext *ctx,
ReorderBufferTXN *txn)
{
/* we should be streaming a trasanction */
@@ -1859,7 +1859,7 @@ pgoutput_stream_stop(struct LogicalDecodingContext *ctx,
* it's subtransactions, if it's a toplevel transaction).
*/
static void
-pgoutput_stream_abort(struct LogicalDecodingContext *ctx,
+pgoutput_stream_abort(LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr abort_lsn)
{
@@ -1892,7 +1892,7 @@ pgoutput_stream_abort(struct LogicalDecodingContext *ctx,
* it's subtransactions).
*/
static void
-pgoutput_stream_commit(struct LogicalDecodingContext *ctx,
+pgoutput_stream_commit(LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr commit_lsn)
{
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index 8edae7b..a9dbf32 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -325,7 +325,7 @@ typedef enum
RECOVERY_TARGET_ACTION_SHUTDOWN
} RecoveryTargetAction;
-struct LogicalDecodingContext;
+typedef struct LogicalDecodingContext LogicalDecodingContext;
struct XLogRecordBuffer;
/*
@@ -354,7 +354,7 @@ typedef struct RmgrData
void (*rm_startup) (void);
void (*rm_cleanup) (void);
void (*rm_mask) (char *pagedata, BlockNumber blkno);
- void (*rm_decode) (struct LogicalDecodingContext *ctx,
+ void (*rm_decode) (LogicalDecodingContext *ctx,
struct XLogRecordBuffer *buf);
} RmgrData;
diff --git a/src/include/replication/logical.h b/src/include/replication/logical.h
index 5f49554..279bb6f 100644
--- a/src/include/replication/logical.h
+++ b/src/include/replication/logical.h
@@ -14,9 +14,9 @@
#include "replication/output_plugin.h"
#include "replication/slot.h"
-struct LogicalDecodingContext;
+typedef struct LogicalDecodingContext LogicalDecodingContext;
-typedef void (*LogicalOutputPluginWriterWrite) (struct LogicalDecodingContext *lr,
+typedef void (*LogicalOutputPluginWriterWrite) (LogicalDecodingContext *lr,
XLogRecPtr Ptr,
TransactionId xid,
bool last_write
@@ -24,13 +24,13 @@ typedef void (*LogicalOutputPluginWriterWrite) (struct LogicalDecodingContext *l
typedef LogicalOutputPluginWriterWrite LogicalOutputPluginWriterPrepareWrite;
-typedef void (*LogicalOutputPluginWriterUpdateProgress) (struct LogicalDecodingContext *lr,
+typedef void (*LogicalOutputPluginWriterUpdateProgress) (LogicalDecodingContext *lr,
XLogRecPtr Ptr,
TransactionId xid,
bool skipped_xact
);
-typedef struct LogicalDecodingContext
+struct LogicalDecodingContext
{
/* memory context this is all allocated in */
MemoryContext context;
@@ -109,7 +109,7 @@ typedef struct LogicalDecodingContext
TransactionId write_xid;
/* Are we processing the end LSN of a transaction? */
bool end_xact;
-} LogicalDecodingContext;
+};
extern void CheckLogicalDecodingRequirements(void);
diff --git a/src/include/replication/output_plugin.h b/src/include/replication/output_plugin.h
index 2d89d26..6772935 100644
--- a/src/include/replication/output_plugin.h
+++ b/src/include/replication/output_plugin.h
@@ -11,7 +11,7 @@
#include "replication/reorderbuffer.h"
-struct LogicalDecodingContext;
+typedef struct LogicalDecodingContext LogicalDecodingContext;
struct OutputPluginCallbacks;
typedef enum OutputPluginOutputType
@@ -44,7 +44,7 @@ extern PGDLLEXPORT void _PG_output_plugin_init(struct OutputPluginCallbacks *cb)
* "is_init" will be set to "true" if the decoding slot just got defined. When
* the same slot is used from there one, it will be "false".
*/
-typedef void (*LogicalDecodeStartupCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStartupCB) (LogicalDecodingContext *ctx,
OutputPluginOptions *options,
bool is_init);
@@ -52,13 +52,13 @@ typedef void (*LogicalDecodeStartupCB) (struct LogicalDecodingContext *ctx,
* Callback called for every (explicit or implicit) BEGIN of a successful
* transaction.
*/
-typedef void (*LogicalDecodeBeginCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeBeginCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn);
/*
* Callback for every individual change in a successful transaction.
*/
-typedef void (*LogicalDecodeChangeCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeChangeCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
Relation relation,
ReorderBufferChange *change);
@@ -66,7 +66,7 @@ typedef void (*LogicalDecodeChangeCB) (struct LogicalDecodingContext *ctx,
/*
* Callback for every TRUNCATE in a successful transaction.
*/
-typedef void (*LogicalDecodeTruncateCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeTruncateCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
int nrelations,
Relation relations[],
@@ -75,14 +75,14 @@ typedef void (*LogicalDecodeTruncateCB) (struct LogicalDecodingContext *ctx,
/*
* Called for every (explicit or implicit) COMMIT of a successful transaction.
*/
-typedef void (*LogicalDecodeCommitCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeCommitCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr commit_lsn);
/*
* Called for the generic logical decoding messages.
*/
-typedef void (*LogicalDecodeMessageCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeMessageCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr message_lsn,
bool transactional,
@@ -93,13 +93,13 @@ typedef void (*LogicalDecodeMessageCB) (struct LogicalDecodingContext *ctx,
/*
* Filter changes by origin.
*/
-typedef bool (*LogicalDecodeFilterByOriginCB) (struct LogicalDecodingContext *ctx,
+typedef bool (*LogicalDecodeFilterByOriginCB) (LogicalDecodingContext *ctx,
RepOriginId origin_id);
/*
* Called to shutdown an output plugin.
*/
-typedef void (*LogicalDecodeShutdownCB) (struct LogicalDecodingContext *ctx);
+typedef void (*LogicalDecodeShutdownCB) (LogicalDecodingContext *ctx);
/*
* Called before decoding of PREPARE record to decide whether this
@@ -107,35 +107,35 @@ typedef void (*LogicalDecodeShutdownCB) (struct LogicalDecodingContext *ctx);
* commit_prepared/rollback_prepared callbacks or wait till COMMIT PREPARED
* and sent as usual transaction.
*/
-typedef bool (*LogicalDecodeFilterPrepareCB) (struct LogicalDecodingContext *ctx,
+typedef bool (*LogicalDecodeFilterPrepareCB) (LogicalDecodingContext *ctx,
TransactionId xid,
const char *gid);
/*
* Callback called for every BEGIN of a prepared trnsaction.
*/
-typedef void (*LogicalDecodeBeginPrepareCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeBeginPrepareCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn);
/*
* Called for PREPARE record unless it was filtered by filter_prepare()
* callback.
*/
-typedef void (*LogicalDecodePrepareCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodePrepareCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr prepare_lsn);
/*
* Called for COMMIT PREPARED.
*/
-typedef void (*LogicalDecodeCommitPreparedCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeCommitPreparedCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr commit_lsn);
/*
* Called for ROLLBACK PREPARED.
*/
-typedef void (*LogicalDecodeRollbackPreparedCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeRollbackPreparedCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr prepare_end_lsn,
TimestampTz prepare_time);
@@ -146,7 +146,7 @@ typedef void (*LogicalDecodeRollbackPreparedCB) (struct LogicalDecodingContext *
* transaction (may be called repeatedly, if it's streamed in multiple
* chunks).
*/
-typedef void (*LogicalDecodeStreamStartCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamStartCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn);
/*
@@ -154,14 +154,14 @@ typedef void (*LogicalDecodeStreamStartCB) (struct LogicalDecodingContext *ctx,
* transaction to a remote node (may be called repeatedly, if it's streamed
* in multiple chunks).
*/
-typedef void (*LogicalDecodeStreamStopCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamStopCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn);
/*
* Called to discard changes streamed to remote node from in-progress
* transaction.
*/
-typedef void (*LogicalDecodeStreamAbortCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamAbortCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr abort_lsn);
@@ -169,7 +169,7 @@ typedef void (*LogicalDecodeStreamAbortCB) (struct LogicalDecodingContext *ctx,
* Called to prepare changes streamed to remote node from in-progress
* transaction. This is called as part of a two-phase commit.
*/
-typedef void (*LogicalDecodeStreamPrepareCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamPrepareCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr prepare_lsn);
@@ -177,14 +177,14 @@ typedef void (*LogicalDecodeStreamPrepareCB) (struct LogicalDecodingContext *ctx
* Called to apply changes streamed to remote node from in-progress
* transaction.
*/
-typedef void (*LogicalDecodeStreamCommitCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamCommitCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr commit_lsn);
/*
* Callback for streaming individual changes from in-progress transactions.
*/
-typedef void (*LogicalDecodeStreamChangeCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamChangeCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
Relation relation,
ReorderBufferChange *change);
@@ -193,7 +193,7 @@ typedef void (*LogicalDecodeStreamChangeCB) (struct LogicalDecodingContext *ctx,
* Callback for streaming generic logical decoding messages from in-progress
* transactions.
*/
-typedef void (*LogicalDecodeStreamMessageCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamMessageCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
XLogRecPtr message_lsn,
bool transactional,
@@ -204,7 +204,7 @@ typedef void (*LogicalDecodeStreamMessageCB) (struct LogicalDecodingContext *ctx
/*
* Callback for streaming truncates from in-progress transactions.
*/
-typedef void (*LogicalDecodeStreamTruncateCB) (struct LogicalDecodingContext *ctx,
+typedef void (*LogicalDecodeStreamTruncateCB) (LogicalDecodingContext *ctx,
ReorderBufferTXN *txn,
int nrelations,
Relation relations[],
@@ -243,8 +243,8 @@ typedef struct OutputPluginCallbacks
} OutputPluginCallbacks;
/* Functions in replication/logical/logical.c */
-extern void OutputPluginPrepareWrite(struct LogicalDecodingContext *ctx, bool last_write);
-extern void OutputPluginWrite(struct LogicalDecodingContext *ctx, bool last_write);
-extern void OutputPluginUpdateProgress(struct LogicalDecodingContext *ctx, bool skipped_xact);
+extern void OutputPluginPrepareWrite(LogicalDecodingContext *ctx, bool last_write);
+extern void OutputPluginWrite(LogicalDecodingContext *ctx, bool last_write);
+extern void OutputPluginUpdateProgress(LogicalDecodingContext *ctx, bool skipped_xact);
#endif /* OUTPUT_PLUGIN_H */
--
1.8.3.1
Peter Smith <smithpb2250@gmail.com> writes:
AFAICT just by pre-declaring the typedef struct
LogicalDecodingContext, all of those 'struct LogicalDecodingContext'
can be culled, resulting in cleaner and more consistent function
signatures.
Sadly, this is almost certainly going to cause bitching on the part of
some compilers, because depending on the order of header inclusions
they are going to see multiple typedefs for the same name. Redundant
"struct foo" declarations are portable C, but redundant "typedef foo"
not so much.
I also wonder if this passes headerscheck and cpluspluscheck.
regards, tom lane
On Thu, Mar 2, 2023 at 10:04 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Smith <smithpb2250@gmail.com> writes:
AFAICT just by pre-declaring the typedef struct
LogicalDecodingContext, all of those 'struct LogicalDecodingContext'
can be culled, resulting in cleaner and more consistent function
signatures.Sadly, this is almost certainly going to cause bitching on the part of
some compilers, because depending on the order of header inclusions
they are going to see multiple typedefs for the same name. Redundant
"struct foo" declarations are portable C, but redundant "typedef foo"
not so much.
Bah, I should not have used that tip-of-an-iceberg metaphor; it turns
out I was actually standing on the ship...
So does your reply mean there is no way really to be sure if such
changes are OK or not, other than to push them and then revert them
if/when one of the BF animals complains? If that is the case, then
it's not worth the hassle to pursue this any further.
I also wonder if this passes headerscheck and cpluspluscheck.
Thanks for pointing me to those - I didn't know about them.
Aside: Is there missing documentation for those targets here:
https://www.postgresql.org/docs/devel/regress.html
~
FWIW, both those tests passed OK. What does "pass" even mean -- does
it confirm this patch doesn't suffer the multiple typedef problem you
anticipated after all?
[postgres@CentOS7-x64 oss_postgres_misc]$ make headerscheck
make -C ./src/backend generated-headers
make[1]: Entering directory `/home/postgres/oss_postgres_misc/src/backend'
make -C catalog distprep generated-header-symlinks
make[2]: Entering directory
`/home/postgres/oss_postgres_misc/src/backend/catalog'
make[2]: Nothing to be done for `distprep'.
make[2]: Nothing to be done for `generated-header-symlinks'.
make[2]: Leaving directory
`/home/postgres/oss_postgres_misc/src/backend/catalog'
make -C nodes distprep generated-header-symlinks
make[2]: Entering directory `/home/postgres/oss_postgres_misc/src/backend/nodes'
make[2]: Nothing to be done for `distprep'.
make[2]: Nothing to be done for `generated-header-symlinks'.
make[2]: Leaving directory `/home/postgres/oss_postgres_misc/src/backend/nodes'
make -C utils distprep generated-header-symlinks
make[2]: Entering directory `/home/postgres/oss_postgres_misc/src/backend/utils'
make[2]: Nothing to be done for `distprep'.
make[2]: Nothing to be done for `generated-header-symlinks'.
make[2]: Leaving directory `/home/postgres/oss_postgres_misc/src/backend/utils'
make[1]: Leaving directory `/home/postgres/oss_postgres_misc/src/backend'
./src/tools/pginclude/headerscheck . /home/postgres/oss_postgres_misc
[postgres@CentOS7-x64 oss_postgres_misc]$
[postgres@CentOS7-x64 oss_postgres_misc]$ make cpluspluscheck
make -C ./src/backend generated-headers
make[1]: Entering directory `/home/postgres/oss_postgres_misc/src/backend'
make -C catalog distprep generated-header-symlinks
make[2]: Entering directory
`/home/postgres/oss_postgres_misc/src/backend/catalog'
make[2]: Nothing to be done for `distprep'.
make[2]: Nothing to be done for `generated-header-symlinks'.
make[2]: Leaving directory
`/home/postgres/oss_postgres_misc/src/backend/catalog'
make -C nodes distprep generated-header-symlinks
make[2]: Entering directory `/home/postgres/oss_postgres_misc/src/backend/nodes'
make[2]: Nothing to be done for `distprep'.
make[2]: Nothing to be done for `generated-header-symlinks'.
make[2]: Leaving directory `/home/postgres/oss_postgres_misc/src/backend/nodes'
make -C utils distprep generated-header-symlinks
make[2]: Entering directory `/home/postgres/oss_postgres_misc/src/backend/utils'
make[2]: Nothing to be done for `distprep'.
make[2]: Nothing to be done for `generated-header-symlinks'.
make[2]: Leaving directory `/home/postgres/oss_postgres_misc/src/backend/utils'
make[1]: Leaving directory `/home/postgres/oss_postgres_misc/src/backend'
./src/tools/pginclude/cpluspluscheck . /home/postgres/oss_postgres_misc
[postgres@CentOS7-x64 oss_postgres_misc]$
------
Kind Regards,
Peter Smith.
Fujitsu Australia
Peter Smith <smithpb2250@gmail.com> writes:
On Thu, Mar 2, 2023 at 10:04 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Sadly, this is almost certainly going to cause bitching on the part of
some compilers, because depending on the order of header inclusions
they are going to see multiple typedefs for the same name. Redundant
"struct foo" declarations are portable C, but redundant "typedef foo"
not so much.
So does your reply mean there is no way really to be sure if such
changes are OK or not, other than to push them and then revert them
if/when one of the BF animals complains?
We know which compilers don't like that, I believe, but you'd have
to dig in the commit log or mail archives to find out.
[ ... pokes around ... ] The commit log entries I could find about
this suggest that (at least) older gcc versions complain. Maybe
those are all gone from the buildfarm now, but I wouldn't bet on it.
We were fixing this sort of thing as recently as aa3ac6453.
I also wonder if this passes headerscheck and cpluspluscheck.
FWIW, both those tests passed OK. What does "pass" even mean -- does
it confirm this patch doesn't suffer the multiple typedef problem you
anticipated after all?
No, those have nothing to do with duplicate typedefs. headerscheck is
about whether anything is dependent on inclusion order, which I wondered
about for this patch. cpluspluscheck is about whether C++ compilers will
spit up on any of our headers (due to, eg, identifiers that are C++
keywords); we try to keep them clean for the benefit of people who write
extensions in C++. I wouldn't have expected cpluspluscheck to show
anything new with this patch, but people tend to always run these tools
together.
regards, tom lane
I wrote:
Peter Smith <smithpb2250@gmail.com> writes:
On Thu, Mar 2, 2023 at 10:04 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Sadly, this is almost certainly going to cause bitching on the part of
some compilers, because depending on the order of header inclusions
they are going to see multiple typedefs for the same name.
So does your reply mean there is no way really to be sure if such
changes are OK or not, other than to push them and then revert them
if/when one of the BF animals complains?
We know which compilers don't like that, I believe, but you'd have
to dig in the commit log or mail archives to find out.
I looked into the C standard to see what I could find about this.
C99 specifically describes the use of "struct foo" to forward-declare
a struct type whose meaning will be provided later. It also says
[#8] If a type specifier of the form
struct-or-union identifier
or
enum identifier
occurs other than as part of one of the above forms, and a
declaration of the identifier as a tag is visible, then it
specifies the same type as that other declaration, and does
not redeclare the tag.
which appears to me to specifically authorize the appearance of
multiple forward declarations. On the other hand, no such wording
appears for typedefs; they're just plain identifiers with the same
scope rules as other identifiers. Maybe later versions of the C
spec clarify this, but I think duplicate typedefs are pretty
clearly not OK per C99. Perhaps with sufficiently tight warning
or language-version options, you could get modern gcc or clang to
complain about it.
regards, tom lane
I wrote:
Maybe later versions of the C
spec clarify this, but I think duplicate typedefs are pretty
clearly not OK per C99.
Further research shows that C11 allows this, but it's definitely
not okay in C99, which is still our reference standard.
Perhaps with sufficiently tight warning
or language-version options, you could get modern gcc or clang to
complain about it.
clang seems to do so as soon as you restrict it to C99:
$ cat dup.c
typedef int foo;
typedef int foo;
$ clang -c -std=gnu99 dup.c
dup.c:2:13: warning: redefinition of typedef 'foo' is a C11 feature [-Wtypedef-redefinition]
typedef int foo;
^
dup.c:1:13: note: previous definition is here
typedef int foo;
^
1 warning generated.
I couldn't get gcc to issue a similar warning without resorting
to -Wpedantic, which of course whines about a ton of other stuff.
I'm a little inclined to see if I can turn on -std=gnu99 on my
clang-based buildfarm animals. I use that with gcc for my
normal development activities, but now that I see that clang
catches some things gcc doesn't ...
regards, tom lane
On Thu, Mar 2, 2023 at 12:40 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I wrote:
Peter Smith <smithpb2250@gmail.com> writes:
On Thu, Mar 2, 2023 at 10:04 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Sadly, this is almost certainly going to cause bitching on the part of
some compilers, because depending on the order of header inclusions
they are going to see multiple typedefs for the same name.So does your reply mean there is no way really to be sure if such
changes are OK or not, other than to push them and then revert them
if/when one of the BF animals complains?We know which compilers don't like that, I believe, but you'd have
to dig in the commit log or mail archives to find out.I looked into the C standard to see what I could find about this.
C99 specifically describes the use of "struct foo" to forward-declare
a struct type whose meaning will be provided later. It also says[#8] If a type specifier of the form
struct-or-union identifier
or
enum identifier
occurs other than as part of one of the above forms, and a
declaration of the identifier as a tag is visible, then it
specifies the same type as that other declaration, and does
not redeclare the tag.which appears to me to specifically authorize the appearance of
multiple forward declarations. On the other hand, no such wording
appears for typedefs; they're just plain identifiers with the same
scope rules as other identifiers. Maybe later versions of the C
spec clarify this, but I think duplicate typedefs are pretty
clearly not OK per C99. Perhaps with sufficiently tight warning
or language-version options, you could get modern gcc or clang to
complain about it.
I was reading this post [1]https://stackoverflow.com/questions/26240370/why-are-typedef-identifiers-allowed-to-be-declared-multiple-times/26240595#26240595, and more specifically, this specification
note [2]https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1360.htm which seems to explain things
Apparently, not all C99 compilers can be assumed to work using the
strict C99 rules. So I will abandon this idea.
Thanks for your replies.
------
[1]: https://stackoverflow.com/questions/26240370/why-are-typedef-identifiers-allowed-to-be-declared-multiple-times/26240595#26240595
[2]: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1360.htm
Kind Regards,
Peter Smith.
Fujitsu Australia
Peter Smith <smithpb2250@gmail.com> writes:
Apparently, not all C99 compilers can be assumed to work using the
strict C99 rules.
While googling this issue I came across a statement that clang currently
defaults to C17 rules. Even relatively old compilers might default to
C11. But considering how long we held on to C89, I doubt we'll want
to move the project minimum to C11 for some years yet.
So I will abandon this idea.
There might still be room to do something here, just not quite
that way. Maybe some actual header refactoring is called for?
regards, tom lane
I wrote:
I'm a little inclined to see if I can turn on -std=gnu99 on my
clang-based buildfarm animals. I use that with gcc for my
normal development activities, but now that I see that clang
catches some things gcc doesn't ...
FTR: done on sifaka and longfin.
regards, tom lane
On 02.03.23 04:00, Tom Lane wrote:
I wrote:
I'm a little inclined to see if I can turn on -std=gnu99 on my
clang-based buildfarm animals. I use that with gcc for my
normal development activities, but now that I see that clang
catches some things gcc doesn't ...FTR: done on sifaka and longfin.
mylodon already does something similar.
On 02.03.23 03:46, Tom Lane wrote:
Peter Smith <smithpb2250@gmail.com> writes:
Apparently, not all C99 compilers can be assumed to work using the
strict C99 rules.While googling this issue I came across a statement that clang currently
defaults to C17 rules. Even relatively old compilers might default to
C11. But considering how long we held on to C89, I doubt we'll want
to move the project minimum to C11 for some years yet.
We need to wait until we de-support Visual Studio older then 2019.
(Current minimum is 2015 (changed from 2013 for PG16).)