On Tue, Jun 24, 2025 at 9:48 AM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
>
> I have included the changes for
> it in v14-0003 patch.
>
Thanks for the patches. I have reviewed patch001 alone, please find
few comments:
1)
+ <para>
+ The <literal>RESET</literal> clause will reset the publication to the
+ default state which includes resetting the publication parameters, setting
+ <literal>ALL TABLES</literal> flag to <literal>false</literal> and
+ dropping all relations and schemas that are associated with the
+ publication.
</para>
It is misleading, as far as I have understood, we do not drop the
tables or schemas associated with the pub; we just remove those from
the publication's object list. See previous doc:
"The ADD and DROP clauses will add and remove one or more
tables/schemas from the publication"
Perhaps we want to say the same thing when we speak about the 'drop'
aspect of RESET.
2)
AlterPublicationReset():
+ if (!OidIsValid(prid))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("relation \"%s\" is not part of the publication",
+ get_rel_name(relid))));
Can you please help me understand which scenario will give this error?
Another question is do we really need this error? IIUC, we generally
give errors if a user has explicitly called out a name of an object
and that object is not found. Example:
postgres=# alter publication pubnew drop table t1,tab2;
ERROR: relation "t1" is not part of the publication
While in a few other cases, we pass missing_okay as true and do not
give errors. Please see other callers of performDeletion in
publicationcmds.c itself. There we have usage of missing_okay=true. I
have not researched myself, but please analyze the cases where
missing_okay is passed as true to figure out if those match our RESET
case. Try to reproduce if possible and then take a call.
3)
+ALTER PUBLICATION testpub_reset ADD ALL TABLES IN SCHEMA public;
+ERROR: syntax error at or near "ALL"
+LINE 1: ALTER PUBLICATION testpub_reset ADD ALL TABLES IN SCHEMA pub...
There is a problem in syntax, I think the intention of testcase was to
run this query successfully.
thanks
Shveta