hi.
review based on v16.

because of
https://postgr.es/m/cafc+b6pwqisl+3rvlxn9vhc8aonp4ov9c6u+bvd6kmwmdbd...@mail.gmail.com

in copy_global_file_to_out_file, now it is:
    if (strcmp(outfile, "-") == 0)
        OPF = stdout;
I am confused, why "-" means stdout.
``touch ./- `` command works fine.
i think dash is not special character, you may see
https://stackoverflow.com/a/40650391/15603477


+ /* Create a subdirectory with 'databases' name under main directory. */
+ if (mkdir(db_subdir, 0755) != 0)
+ pg_log_error("could not create subdirectory \"%s\": %m", db_subdir);
here we should use pg_fatal?


pg_log_info("executing %s", sqlstatement.data);
change to
pg_log_info("executing query: %s", sqlstatement.data);
message would be more similar to the next pg_log_error(...) message.


+ /*
+ * User is suggested to use single database dump for --list option.
+ */
+ if (opts->tocSummary)
+ pg_fatal("option -l/--list cannot be used when using dump of pg_dumpall");
maybe change to
+ pg_fatal("option -l/--list cannot be used when restoring multiple databases");

$BIN10/pg_restore --format=directory --list dir10_x
if the directory only has one database, then we can actually print out
the tocSummary.
if the directory has more than one database then pg_fatal.
To tolerate this corner case (only one database) means that pg_restore
--list requires a DB connection,
but I am not sure that is fine.
anyway, the attached patch allows this corner case.


PrintTOCSummary can only print out summary for a single database.
so we don't need to change PrintTOCSummary.


+ /*
+ * To restore multiple databases, -C (create database) option should
be specified
+ * or all databases should be created before pg_restore.
+ */
+ if (opts->createDB != 1)
+ pg_log_info("restoring dump of pg_dumpall without -C option, there
might be multiple databases in directory.");

we can change it to
+ if (opts->createDB != 1 && num_db_restore > 0)
+ pg_log_info("restoring multiple databases without -C option.");


Bug.
when pg_restore --globals-only can be applied when we are restoring a
single database (can be an output of pg_dump).


There are some tests per https://commitfest.postgresql.org/52/5495, I
will check it later.
The attached patch is the change for the above reviews.

Attachment: v16_misc_changes.nocfbot
Description: Binary data

Reply via email to