On Sun, Jul 23, 2023 at 6:09 AM Ahmed Ibrahim <ahmed.ibr.has...@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? + dropStmt[strlen(dropStmt) - 2] = ' '; + dropStmt[strlen(dropStmt) - 1] = '\0'; Try to evaluate the strlen() once and reuse it. + appendPQExpBufferStr(ftStmt, dropStmt); + appendPQExpBufferStr(ftStmt, "WITH(FORCE);"); + te->dropStmt = ftStmt->data; + } + 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. [1]: https://wiki.postgresql.org/wiki/Creating_Clean_Patches [2]: https://wiki.postgresql.org/wiki/Submitting_a_Patch Best regards, Gurjeet http://Gurje.et