pg_create_logical_replication_slot returns text instead of name
Hi,
The documentation[1]https://www.postgresql.org/docs/devel/static/functions-admin.html#FUNCTIONS-REPLICATION says that both pg_create_logical_replication_slot
and pg_create_physical_replication_slot returns slot_name as a name
type. But only pg_create_logical_replication_slot returns it as text
type. I think these should be united as the name type. Attached small
patch fixes it.
[1]: https://www.postgresql.org/docs/devel/static/functions-admin.html#FUNCTIONS-REPLICATION
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
fix_create_logical_slot.patchapplication/octet-stream; name=fix_create_logical_slot.patchDownload
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 8cd8bf4..7251552 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1081,7 +1081,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,
- OUT slot_name text, OUT lsn pg_lsn)
+ OUT slot_name name, OUT lsn pg_lsn)
RETURNS RECORD
LANGUAGE INTERNAL
STRICT VOLATILE
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 23af323..450f737 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -142,7 +142,7 @@ pg_create_logical_replication_slot(PG_FUNCTION_ARGS)
/* build initial snapshot, might take a while */
DecodingContextFindStartpoint(ctx);
- values[0] = CStringGetTextDatum(NameStr(MyReplicationSlot->data.name));
+ values[0] = NameGetDatum(&MyReplicationSlot->data.name);
values[1] = LSNGetDatum(MyReplicationSlot->data.confirmed_flush);
/* don't need the decoding context anymore */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 00b59fd..a146510 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -9803,7 +9803,7 @@
{ oid => '3786', descr => 'set up a logical replication slot',
proname => 'pg_create_logical_replication_slot', provolatile => 'v',
proparallel => 'u', prorettype => 'record', proargtypes => 'name name bool',
- proallargtypes => '{name,name,bool,text,pg_lsn}',
+ proallargtypes => '{name,name,bool,name,pg_lsn}',
proargmodes => '{i,i,i,o,o}',
proargnames => '{slot_name,plugin,temporary,slot_name,lsn}',
prosrc => 'pg_create_logical_replication_slot' },
On Thu, Jul 12, 2018 at 07:00:48PM +0900, Masahiko Sawada wrote:
The documentation[1] says that both pg_create_logical_replication_slot
and pg_create_physical_replication_slot returns slot_name as a name
type. But only pg_create_logical_replication_slot returns it as text
type. I think these should be united as the name type. Attached small
patch fixes it.[1] https://www.postgresql.org/docs/devel/static/functions-admin.html#FUNCTIONS-REPLICATION
That's a small thing, but I agree with you. As far as I can see slot
names are always mapped with the name type. I'll push that tomorrow if
there are no objections.
--
Michael
On Thu, Jul 12, 2018 at 10:18:53PM +0900, Michael Paquier wrote:
That's a small thing, but I agree with you. As far as I can see slot
names are always mapped with the name type. I'll push that tomorrow if
there are no objections.
Pushed, with a catalog version bump.
While double-checking things, I have noticed that pg_stat_wal_receiver
also uses text for slot names. I am not sure if this one is worth
bothering as the docs point out the correct type, just mentioning.
--
Michael
On Fri, Jul 13, 2018 at 9:48 AM, Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Jul 12, 2018 at 10:18:53PM +0900, Michael Paquier wrote:
That's a small thing, but I agree with you. As far as I can see slot
names are always mapped with the name type. I'll push that tomorrow if
there are no objections.Pushed, with a catalog version bump.
Thank you!
While double-checking things, I have noticed that pg_stat_wal_receiver
also uses text for slot names. I am not sure if this one is worth
bothering as the docs point out the correct type, just mentioning.
Hmm, I'm also not sure about the policy of usage of name data type for
columns that show an object identifier on external servers. There is a
similar case; we have the pubname in pg_subscritpion as name type
whereas the subpublications in pg_subscription is text[] type.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Fri, Jul 13, 2018 at 11:14:01AM +0900, Masahiko Sawada wrote:
Hmm, I'm also not sure about the policy of usage of name data type for
columns that show an object identifier on external servers. There is a
similar case; we have the pubname in pg_subscritpion as name type
whereas the subpublications in pg_subscription is text[] type.
Let's not bother then. In the case of the function you pointed out the
intention behind the code was clear anyway, so that's better now.
--
Michael
On Fri, Jul 13, 2018 at 11:22 AM, Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Jul 13, 2018 at 11:14:01AM +0900, Masahiko Sawada wrote:
Hmm, I'm also not sure about the policy of usage of name data type for
columns that show an object identifier on external servers. There is a
similar case; we have the pubname in pg_subscritpion as name type
whereas the subpublications in pg_subscription is text[] type.Let's not bother then. In the case of the function you pointed out the
intention behind the code was clear anyway, so that's better now.
Yeah, I agreed with you.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center