I have addressed all your comments except for the tests.
I have tried adding test cases but I wasn't able to do it as it's in my mind. I am not able to do things like having connections to the database and trying to force the restore, then it will complete successfully otherwise it shows errors.
In the meantime I will continue trying to do the test cases. If anyone can help on that, I will appreciate it.
Thanks
On Thu, Jul 27, 2023 at 1:36 AM Gurjeet Singh <gurjeet@singh.im> wrote:
On Sun, Jul 23, 2023 at 6:09 AM Ahmed Ibrahim <ahmed.ibr.hashim@gmail.com> wrote: > > Hi everyone, > > I have been working on this. This is a proposed patch for it so we have a force option for DROPping the database. > > I'd appreciate it if anyone can review.
Hi Ahmed,
Thanks for working on this patch!
+ + int force;
That extra blank line is unnecessary.
Using the bool data type, instead of int, for this option would've more natural.
+ if (ropt->force){
Postgres coding style is to place the curly braces on a new line, by themselves.
+ char *dropStmt = pg_strdup(te->dropStmt);
See if you can use pnstrdup(). Using that may obviate the need for doing the null-placement acrobatics below.
+ PQExpBuffer ftStmt = createPQExpBuffer();
What does the 'ft' stand for in this variable's name?
Remove the extra trailing whitespace on that last blank line.
I think this whole code block needs to be protected by an 'if (ropt->createDB)' check, like it's done about 20 lines above this hunk. Otherwise, we may be appending 'WITH (FORCE)' for the DROP command of a different (not a database) object type.
Also, you may want to check that the target database version is one that supports WITH force option. This command will fail for anything before v13.
The patch needs doc changes (pg_restore.sgml). And it needs to mention --force option in the help output, as well (usage() function).
Can you please see if you can add appropriate test case for this. The committers may insist on it, when reviewing.
Here are a couple of helpful links on how to prepare and submit patches to the community. You may not need to strictly adhere to these, but try to pick up a few recommendations that would make the reviewer's job a bit easier.