On Thu, Jul 25, 2024 at 5:56 PM Kirill Reshke <reshkekir...@gmail.com> wrote: > > > +/* > > + * If the tuple to be modified could not be found, a log message is > > emitted. > > + */ > > +static void > > +report_tuple_not_found(CmdType cmd, Relation targetrel, bool is_partition) > > +{ > > + Assert(cmd == CMD_UPDATE || cmd == CMD_DELETE); > > + > > + /* XXX should this be promoted to ereport(LOG) perhaps? */ > > + elog(DEBUG1, > > + "logical replication did not find row to be %s in replication target > > relation%s \"%s\"", > > + cmd == CMD_UPDATE ? "updated" : "deleted", > > + is_partition ? "'s partition" : "", > > + RelationGetRelationName(targetrel)); > > +} > > Encapsulating report logic inside function is ok, >
This could even be a separate patch as it is not directly to other parts of the 0002 patch. BTW, the problem statement for 0002 is not explicitly stated like which part of the code we want to optimize by removing duplication. Also, as proposed the name apply_handle_tuple_routing() for the function doesn't seem suitable as it no longer remains similar to other apply_handle_* functions where we perform the required operation like insert or update the tuple. How about naming it as apply_tuple_routing()? -- With Regards, Amit Kapila.