On Tue, 11 Nov 2025 at 09:59, Shlok Kyal <[email protected]> wrote: > > On Tue, 11 Nov 2025 at 09:02, Amit Kapila <[email protected]> wrote: > > > > On Mon, Nov 10, 2025 at 4:22 PM vignesh C <[email protected]> wrote: > > > > > > On Mon, 10 Nov 2025 at 14:34, Shlok Kyal <[email protected]> wrote: > > > > > > > > While working on another thread, I found that in HEAD gram.y has > > > > grammar which was committed as part of this thread: > > > > ``` > > > > | CREATE PUBLICATION name FOR pub_obj_type_list > > > > opt_definition > > > > { > > > > CreatePublicationStmt *n = > > > > makeNode(CreatePublicationStmt); > > > > > > > > n->pubname = $3; > > > > n->pubobjects = (List *) $5; > > > > preprocess_pub_all_objtype_list($5, > > > > &n->for_all_tables, > > > > > > > > &n->for_all_sequences, > > > > yyscanner); > > > > n->options = $6; > > > > $$ = (Node *) n; > > > > } > > > > ``` > > > > > > > > Here we are assigning "n->pubobjects = (List *) $5". But later in the > > > > code this is not used anywhere for ALL TABLES/ ALL SEQUENCES > > > > publication. It is used for other publications (not ALL TABLES/ > > > > SEQUENCES) inside function "ObjectsInPublicationToOids" > > > > > > > > So are we required to assign "n->pubobjects" here? > > > > > > > > I have created a patch to remove this assignment. It passed "make > > > > check-world". > > > > > > I agree that this is not required for ALL TABLES/ALL SEQUENCES cases. > > > > > > > I also agree. BTW, can we change pub_obj_type_list to > > pub_all_obj_type_list to be more explicit about this variant? If you > > agree then we can make similar changes > > (pub_obj_type->pub_all_obj_type) at the following places as well: > > > > + * CREATE PUBLICATION FOR ALL pub_obj_type [, ...] [WITH options] > > + * > > + * pub_obj_type is one of: > > + * > > > Hi Vignesh, Amit > > Thanks for confirming. > Also, I agree that the name 'pub_all_obj_type' will be more suitable. > I have updated the patch for the same.
Thanks, this version looks good to me, I don't have any comments. Regards, Vignesh
