pg_create_logical_replication_slot argument incongruency
Hello,
The function `pg_create_logical_replication_slot()` is documented to have
a `two_phase` argument(note the underscore), but the function instead
requires `twophase`.
```
\df pg_catalog.pg_create_logical_replication_slot
List of functions
-[ RECORD 1
]-------+---------------------------------------------------------------------------------------------------------------------------------
Schema | pg_catalog
Name | pg_create_logical_replication_slot
Result data type | record
Argument data types | slot_name name, plugin name, temporary boolean
DEFAULT false, twophase boolean DEFAULT false, OUT slot_name name, OUT lsn
pg_lsn
Type | func
```
This was introduced in commit 19890a06.
IMHO we should use the documented argument name `two_phase` and change the
function to accept it.
What do you think?
Please, check the attached patch.
Cheers,
Florin
--
*www.enterprisedb.com <http://www.enterprisedb.com/>*
Attachments:
two_phase_slot_v1.patchapplication/octet-stream; name=two_phase_slot_v1.patchDownload
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index f63a04c667..40f5459f1e 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25891,7 +25891,8 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
<indexterm>
<primary>pg_create_logical_replication_slot</primary>
</indexterm>
- <function>pg_create_logical_replication_slot</function> ( <parameter>slot_name</parameter> <type>name</type>, <parameter>plugin</parameter> <type>name</type> <optional>, <parameter>temporary</parameter> <type>boolean</type>, <parameter>two_phase</parameter> <type>boolean</type> </optional> )
+ <function>pg_create_logical_replication_slot
+ </function> ( <parameter>slot_name</parameter> <type>name</type>, <parameter>plugin</parameter> <type>name</type> <optional>, <parameter>temporary</parameter> <type>boolean</type>, <parameter>two_phase</parameter> <type>boolean</type> </optional> )
<returnvalue>record</returnvalue>
( <parameter>slot_name</parameter> <type>name</type>,
<parameter>lsn</parameter> <type>pg_lsn</type> )
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index a416e94d37..26cd925ff6 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -451,7 +451,7 @@ AS 'pg_create_physical_replication_slot';
CREATE OR REPLACE FUNCTION pg_create_logical_replication_slot(
IN slot_name name, IN plugin name,
IN temporary boolean DEFAULT false,
- IN twophase boolean DEFAULT false,
+ IN two_phase boolean DEFAULT false,
OUT slot_name name, OUT lsn pg_lsn)
RETURNS RECORD
LANGUAGE INTERNAL
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 79669bf5a2..f6ae1c4a5f 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10703,7 +10703,7 @@
proargtypes => 'name name bool bool',
proallargtypes => '{name,name,bool,bool,name,pg_lsn}',
proargmodes => '{i,i,i,i,o,o}',
- proargnames => '{slot_name,plugin,temporary,twophase,slot_name,lsn}',
+ proargnames => '{slot_name,plugin,temporary,two_phase,slot_name,lsn}',
prosrc => 'pg_create_logical_replication_slot' },
{ oid => '4222',
descr => 'copy a logical replication slot, changing temporality and plugin',
On Mon, Sep 19, 2022 at 07:02:16PM +0200, Florin Irion wrote:
This was introduced in commit 19890a06.
IMHO we should use the documented argument name `two_phase` and change the
function to accept it.What do you think?
19890a0 is included in REL_14_STABLE, and changing an argument name is
not acceptable in a stable branch as it would imply a catversion
bump. Let's change the docs so as we document the parameter as
"twophase", instead.
--
Michael
On 20/09/22 03:33, Michael Paquier wrote:
On Mon, Sep 19, 2022 at 07:02:16PM +0200, Florin Irion wrote:
This was introduced in commit 19890a06.
IMHO we should use the documented argument name `two_phase` and change the
function to accept it.What do you think?
19890a0 is included in REL_14_STABLE, and changing an argument name is
not acceptable in a stable branch as it would imply a catversion
bump. Let's change the docs so as we document the parameter as
"twophase", instead.
--
Michael
I understand.
OK, patch only for the docs attached.
Cheers,
Florin
www.enterprisedb.com
Attachments:
two_phase_slot_v2.patchtext/plain; charset=UTF-8; name=two_phase_slot_v2.patchDownload
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index f63a04c667..f1a8a9a0ba 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25891,7 +25891,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
<indexterm>
<primary>pg_create_logical_replication_slot</primary>
</indexterm>
- <function>pg_create_logical_replication_slot</function> ( <parameter>slot_name</parameter> <type>name</type>, <parameter>plugin</parameter> <type>name</type> <optional>, <parameter>temporary</parameter> <type>boolean</type>, <parameter>two_phase</parameter> <type>boolean</type> </optional> )
+ <function>pg_create_logical_replication_slot</function> ( <parameter>slot_name</parameter> <type>name</type>, <parameter>plugin</parameter> <type>name</type> <optional>, <parameter>temporary</parameter> <type>boolean</type>, <parameter>twophase</parameter> <type>boolean</type> </optional> )
<returnvalue>record</returnvalue>
( <parameter>slot_name</parameter> <type>name</type>,
<parameter>lsn</parameter> <type>pg_lsn</type> )
@@ -25904,7 +25904,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup());
the slot should not be permanently stored to disk and is only meant
for use by the current session. Temporary slots are also
released upon any error. The optional fourth parameter,
- <parameter>two_phase</parameter>, when set to true, specifies
+ <parameter>twophase</parameter>, when set to true, specifies
that the decoding of prepared transactions is enabled for this
slot. A call to this function has the same effect as the replication
protocol command <literal>CREATE_REPLICATION_SLOT ... LOGICAL</literal>.
On Tue, Sep 20, 2022 at 08:41:56AM +0200, Florin Irion wrote:
OK, patch only for the docs attached.
Thanks, applied.
--
Michael