On Thu, Jul 7, 2022 at 10:58 AM Fujii Masao <masao.fu...@oss.nttdata.com> wrote: > But if many think that it's worth adding the test, I will give a try. But > even in that case, I think it's better to commit the proposed patch at first > to fix the bug, and then to write the patch adding the test.
I don't think that we necessarily need to have a test for this patch. It's true that we don't really have good test coverage of write-ahead logging and recovery, but this doesn't seem like the most important thing to be testing in that area, either, and developing stable tests for stuff like this can be a lot of work. I do kind of feel like the patch is fixing two separate bugs. The change to SendBaseBackup() is fixing the problem that, because there's SQL access on replication connections, we could try to start a backup in the middle of another backup by mixing and matching the two different methods of doing backups. The change to do_pg_abort_backup() is fixing the fact that, after aborting a base backup, we don't reset the session state properly so that another backup can be tried afterwards. I don't know if it's worth committing them separately - they are very small fixes. But it would probably at least be good to highlight in the commit message that there are two different issues. -- Robert Haas EDB: http://www.enterprisedb.com
