RE: Functions 'is_publishable_class' and 'is_publishable_relation' should stay together.

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