Re: Synchronizing slots from primary to standby

Hi,

On 11/27/23 1:23 PM, Zhijie Hou (Fujitsu) wrote:
> On Monday, November 27, 2023 8:05 PM Drouvot, Bertrand <bertranddrouvot.pg@gmail.com> wrote:
> 
> Hi,
> 
>> On 11/6/23 2:30 AM, Zhijie Hou (Fujitsu) wrote:
>>> On Friday, November 3, 2023 7:32 PM Amit Kapila
>> <amit.kapila16@gmail.com>
>>>>
>>>> I don't see a corresponding change in repl_gram.y. I think the following part
>> of
>>>> the code needs to be changed:
>>>> /* CREATE_REPLICATION_SLOT slot [TEMPORARY] LOGICAL plugin [options]
>> */
>>>> | K_CREATE_REPLICATION_SLOT IDENT opt_temporary K_LOGICAL IDENT
>>>> create_slot_options
>>>>
>>>
>>> I think after 0266e98, we started to use the new syntax(see the
>>> generic_option_list rule) and we can avoid changing the repl_gram.y when
>> adding
>>> new options. The new failover can be detected when parsing the generic
>> option
>>> list(in parseCreateReplSlotOptions).
>>
>> Did not look in details but it looks like there is more to do here as
>> this is failing (with v39_2):
>>
>> "
>> postgres@primary: psql replication=database
>> psql (17devel)
>> Type "help" for help.
>>
>> postgres=# CREATE_REPLICATION_SLOT test_logical20 LOGICAL pgoutput
>> FAILOVER;
>> ERROR:  syntax error
> 
> I think the command you executed is of old syntax style, which was kept for
> compatibility with older releases. And I think we can avoid supporting new
> option for the old syntax as described in the original thread[1] of commit
> 0266e98. So, the "syntax error" is as expected IIUC.
> 
> The new style command is like:
> CREATE_REPLICATION_SLOT test_logical20 LOGICAL pgoutput (FAILOVER);
> 

If / As we are not going to support the old syntax for the FAILOVER option
so I think we can get rid of the check on "use_new_options_syntax" here:

-
+               if (failover)
+               {
+                       appendStringInfoString(&cmd, "FAILOVER");
+                       if (use_new_options_syntax)
+                               appendStringInfoString(&cmd, ", ");
+                       else
+                               appendStringInfoChar(&cmd, ' ');
+               }

as we'd error out before if using the old syntax.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com