Обсуждение: Doc: CREATE_REPLICATION_SLOT command requires the plugin name

Поиск
Список
Период
Сортировка

Doc: CREATE_REPLICATION_SLOT command requires the plugin name

От
Antonin Houska
Дата:
I got a syntax error when using the command according to the existing
documentation. The output_plugin parameter needs to be passed too.

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com

diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 24e93f9b28..6065c18fdf 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1942,7 +1942,7 @@ The commands accepted in replication mode are:
   </varlistentry>
 
   <varlistentry id="protocol-replication-create-slot" xreflabel="CREATE_REPLICATION_SLOT">
-   <term><literal>CREATE_REPLICATION_SLOT</literal> <replaceable class="parameter">slot_name</replaceable> [
<literal>TEMPORARY</literal>] { <literal>PHYSICAL</literal> | <literal>LOGICAL</literal> } [ ( <replaceable
class="parameter">option</replaceable>[, ...] ) ]
 
+   <term><literal>CREATE_REPLICATION_SLOT</literal> <replaceable class="parameter">slot_name</replaceable> [
<literal>TEMPORARY</literal>] { <literal>PHYSICAL</literal> | <literal>LOGICAL</literal> <replaceable
class="parameter">output_plugin</replaceable>} [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ]
 
      <indexterm><primary>CREATE_REPLICATION_SLOT</primary></indexterm>
     </term>
     <listitem>

Re: Doc: CREATE_REPLICATION_SLOT command requires the plugin name

От
Amit Kapila
Дата:
On Tue, Feb 1, 2022 at 3:44 PM Antonin Houska <ah@cybertec.at> wrote:
>
> I got a syntax error when using the command according to the existing
> documentation. The output_plugin parameter needs to be passed too.
>

Why do we need it for physical slots? The syntax in repl_gram.y is as follows:
/* CREATE_REPLICATION_SLOT slot TEMPORARY PHYSICAL [options] */
K_CREATE_REPLICATION_SLOT IDENT opt_temporary K_PHYSICAL create_slot_options
...
/* CREATE_REPLICATION_SLOT slot TEMPORARY LOGICAL plugin [options] */
| K_CREATE_REPLICATION_SLOT IDENT opt_temporary K_LOGICAL IDENT
create_slot_options

The logical slots do need output_plugin but not physical ones.

-- 
With Regards,
Amit Kapila.

Re: Doc: CREATE_REPLICATION_SLOT command requires the plugin name

От
Antonin Houska
Дата:
Amit Kapila <amit.kapila16@gmail.com> wrote:

> On Tue, Feb 1, 2022 at 3:44 PM Antonin Houska <ah@cybertec.at> wrote:
> >
> > I got a syntax error when using the command according to the existing
> > documentation. The output_plugin parameter needs to be passed too.
> >
> 
> Why do we need it for physical slots?

Sure we don't, the missing curly brackets seem to be the problem. I used the
other form of the command for reference, which therefore might need a minor
fix too.

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com

diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 24e93f9b28..49fae6c669 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1942,7 +1942,7 @@ The commands accepted in replication mode are:
   </varlistentry>
 
   <varlistentry id="protocol-replication-create-slot" xreflabel="CREATE_REPLICATION_SLOT">
-   <term><literal>CREATE_REPLICATION_SLOT</literal> <replaceable class="parameter">slot_name</replaceable> [
<literal>TEMPORARY</literal>] { <literal>PHYSICAL</literal> | <literal>LOGICAL</literal> } [ ( <replaceable
class="parameter">option</replaceable>[, ...] ) ]
 
+   <term><literal>CREATE_REPLICATION_SLOT</literal> <replaceable class="parameter">slot_name</replaceable> [
<literal>TEMPORARY</literal>] { <literal>PHYSICAL</literal> | { <literal>LOGICAL</literal> <replaceable
class="parameter">output_plugin</replaceable>} } [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ]
 
      <indexterm><primary>CREATE_REPLICATION_SLOT</primary></indexterm>
     </term>
     <listitem>
@@ -2085,7 +2085,8 @@ The commands accepted in replication mode are:
   </varlistentry>
 
   <varlistentry>
