From: | jian he <jian(dot)universality(at)gmail(dot)com> |
---|---|
To: | Mahendra Singh Thalor <mahi6run(at)gmail(dot)com> |
Cc: | Srinath Reddy <srinath2133(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Non-text mode for pg_dumpall |
Date: | 2025-02-11 15:09:40 |
Message-ID: | CACJufxEAWxWovH_dys5A2z53OGpftX_jJRTd1y8eB3HaojY+1A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
hi.
review based on v16.
because of
https://postgr.es/m/CAFC+b6pWQiSL+3rvLxN9vhC8aONp4OV9c6u+BVD6kmWmDbd1WQ@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 | Content-Type | Size |
---|---|---|
v16_misc_changes.nocfbot | application/octet-stream | 6.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2025-02-11 15:14:04 | Re: should we have a fast-path planning for OLTP starjoins? |
Previous Message | Matheus Alcantara | 2025-02-11 14:51:28 | Re: explain analyze rows=%.0f |