On Fri, Jul 29, 2022 at 11:55 AM houzj.f...@fujitsu.com
<houzj.f...@fujitsu.com> wrote:
>
> On Friday, July 29, 2022 7:17 AM Peter Smith <smithpb2...@gmail.com> wrote:
> > During a recent review, I happened to notice that in the file
> > src/backend/catalog/pg_publication.c the two functions
> > 'is_publishable_class'
> > and 'is_publishable_relation' used to be [1] adjacent in the source code.
> > This is
> > also evident in 'is_publishable_relation' because the wording of the
> > function
> > comment just refers to the prior function (e.g. "Another variant of this,
> > taking a
> > Relation.") and also this just "wraps" the prior function.
> >
> > It seems that sometime last year another commit [2] inadvertently inserted
> > another function ('filter_partitions') between those aforementioned, and
> > that
> > means the "Another variant of this" comment doesn't make much sense
> > anymore.
>
> Agreed.
>
> Personally, I think it would be better to modify the comments of
> is_publishable_relation and directly mention the function name it refers to
> which can prevent future code to break it again.
I'd intended only to make the minimal changes necessary to set things
right again, but your way is better.
>
> Besides,
>
> /*
> * Returns if relation represented by oid and Form_pg_class entry
> * is publishable.
> *
> * Does same checks as the above,
>
> This comment was also intended to refer to the function
> check_publication_add_relation(), but is invalid now because there is another
> function check_publication_add_schema() inserted between them. We'd better fix
> this as well.
Thanks, I'll post another patch later to address that one too.
------
Kind Regards,
Peter Smith.
Fujitsu Australia