-    <term><literal>CREATE_REPLICATION_SLOT</literal> <replaceable class="parameter">slot_name</replaceable> [
<literal>TEMPORARY</literal>] { <literal>PHYSICAL</literal> [ <literal>RESERVE_WAL</literal> ] |
<literal>LOGICAL</literal><replaceable class="parameter">output_plugin</replaceable> [
<literal>EXPORT_SNAPSHOT</literal>| <literal>NOEXPORT_SNAPSHOT</literal> | <literal>USE_SNAPSHOT</literal> |
<literal>TWO_PHASE</literal>] }
 
+    <term><literal>CREATE_REPLICATION_SLOT</literal> <replaceable class="parameter">slot_name</replaceable> [
<literal>TEMPORARY</literal>] { <literal>PHYSICAL</literal> [ <literal>RESERVE_WAL</literal> ] | {
<literal>LOGICAL</literal><replaceable class="parameter">output_plugin</replaceable>
 
+} [ <literal>EXPORT_SNAPSHOT</literal> | <literal>NOEXPORT_SNAPSHOT</literal> | <literal>USE_SNAPSHOT</literal> |
<literal>TWO_PHASE</literal>] }
 
     </term>
     <listitem>
      <para>

Re: Doc: CREATE_REPLICATION_SLOT command requires the plugin name

От
Amit Kapila
Дата:
On Tue, Feb 1, 2022 at 5:43 PM Antonin Houska <ah@cybertec.at> wrote:
>
> Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> > On Tue, Feb 1, 2022 at 3:44 PM Antonin Houska <ah@cybertec.at> wrote:
> > >
> > > I got a syntax error when using the command according to the existing
> > > documentation. The output_plugin parameter needs to be passed too.
> > >
> >
> > Why do we need it for physical slots?
>
> Sure we don't, the missing curly brackets seem to be the problem. I used the
> other form of the command for reference, which therefore might need a minor
> fix too.
>

Okay, I see that it is not very clear from the documentation.

   <varlistentry>
-    <term><literal>CREATE_REPLICATION_SLOT</literal> <replaceable
class="parameter">slot_name</replaceable> [
<literal>TEMPORARY</literal> ] { <literal>PHYSICAL</literal> [
<literal>RESERVE_WAL</literal> ] | <literal>LOGICAL</literal>
<replaceable class="parameter">output_plugin</replaceable> [
<literal>EXPORT_SNAPSHOT</literal> |
<literal>NOEXPORT_SNAPSHOT</literal> | <literal>USE_SNAPSHOT</literal>
| <literal>TWO_PHASE</literal> ] }
+    <term><literal>CREATE_REPLICATION_SLOT</literal> <replaceable
class="parameter">slot_name</replaceable> [
<literal>TEMPORARY</literal> ] { <literal>PHYSICAL</literal> [
<literal>RESERVE_WAL</literal> ] | { <literal>LOGICAL</literal>
<replaceable class="parameter">output_plugin</replaceable>
+} [ <literal>EXPORT_SNAPSHOT</literal> |
<literal>NOEXPORT_SNAPSHOT</literal> | <literal>USE_SNAPSHOT</literal>
| <literal>TWO_PHASE</literal> ] }

Instead of adding additional '{}', can't we simply use:
{ <literal>PHYSICAL</literal> [ <literal>RESERVE_WAL</literal> ] |
<literal>LOGICAL</literal> <replaceable
class="parameter">output_plugin</replaceable> } [
<literal>EXPORT_SNAPSHOT</literal> |
<literal>NOEXPORT_SNAPSHOT</literal> | <literal>USE_SNAPSHOT</literal>
| <literal>TWO_PHASE</literal> ]

I am not sure change to other syntax is useful as that is kept for
backward compatibility. I think we can keep that as it is.

-- 
With Regards,
Amit Kapila.

Re: Doc: CREATE_REPLICATION_SLOT command requires the plugin name

От
Antonin Houska
Дата:
Amit Kapila <amit.kapila16@gmail.com> wrote:

> On Tue, Feb 1, 2022 at 5:43 PM Antonin Houska <ah@cybertec.at> wrote:
> >
> > Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > > On Tue, Feb 1, 2022 at 3:44 PM Antonin Houska <ah@cybertec.at> wrote:
> > > >
> > > > I got a syntax error when using the command according to the existing
> > > > documentation. The output_plugin parameter needs to be passed too.
> > > >
> > >
> > > Why do we need it for physical slots?
> >
> > Sure we don't, the missing curly brackets seem to be the problem. I used the
> > other form of the command for reference, which therefore might need a minor
> > fix too.
> >
>
> Instead of adding additional '{}', can't we simply use:
> { <literal>PHYSICAL</literal> [ <literal>RESERVE_WAL</literal> ] |
> <literal>LOGICAL</literal> <replaceable
> class="parameter">output_plugin</replaceable> } [
> <literal>EXPORT_SNAPSHOT</literal> |
> <literal>NOEXPORT_SNAPSHOT</literal> | <literal>USE_SNAPSHOT</literal>
> | <literal>TWO_PHASE</literal> ]

