Hi, I agree that —drop-cascade does not make sense for pg_dumpall, so I removed them.
> are we expecting more things to appear after the semi-colon? No, I was just trying to “reuse” original statement as much as possible. Append “\n” manually should also do the job, and I’ve updated the patch as you suggests.
0001-pg_dump-restore-add-drop-cascade-option.patch
Description: Binary data
> 2021年5月29日 上午2:39，Greg Sabino Mullane <htamf...@gmail.com> 写道： > > Overall the patch looks good, but I did notice a few small things: > > 1. In pg_dumpall.c, the section /* Add long options to the pg_dump argument > list */, we are now > passing along the --drop-cascade option. However, --clean is not passed in, > so > any call to pg_dumpall using --drop-cascade fails a the pg_dump step. You'll > note > that --if-exists it not passed along either; because we are dropping the > whole database, we don't > need to have pg_dump worry about dropping objects at all. So I think that > --drop-cascade should NOT be passed along from pg_dumpall to pg_dump. > > 2. I'm not even sure if --drop-cascade makes sense for pg_dumpall, as you > cannot cascade global things like databases and roles. > > 3. In the file pg_backup_archiver.c, the patch does a > stmtEnd = strstr(mark + strlen(buffer), ";");" and then spits > out things "past" the semicolon as the final %s in the appendPQExpBuffer > line. > I'm not clear why: are we expecting more things to appear after the > semi-colon? > Why not just append a "\n" manually as part of the previous %s? > > Cheers, > Greg > > The new status of this patch is: Waiting on Author
