On Friday, July 29, 2022 7:17 AM Peter Smith <smithpb2250@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.
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.
Best regards,
Hou zj