Do you mean changing

CREATE_REPLICATION_SLOT slot_name [ TEMPORARY ] { PHYSICAL [ RESERVE_WAL ] | LOGICAL output_plugin [ EXPORT_SNAPSHOT |
NOEXPORT_SNAPSHOT| USE_SNAPSHOT | TWO_PHASE ] } 

to

CREATE_REPLICATION_SLOT slot_name [ TEMPORARY ] { PHYSICAL [ RESERVE_WAL ] | LOGICAL output_plugin } [ EXPORT_SNAPSHOT
|NOEXPORT_SNAPSHOT | USE_SNAPSHOT | TWO_PHASE ] 

?

I'm not sure, IMHO that would still allow for commands like

CREATE_REPLICATION_SLOT slot_name PHYSICAL output_plugin


--
Antonin Houska
Web: https://www.cybertec-postgresql.com

Re: Doc: CREATE_REPLICATION_SLOT command requires the plugin name

От
Amit Kapila
Дата:
On Wed, Feb 2, 2022 at 12:41 PM Antonin Houska <ah@cybertec.at> wrote:
>
> Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> > On Tue, Feb 1, 2022 at 5:43 PM Antonin Houska <ah@cybertec.at> wrote:
> > >
> > > Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > > On Tue, Feb 1, 2022 at 3:44 PM Antonin Houska <ah@cybertec.at> wrote:
> > > > >
> > > > > I got a syntax error when using the command according to the existing
> > > > > documentation. The output_plugin parameter needs to be passed too.
> > > > >
> > > >
> > > > Why do we need it for physical slots?
> > >
> > > Sure we don't, the missing curly brackets seem to be the problem. I used the
> > > other form of the command for reference, which therefore might need a minor
> > > fix too.
> > >
> >
> > Instead of adding additional '{}', can't we simply use:
> > { <literal>PHYSICAL</literal> [ <literal>RESERVE_WAL</literal> ] |
> > <literal>LOGICAL</literal> <replaceable
> > class="parameter">output_plugin</replaceable> } [
> > <literal>EXPORT_SNAPSHOT</literal> |
> > <literal>NOEXPORT_SNAPSHOT</literal> | <literal>USE_SNAPSHOT</literal>
> > | <literal>TWO_PHASE</literal> ]
>
> Do you mean changing
>
> CREATE_REPLICATION_SLOT slot_name [ TEMPORARY ] { PHYSICAL [ RESERVE_WAL ] | LOGICAL output_plugin [ EXPORT_SNAPSHOT
|NOEXPORT_SNAPSHOT | USE_SNAPSHOT | TWO_PHASE ] }
 
>
> to
>
> CREATE_REPLICATION_SLOT slot_name [ TEMPORARY ] { PHYSICAL [ RESERVE_WAL ] | LOGICAL output_plugin } [
EXPORT_SNAPSHOT| NOEXPORT_SNAPSHOT | USE_SNAPSHOT | TWO_PHASE ]
 
>
> ?
>
> I'm not sure, IMHO that would still allow for commands like
>
> CREATE_REPLICATION_SLOT slot_name PHYSICAL output_plugin
>

I don't think so. Symbol '|' differentiates that, check its usage at
other places like Create Table doc page [1]. Considering that, it
seems to me that the way it is currently mentioned in docs is correct.

CREATE_REPLICATION_SLOT slot_name [ TEMPORARY ] { PHYSICAL [
RESERVE_WAL ] | LOGICAL output_plugin [ EXPORT_SNAPSHOT |
NOEXPORT_SNAPSHOT | USE_SNAPSHOT | TWO_PHASE ] }

According to me, this means output_plugin and all other options
related to snapshot can be mentioned only for logical slots.

[1] - https://www.postgresql.org/docs/devel/sql-createtable.html

-- 
With Regards,
Amit Kapila.