Re: [18] CREATE SUBSCRIPTION ... SERVER

On Sat, 1 Mar 2025 at 04:35, Jeff Davis <pgsql@j-davis.com> wrote:
>
> On Mon, 2024-12-16 at 20:05 -0800, Jeff Davis wrote:
> > On Wed, 2024-10-30 at 08:08 -0700, Jeff Davis wrote:
> >
>
> Rebased v14.
>
> The approach has changed multiple times. It starte off with more in-
> core code, but in response to review feedback, has become more
> decoupled from core and more coupled to postgres_fdw.
>
> But the patch has been about the same (just rebases) since March of
> last year, and hasn't gotten feedback since. I still think it's a nice
> feature, but I'd like some feedback on the externals of the feature.

Few comments:
1) \dRs+ sub does not include the server info:
postgres=# \dRs+ sub*

                                 List of subscriptions
 Name |  Owner  | Enabled | Publication | Binary | Streaming |
Two-phase commit | Disable on error | Origin | Password required | Run
as owner? | Failover | Synchronous commit |
    Conninfo                 | Skip LSN
------+---------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------------------+-------------
-----------------------------+----------
 sub  | vignesh | t       | {pub1}      | f      | parallel  | d
         | f                | any    | t                 | f
  | f        | off                |
                             | 0/0

2) Tab completion for alter subscription also should include server:
+++ b/src/bin/psql/tab-complete.in.c
@@ -3704,7 +3704,7 @@ match_previous_words(int pattern_id,

 /* CREATE SUBSCRIPTION */
        else if (Matches("CREATE", "SUBSCRIPTION", MatchAny))
-               COMPLETE_WITH("CONNECTION");
+               COMPLETE_WITH("SERVER", "CONNECTION");


postgres=# alter subscription sub3
ADD PUBLICATION      DISABLE              ENABLE               REFRESH
PUBLICATION  SET
CONNECTION           DROP PUBLICATION     OWNER TO             RENAME
TO            SKIP (

3) In case of binary mode, pg_dump creates subscription using server
option, but not in normal mode:
+       if (dopt->binary_upgrade && fout->remoteVersion >= 180000)
+               appendPQExpBufferStr(query, " fs.srvname AS subservername,\n"
+                                                        "
o.remote_lsn AS suboriginremotelsn,\n"
+                                                        " s.subenabled,\n"
+                                                        " s.subfailover\n");
+       else
+               appendPQExpBufferStr(query, " NULL AS subservername,\n"
+                                                        " NULL AS
suboriginremotelsn,\n"
+                                                        " false AS
subenabled,\n"
+                                                        " false AS
subfailover\n");

If there is some specific reason, we should at least add some comments.

Regards,
Vignesh