Re: pg_upgrade and logical replication
От | Julien Rouhaud |
---|---|
Тема | Re: pg_upgrade and logical replication |
Дата | |
Msg-id | 20230413084557.vgcqedgtmjzgiju7@jrouhaud обсуждение исходный текст |
Ответ на | Re: pg_upgrade and logical replication (Julien Rouhaud <rjuju123@gmail.com>) |
Список | pgsql-hackers |
On Thu, Apr 13, 2023 at 10:51:10AM +0800, Julien Rouhaud wrote: > > On Wed, Apr 12, 2023 at 09:48:15AM +0000, Hayato Kuroda (Fujitsu) wrote: > > > > 5. AlterSubscription > > > > ``` > > + supported_opts = SUBOPT_RELID | SUBOPT_STATE | SUBOPT_LSN; > > + parse_subscription_options(pstate, stmt->options, > > + supported_opts, &opts); > > + > > + /* relid and state should always be provided. */ > > + Assert(IsSet(opts.specified_opts, SUBOPT_RELID)); > > + Assert(IsSet(opts.specified_opts, SUBOPT_STATE)); > > + > > ``` > > > > SUBOPT_LSN accepts "none" string, which means InvalidLSN. Isn't it better to > > reject it? > > If you mean have an Assert for that I agree. It's not supposed to be used by > users so I don't think having non debug check is sensible, as any user provided > value has no reason to be correct anyway. After looking at the code I remember that I kept the lsn optional in ALTER SUBSCRIPTION name ADD TABLE command processing. For now pg_upgrade checks that all subscriptions have a valid remote_lsn so there should indeed always be a value different from InvalidLSN/none specified, but it's still unclear to me whether this check will eventually be weakened or not, so for now I think it's better to keep AlterSubscription accept this case, here and in all other code paths. If there's a hard objection I will just make the lsn mandatory. > > 9. parseCommandLine > > > > ``` > > + user_opts.preserve_subscriptions = false; > > ``` > > > > I think this initialization is not needed because it is default. > > It's not strictly needed because of C rules but I think it doesn't really hurt > to make it explicit and not have to remember what the standard says. So I looked at nearby code and other option do rely on zero-initialized global variables, so I agree that this initialization should be removed.
В списке pgsql-hackers по дате отправления: