The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested
Hello
> +# run the retail DDL tests at the end make sense to not interrupt with other
> +# tests in case the object names are the same to other objects previously used.
> +test: publication_ddl
retail DDL tests is unclear to me. Might as well remove the comment.
>+ if (pub->alltables || pub->allsequences)
>+ {
>+ appendStringInfo(buf, " FOR ");
>+
>+ if (pub->alltables)
>+ appendStringInfo(buf, "ALL TABLES");
>+
>+ /* The sequence are published only on versions 19+ */
>+ if (pub->allsequences)
>+ appendStringInfo(buf,
>+ "%sALL SEQUENCE",
>+ pub->alltables ? ", " : " ");
>+ }
Critical bug / typo here, I think you meant ALL SEQUENCES instead
of ALL SEQUENCE.
>+ if (pub == NULL)
>+ return (Datum) NULL;
use PG_RETURN_NULL() instead like everywhere else for consistency.
Typos in your commit message and everywhere else in code.. For
example:
"giving" → "given", "thuse" → "thus",
"Comprhensive" → "Comprehensive", "fro" → "for"
Need to run the patch through a spelling / typo check tool
Also, you should test pg_get_publication_ddl() with non-existent
publication as well.
Best regards
Cary Huang
Highgo Software
