On Tue, Jul 16, 2024 at 9:29 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > One related comment: > @@ -1219,8 +1219,14 @@ AlterPublicationTables(AlterPublicationStmt > *stmt, HeapTuple tup, > oldrel = palloc(sizeof(PublicationRelInfo)); > oldrel->whereClause = NULL; > oldrel->columns = NIL; > + > + /* > + * Data loss due to concurrency issues are avoided by locking > + * the relation in ShareRowExclusiveLock as described atop > + * OpenTableList. > + */ > oldrel->relation = table_open(oldrelid, > - ShareUpdateExclusiveLock); > + ShareRowExclusiveLock); > > Isn't it better to lock the required relations in RemovePublicationRelById()? >
On my CentOS VM, the test file '100_bugs.pl' takes ~11s without a patch and ~13.3s with a patch. So, 2 to 2.3s additional time for newly added tests. It isn't worth adding this much extra time for one bug fix. Can we combine table and schema tests into one single test and avoid inheritance table tests as the code for those will mostly follow the same path as a regular table? -- With Regards, Amit Kapila.
