Re: Added schema level support for publication.

On Mon, Jul 12, 2021 at 7:24 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
>
> Thanks for reporting this issue. I felt this issue is the same as the issue which Hou San had reported. This issue is fixed in the v10 patch attached at [1].
> [1] - https://www.postgresql.org/message-id/CALDaNm2%2BtR%2B8R-sD1CSyMbZcZbkintZE-avefjsp7LCkm6HMmw%40mail.gmail.com
>

I did some testing and the issue that I reported does seem to be fixed
by the v10 patch.

I have some patch review comments for the v10 patch:

(1)

The following:

+ if (!OidIsValid(address.objectId))
+ {
+ if (!missing_ok)
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("publication schema \"%s\" in publication \"%s\"
does not exist",
+ schemaname, pubname)));
+ return address;
+ }
+
+ return address;

could just be simplified to:

+ if (!OidIsValid(address.objectId) && !missing_ok)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("publication schema \"%s\" in publication \"%s\" does not exist",
+ schemaname, pubname)));
+ }
+
+ return address;

(2) src/backend/catalog/objectaddress.c

I think there is a potential illegal memory access (psform->psnspcid)
in the case of "!missing_ok", as the tuple is released from the cache
on the previous line.

+ psform = (Form_pg_publication_schema) GETSTRUCT(tup);
+ pubname = get_publication_name(psform->pspubid, false);
+ nspname = get_namespace_name(psform->psnspcid);
+ if (!nspname)
+ {
+ pfree(pubname);
+ ReleaseSysCache(tup);
+ if (!missing_ok)
+ elog(ERROR, "cache lookup failed for schema %u",
+ psform->psnspcid);
+ break;
+ }

I think this should be:

+ psform = (Form_pg_publication_schema) GETSTRUCT(tup);
+ pubname = get_publication_name(psform->pspubid, false);
+ nspname = get_namespace_name(psform->psnspcid);
+ if (!nspname)
+ {
+ Oid psnspcid = psform->psnspcid;
+
+ pfree(pubname);
+ ReleaseSysCache(tup);
+ if (!missing_ok)
+ elog(ERROR, "cache lookup failed for schema %u",
+ psnspcid);
+ break;
+ }

There are two cases of this that need correction (see: case
OCLASS_PUBLICATION_SCHEMA).

(3) Incomplete function header comment

+ * makeSchemaSpec - Create a SchemaSpec with the given type

Should be:

+ * makeSchemaSpec - Create a SchemaSpec with the given type and location

(4) src/bin/psql/describe.c

Shouldn't the following comment say "version 15"?

+ /* Prior to version 14 check was based on all tables */
+ if ((has_pubtype && pubtype == PUBTYPE_TABLE) ||
+ (!has_pubtype && !puballtables))

(5) typedefs.list

I think you also need to add "Form_pg_publication_schema" to typedefs.list.

Regards,
Greg Nancarrow
Fujitsu Australia