pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedSet AUTOCOMMIT to on in script output by pg_dump
15+ messages / 6 participants
[nested] [flat]
Commits from this thread:
2025-01-25 76601c0c88 aaa0f06193 53771e44df c768f54a25 4106942f00 d83a108c10 Doc: recommend "psql -X" for restoring pg_dump scripts.
* Set AUTOCOMMIT to on in script output by pg_dump
@ 2024-10-09 02:10 Shinya Kato <Shinya11.Kato@oss.nttdata.com>
0 siblings, 1 reply; 15+ messages in thread
From: Shinya Kato @ 2024-10-09 02:10 UTC (permalink / raw)
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Hi hackers!
When SQL scripts created with pg_dump/pg_dumpall/pg_restore are executed
in psql with AUTOCOMMIT turned off, they will not succeed in many cases.
This is because the script contains SQL statements that cannot be
executed within a transaction block.
If you simply add set AUTOCOMMIT on to the scripts created by
pg_dump/pg_dumpall/pg_restore, they will work fine.
A patch is attached
No documentation has been added as we could not find any documentation
on the details in the script.
Do you think?
Regards,
Shinya Kato
NTT DATA GROUP CORPORATION
Attachments:
v1-0001-Set-AUTOCOMMIT-to-on-in-script-output-by-pg_dump.patch (text/x-diff, 1.6K) download
From e5b4a6ab95ab5c798ef8c7f7062fb764a74de37a Mon Sep 17 00:00:00 2001
From: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
Date: Wed, 9 Oct 2024 10:28:31 +0900
Subject: [PATCH v1] Set AUTOCOMMIT to on in script output by pg_dump
---
src/bin/pg_dump/pg_backup_archiver.c | 7 +++++++
src/bin/pg_dump/pg_dumpall.c | 6 ++++++
2 files changed, 13 insertions(+)
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 8c20c263c4..b1077a5521 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -472,6 +472,13 @@ RestoreArchive(Archive *AHX)
ahprintf(AH, "BEGIN;\n\n");
}
+ /*
+ * Set AUTOCOMMIT to on when dumping a plain-text file to prevent errors
+ * with SQL statements that cannot be executed inside transaction block.
+ */
+ if (AH->format == archNull || ropt->filename != NULL)
+ ahprintf(AH, "\\set AUTOCOMMIT on\n");
+
/*
* Establish important parameter values right away.
*/
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index e3ad8fb295..006d0201bc 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -553,6 +553,12 @@ main(int argc, char *argv[])
* database we're connected to at the moment is fine.
*/
+ /*
+ * Set AUTOCOMMIT to on to prevent errors with SQL statements that cannot be
+ * executed inside transaction block.
+ */
+ fprintf(OPF, "\\set AUTOCOMMIT on\n\n");
+
/* Restore will need to write to the target cluster */
fprintf(OPF, "SET default_transaction_read_only = off;\n\n");
--
2.43.0
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2024-10-09 03:03 Yugo Nagata <nagata@sraoss.co.jp>
parent: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
0 siblings, 1 reply; 15+ messages in thread
From: Yugo Nagata @ 2024-10-09 03:03 UTC (permalink / raw)
To: Shinya Kato <Shinya11.Kato@oss.nttdata.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Wed, 09 Oct 2024 11:10:37 +0900
Shinya Kato <Shinya11.Kato@oss.nttdata.com> wrote:
> Hi hackers!
>
> When SQL scripts created with pg_dump/pg_dumpall/pg_restore are executed
> in psql with AUTOCOMMIT turned off, they will not succeed in many cases.
> This is because the script contains SQL statements that cannot be
> executed within a transaction block.
>
> If you simply add set AUTOCOMMIT on to the scripts created by
> pg_dump/pg_dumpall/pg_restore, they will work fine.
> A patch is attached
>
> No documentation has been added as we could not find any documentation
> on the details in the script.
>
> Do you think?
I am not sure if it is good to include psql's meta-command in pg_dump/pg_dumpall
results. Can we assume users will always use psql to restore the pg_dump results?
Regards,
Yugo Nagata
--
Yugo Nagata <nagata@sraoss.co.jp>
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2024-10-09 03:53 David G. Johnston <david.g.johnston@gmail.com>
parent: Yugo Nagata <nagata@sraoss.co.jp>
0 siblings, 2 replies; 15+ messages in thread
From: David G. Johnston @ 2024-10-09 03:53 UTC (permalink / raw)
To: Yugo Nagata <nagata@sraoss.co.jp>; +Cc: Shinya Kato <Shinya11.Kato@oss.nttdata.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Tuesday, October 8, 2024, Yugo Nagata <nagata@sraoss.co.jp> wrote:
> On Wed, 09 Oct 2024 11:10:37 +0900
> Shinya Kato <Shinya11.Kato@oss.nttdata.com> wrote:
>
> > Hi hackers!
> >
> > When SQL scripts created with pg_dump/pg_dumpall/pg_restore are executed
> > in psql with AUTOCOMMIT turned off, they will not succeed in many cases.
> > This is because the script contains SQL statements that cannot be
> > executed within a transaction block.
> >
> > If you simply add set AUTOCOMMIT on to the scripts created by
> > pg_dump/pg_dumpall/pg_restore, they will work fine.
> > A patch is attached
> >
>
> I am not sure if it is good to include psql's meta-command in
> pg_dump/pg_dumpall
> results. Can we assume users will always use psql to restore the pg_dump
> results?
Agreed. If we aren’t already outputting psql-only stuff I am a strong -1
for making this the first such case.
It would be nice to describe exactly when there is a problem as well since
very few things require being outside of a transaction. There might be
documentation or code patches possible here to improve matters (like adding
a switch to output begin/commit in the places we’re a user might want
single-transaction behavior) but this approach breaks well-established
encapsulation and overrides user expectations in a bad way (since
autocommit=on is the default they choose to turn it off so turning it back
on silently - not even documented - is bad.)
David J.
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2024-10-09 03:58 Tatsuo Ishii <ishii@postgresql.org>
parent: David G. Johnston <david.g.johnston@gmail.com>
1 sibling, 0 replies; 15+ messages in thread
From: Tatsuo Ishii @ 2024-10-09 03:58 UTC (permalink / raw)
To: david.g.johnston@gmail.com; +Cc: nagata@sraoss.co.jp; Shinya11.Kato@oss.nttdata.com; pgsql-hackers@lists.postgresql.org
>> I am not sure if it is good to include psql's meta-command in
>> pg_dump/pg_dumpall
>> results. Can we assume users will always use psql to restore the pg_dump
>> results?
>
>
> Agreed. If we aren¢t already outputting psql-only stuff I am a strong -1
> for making this the first such case.
I think the pg_dumpall output already includes "\connect".
> It would be nice to describe exactly when there is a problem as well since
> very few things require being outside of a transaction. There might be
> documentation or code patches possible here to improve matters (like adding
> a switch to output begin/commit in the places we¢re a user might want
> single-transaction behavior) but this approach breaks well-established
> encapsulation and overrides user expectations in a bad way (since
> autocommit=on is the default they choose to turn it off so turning it back
> on silently - not even documented - is bad.)
+1.
Best reagards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2024-10-09 04:10 Tom Lane <tgl@sss.pgh.pa.us>
parent: David G. Johnston <david.g.johnston@gmail.com>
1 sibling, 1 reply; 15+ messages in thread
From: Tom Lane @ 2024-10-09 04:10 UTC (permalink / raw)
To: David G. Johnston <david.g.johnston@gmail.com>; +Cc: Yugo Nagata <nagata@sraoss.co.jp>; Shinya Kato <Shinya11.Kato@oss.nttdata.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Tuesday, October 8, 2024, Yugo Nagata <nagata@sraoss.co.jp> wrote:
>> On Wed, 09 Oct 2024 11:10:37 +0900
>> Shinya Kato <Shinya11.Kato@oss.nttdata.com> wrote:
>>> When SQL scripts created with pg_dump/pg_dumpall/pg_restore are executed
>>> in psql with AUTOCOMMIT turned off, they will not succeed in many cases.
> Agreed. If we aren’t already outputting psql-only stuff I am a strong -1
> for making this the first such case.
I really doubt that this is the only way in which you can break a
pg_dump script by executing it in a non-default psql environment.
We'd likely be better advised to spend some documentation effort
recommending that pg_dump scripts be executed under "psql --no-psqlrc".
If AUTOCOMMIT were a mainstream feature then maybe it'd be worth
doing something about this, but IMO it's a deprecated backwater,
so I'm not very excited about it.
If we do want to do something about it, the patch needs more thought
about where to put the additional output. As an example, it looks
like it breaks the expectation that pg_dump-to-text should generate
output identical to pg_dump-to-archive followed by pg_restore-to-text.
> ... but this approach breaks well-established
> encapsulation and overrides user expectations in a bad way (since
> autocommit=on is the default they choose to turn it off so turning it back
> on silently - not even documented - is bad.)
That particular angle doesn't bother me so much, because pg_dump
scripts already feel free to change search_path as well as a bunch
of other server parameters.
regards, tom lane
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2024-10-09 04:37 David G. Johnston <david.g.johnston@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 15+ messages in thread
From: David G. Johnston @ 2024-10-09 04:37 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Yugo Nagata <nagata@sraoss.co.jp>; Shinya Kato <Shinya11.Kato@oss.nttdata.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Tuesday, October 8, 2024, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "David G. Johnston" <david.g.johnston@gmail.com> writes:
> > On Tuesday, October 8, 2024, Yugo Nagata <nagata@sraoss.co.jp> wrote:
> >> On Wed, 09 Oct 2024 11:10:37 +0900
> >> Shinya Kato <Shinya11.Kato@oss.nttdata.com> wrote:
> >>> When SQL scripts created with pg_dump/pg_dumpall/pg_restore are
> executed
> >>> in psql with AUTOCOMMIT turned off, they will not succeed in many
> cases.
>
> > Agreed. If we aren’t already outputting psql-only stuff I am a strong -1
> > for making this the first such case.
>
> I really doubt that this is the only way in which you can break a
> pg_dump script by executing it in a non-default psql environment.
> We'd likely be better advised to spend some documentation effort
> recommending that pg_dump scripts be executed under "psql --no-psqlrc".
+1
Reinforcing that our output script basically assumes a default execution
environment seems worth mentioning even if it seems self-evident once it’s
said.
>
>
> ... but this approach breaks well-established
> > encapsulation and overrides user expectations in a bad way (since
> > autocommit=on is the default they choose to turn it off so turning it
> back
> > on silently - not even documented - is bad.)
>
> That particular angle doesn't bother me so much, because pg_dump
> scripts already feel free to change search_path as well as a bunch
> of other server parameters.
>
I wasn’t referring to the idea these should be restorable on non-PostgreSQL
systems though, only that if someone wanted to just open a connection in
their rust driver and send this text through that session it will (mostly?)
work.
pg_dumpall, though, is fundamentally tied to psql if databases are dumped,
if the resultant script has to be platform independently executable. I’m
open to a patch addressing this more narrowly but I’m still thinking that
we should be telling the user to use defaults instead of enforcing
ourselves.
David J.
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2024-10-09 06:15 Yugo Nagata <nagata@sraoss.co.jp>
parent: David G. Johnston <david.g.johnston@gmail.com>
0 siblings, 1 reply; 15+ messages in thread
From: Yugo Nagata @ 2024-10-09 06:15 UTC (permalink / raw)
To: David G. Johnston <david.g.johnston@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Shinya Kato <Shinya11.Kato@oss.nttdata.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Tue, 8 Oct 2024 21:37:38 -0700
"David G. Johnston" <david.g.johnston@gmail.com> wrote:
> On Tuesday, October 8, 2024, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> > "David G. Johnston" <david.g.johnston@gmail.com> writes:
> > > On Tuesday, October 8, 2024, Yugo Nagata <nagata@sraoss.co.jp> wrote:
> > >> On Wed, 09 Oct 2024 11:10:37 +0900
> > >> Shinya Kato <Shinya11.Kato@oss.nttdata.com> wrote:
> > >>> When SQL scripts created with pg_dump/pg_dumpall/pg_restore are
> > executed
> > >>> in psql with AUTOCOMMIT turned off, they will not succeed in many
> > cases.
> >
> > > Agreed. If we aren$B!G(Bt already outputting psql-only stuff I am a strong -1
> > > for making this the first such case.
> >
> > I really doubt that this is the only way in which you can break a
> > pg_dump script by executing it in a non-default psql environment.
> > We'd likely be better advised to spend some documentation effort
> > recommending that pg_dump scripts be executed under "psql --no-psqlrc".
>
>
> +1
>
> Reinforcing that our output script basically assumes a default execution
> environment seems worth mentioning even if it seems self-evident once it$B!G(Bs
> said.
+1
Regards,
Yugo Nagata
--
Yugo Nagata <nagata@sraoss.co.jp>
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2024-10-10 05:56 Shinya Kato <Shinya11.Kato@oss.nttdata.com>
parent: Yugo Nagata <nagata@sraoss.co.jp>
0 siblings, 2 replies; 15+ messages in thread
From: Shinya Kato @ 2024-10-10 05:56 UTC (permalink / raw)
To: Yugo Nagata <nagata@sraoss.co.jp>; +Cc: David G. Johnston <david.g.johnston@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Thank you all for the comments!
On 2024-10-09 15:15, Yugo Nagata wrote:
> On Tue, 8 Oct 2024 21:37:38 -0700
> "David G. Johnston" <david.g.johnston@gmail.com> wrote:
>
>> On Tuesday, October 8, 2024, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> > "David G. Johnston" <david.g.johnston@gmail.com> writes:
>> > > On Tuesday, October 8, 2024, Yugo Nagata <nagata@sraoss.co.jp> wrote:
>> > >> On Wed, 09 Oct 2024 11:10:37 +0900
>> > >> Shinya Kato <Shinya11.Kato@oss.nttdata.com> wrote:
>> > >>> When SQL scripts created with pg_dump/pg_dumpall/pg_restore are
>> > executed
>> > >>> in psql with AUTOCOMMIT turned off, they will not succeed in many
>> > cases.
>> >
>> > > Agreed. If we aren’t already outputting psql-only stuff I am a strong -1
>> > > for making this the first such case.
>> >
>> > I really doubt that this is the only way in which you can break a
>> > pg_dump script by executing it in a non-default psql environment.
>> > We'd likely be better advised to spend some documentation effort
>> > recommending that pg_dump scripts be executed under "psql --no-psqlrc".
>>
>>
>> +1
>>
>> Reinforcing that our output script basically assumes a default
>> execution
>> environment seems worth mentioning even if it seems self-evident once
>> it’s
>> said.
>
> +1
While adding to the documentation is sufficient if users use it
correctly, users often behave unexpectedly. My intention was to
implement it in a way that works without issues even if misused.
However, since the prevailing opinion seems to favor simply updating the
documentation, I will proceed with that approach.
A new patch is attached.
I am not a native English, so corrections to the texts are welcome.
--
Regards,
Shinya Kato
NTT DATA GROUP CORPORATION
Attachments:
v2-0001-doc-Add-no-psqlrc-in-pg_dump-pg_dumpall-docs.patch (text/x-diff, 6.3K) download
From f2ac5156b8fc02b6b529d3aeef16d80f21bbe08c Mon Sep 17 00:00:00 2001
From: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
Date: Thu, 10 Oct 2024 14:50:39 +0900
Subject: [PATCH v2] doc: Add --no-psqlrc in pg_dump/pg_dumpall docs
---
doc/src/sgml/backup.sgml | 16 +++++++++-------
doc/src/sgml/ref/pg_dump.sgml | 9 ++++++++-
doc/src/sgml/ref/pg_dumpall.sgml | 11 ++++++++++-
3 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index e4e4c56cf1..21ad34f4e0 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -106,10 +106,10 @@ pg_dump <replaceable class="parameter">dbname</replaceable> > <replaceable cl
<para>
Text files created by <application>pg_dump</application> are intended to
- be read in by the <application>psql</application> program. The
- general command form to restore a dump is
+ be read in by the <application>psql</application> program with its default
+ settings. The general command form to restore a dump is
<synopsis>
-psql <replaceable class="parameter">dbname</replaceable> < <replaceable class="parameter">dumpfile</replaceable>
+psql --no-psqlrc <replaceable class="parameter">dbname</replaceable> < <replaceable class="parameter">dumpfile</replaceable>
</synopsis>
where <replaceable class="parameter">dumpfile</replaceable> is the
file output by the <application>pg_dump</application> command. The database <replaceable
@@ -117,7 +117,9 @@ psql <replaceable class="parameter">dbname</replaceable> < <replaceable class
command, so you must create it yourself from <literal>template0</literal>
before executing <application>psql</application> (e.g., with
<literal>createdb -T template0 <replaceable
- class="parameter">dbname</replaceable></literal>). <application>psql</application>
+ class="parameter">dbname</replaceable></literal>). To use
+ <application>psql</application> with its default settings, use the
+ <option>--no-psqlrc</option> option. <application>psql</application>
supports options similar to <application>pg_dump</application> for specifying
the database server to connect to and the user name to use. See
the <xref linkend="app-psql"/> reference page for more information.
@@ -141,7 +143,7 @@ psql <replaceable class="parameter">dbname</replaceable> < <replaceable class
behavior and have <application>psql</application> exit with an
exit status of 3 if an SQL error occurs:
<programlisting>
-psql --set ON_ERROR_STOP=on <replaceable>dbname</replaceable> < <replaceable>dumpfile</replaceable>
+psql --no-psqlrc --set ON_ERROR_STOP=on <replaceable>dbname</replaceable> < <replaceable>dumpfile</replaceable>
</programlisting>
Either way, you will only have a partially restored database.
Alternatively, you can specify that the whole dump should be
@@ -160,7 +162,7 @@ psql --set ON_ERROR_STOP=on <replaceable>dbname</replaceable> < <replaceable>
write to or read from pipes makes it possible to dump a database
directly from one server to another, for example:
<programlisting>
-pg_dump -h <replaceable>host1</replaceable> <replaceable>dbname</replaceable> | psql -h <replaceable>host2</replaceable> <replaceable>dbname</replaceable>
+pg_dump -h <replaceable>host1</replaceable> <replaceable>dbname</replaceable> | psql --no-psqlrc -h <replaceable>host2</replaceable> <replaceable>dbname</replaceable>
</programlisting>
</para>
@@ -205,7 +207,7 @@ pg_dumpall > <replaceable>dumpfile</replaceable>
</synopsis>
The resulting dump can be restored with <application>psql</application>:
<synopsis>
-psql -f <replaceable class="parameter">dumpfile</replaceable> postgres
+psql --no-psqlrc -f <replaceable class="parameter">dumpfile</replaceable> postgres
</synopsis>
(Actually, you can specify any existing database name to start from,
but if you are loading into an empty cluster then <literal>postgres</literal>
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index ffc29b04fb..b6c33c3c2f 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -1636,6 +1636,13 @@ CREATE DATABASE foo WITH TEMPLATE template0;
option will be automatically enabled by the subscriber if the subscription
had been originally created with <literal>two_phase = true</literal> option.
</para>
+
+ <para>
+ It is generally recommended to use the <option>--no-psqlrc</option> option
+ when restoring a database from a <application>pg_dump</application> script
+ to ensure a clean restore process and prevent potential conflicts with
+ existing <application>psql</application> configurations.
+ </para>
</refsect1>
<refsect1 id="pg-dump-examples" xreflabel="Examples">
@@ -1653,7 +1660,7 @@ CREATE DATABASE foo WITH TEMPLATE template0;
<literal>newdb</literal>:
<screen>
-<prompt>$</prompt> <userinput>psql -d newdb -f db.sql</userinput>
+<prompt>$</prompt> <userinput>psql --no-psqlrc -d newdb -f db.sql</userinput>
</screen>
</para>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 4d7c046468..f8db63326a 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -817,6 +817,15 @@ exclude database <replaceable class="parameter">PATTERN</replaceable>
database creation will fail for databases in non-default
locations.
</para>
+
+ <para>
+ It is generally recommended to use the <option>--no-psqlrc</option> option
+ when restoring a database from a <application>pg_dumpall</application> script
+ to ensure a clean restore process and prevent potential conflicts with
+ existing <application>psql</application> configurations. Additionally,
+ because the script includes <application>psql</application> meta-commands,
+ it is incompatible with clients other than <application>psql</application>.
+ </para>
</refsect1>
@@ -833,7 +842,7 @@ exclude database <replaceable class="parameter">PATTERN</replaceable>
<para>
To restore database(s) from this file, you can use:
<screen>
-<prompt>$</prompt> <userinput>psql -f db.out postgres</userinput>
+<prompt>$</prompt> <userinput>psql --no-psqlrc -f db.out postgres</userinput>
</screen>
It is not important to which database you connect here since the
script file created by <application>pg_dumpall</application> will
--
2.43.0
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2024-10-17 00:32 Shinya Kato <Shinya11.Kato@oss.nttdata.com>
parent: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
1 sibling, 0 replies; 15+ messages in thread
From: Shinya Kato @ 2024-10-17 00:32 UTC (permalink / raw)
To: Yugo Nagata <nagata@sraoss.co.jp>; +Cc: David G. Johnston <david.g.johnston@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On 2024-10-10 14:56, Shinya Kato wrote:
> A new patch is attached.
> I am not a native English, so corrections to the texts are welcome.
I created a commit fest entry.
https://commitfest.postgresql.org/50/5306/
--
Regards,
Shinya Kato
NTT DATA GROUP CORPORATION
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2025-01-17 15:45 Robert Treat <rob@xzilla.net>
parent: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
1 sibling, 1 reply; 15+ messages in thread
From: Robert Treat @ 2025-01-17 15:45 UTC (permalink / raw)
To: Shinya Kato <Shinya11.Kato@oss.nttdata.com>; +Cc: Yugo Nagata <nagata@sraoss.co.jp>; David G. Johnston <david.g.johnston@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Oct 10, 2024 at 1:56 AM Shinya Kato
<Shinya11.Kato@oss.nttdata.com> wrote:
> Thank you all for the comments!
<snip>
> While adding to the documentation is sufficient if users use it
> correctly, users often behave unexpectedly. My intention was to
> implement it in a way that works without issues even if misused.
> However, since the prevailing opinion seems to favor simply updating the
> documentation, I will proceed with that approach.
>
> A new patch is attached.
> I am not a native English, so corrections to the texts are welcome.
>
This looks pretty good to me. I think there are a couple of minor
grammar changes that could be made, and I think the pg_dumpall section
could use a couple tweaks, specifically 1) not all incantations of
pg_dumpall emit psql meta commands (-g comes to mind quickly) and ISTR
some non-psql clients honor psql meta commands, so I would lessen the
language around incompatibility, and 2) I think adding an explicit -f
for the database name in the pg_dumpall is clearer, and mirrors the
pg_dump example.
suggested diffs attached, let me know if you would like a consolidated patch
Robert Treat
https://xzilla.net
Attachments:
xzilla-pg_dump-no-psqlrc.diff (application/octet-stream, 2.2K) download
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 21ad34f4e0..15c8cdb8aa 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -106,7 +106,7 @@ pg_dump <replaceable class="parameter">dbname</replaceable> > <replaceable cl
<para>
Text files created by <application>pg_dump</application> are intended to
- be read in by the <application>psql</application> program with its default
+ be read in by the <application>psql</application> program using its default
settings. The general command form to restore a dump is
<synopsis>
psql --no-psqlrc <replaceable class="parameter">dbname</replaceable> < <replaceable class="parameter">dumpfile</replaceable>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 9208387ec1..636c27fe17 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -823,8 +823,8 @@ exclude database <replaceable class="parameter">PATTERN</replaceable>
when restoring a database from a <application>pg_dumpall</application> script
to ensure a clean restore process and prevent potential conflicts with
existing <application>psql</application> configurations. Additionally,
- because the script includes <application>psql</application> meta-commands,
- it is incompatible with clients other than <application>psql</application>.
+ because the script can include <application>psql</application> meta-commands,
+ it may be incompatible with clients other than <application>psql</application>.
</para>
</refsect1>
@@ -842,9 +842,9 @@ exclude database <replaceable class="parameter">PATTERN</replaceable>
<para>
To restore database(s) from this file, you can use:
<screen>
-<prompt>$</prompt> <userinput>psql --no-psqlrc -f db.out postgres</userinput>
+<prompt>$</prompt> <userinput>psql --no-psqlrc -f db.out -d postgres</userinput>
</screen>
- It is not important to which database you connect here since the
+ It is not important which database you connect to here since the
script file created by <application>pg_dumpall</application> will
contain the appropriate commands to create and connect to the saved
databases. An exception is that if you specified <option>--clean</option>,
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2025-01-17 20:11 Tom Lane <tgl@sss.pgh.pa.us>
parent: Robert Treat <rob@xzilla.net>
0 siblings, 1 reply; 15+ messages in thread
From: Tom Lane @ 2025-01-17 20:11 UTC (permalink / raw)
To: Robert Treat <rob@xzilla.net>; +Cc: Shinya Kato <Shinya11.Kato@oss.nttdata.com>; Yugo Nagata <nagata@sraoss.co.jp>; David G. Johnston <david.g.johnston@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Robert Treat <rob@xzilla.net> writes:
> suggested diffs attached, let me know if you would like a consolidated patch
Sadly, the cfbot is now confused since it doesn't understand the idea
of an incremental patch. Somebody please post a consolidated patch.
For myself, I'd suggest writing the examples with -X not --no-psqlrc.
-X is shorter, it's more likely to be what people would actually
type, and it's not jarringly inconsistent with the adjacent use
of -h and other short-form switches. We can write the added
paragraphs like
It is generally recommended to use the <option>-X</option>
(<option>--no-psqlrc</option>) option when restoring a database ...
to provide clarity about what the switch does.
regards, tom lane
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2025-01-22 13:01 Shinya Kato <Shinya11.Kato@oss.nttdata.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 15+ messages in thread
From: Shinya Kato @ 2025-01-22 13:01 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Treat <rob@xzilla.net>; Yugo Nagata <nagata@sraoss.co.jp>; David G. Johnston <david.g.johnston@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Hi, thank you for the reviews.
On 2025-01-18 00:45, Robert Treat wrote:
> This looks pretty good to me. I think there are a couple of minor
> grammar changes that could be made, and I think the pg_dumpall section
> could use a couple tweaks, specifically 1) not all incantations of
> pg_dumpall emit psql meta commands (-g comes to mind quickly) and ISTR
> some non-psql clients honor psql meta commands, so I would lessen the
> language around incompatibility, and 2) I think adding an explicit -f
> for the database name in the pg_dumpall is clearer, and mirrors the
> pg_dump example.
Thanks, I had no reason to oppose it, so I reflected that in the patch.
On 2025-01-18 05:11, Tom Lane wrote:
> Robert Treat <rob@xzilla.net> writes:
>> suggested diffs attached, let me know if you would like a consolidated
>> patch
>
> Sadly, the cfbot is now confused since it doesn't understand the idea
> of an incremental patch. Somebody please post a consolidated patch.
I've attached new consolidated patch.
> For myself, I'd suggest writing the examples with -X not --no-psqlrc.
> -X is shorter, it's more likely to be what people would actually
> type, and it's not jarringly inconsistent with the adjacent use
> of -h and other short-form switches. We can write the added
> paragraphs like
>
> It is generally recommended to use the <option>-X</option>
> (<option>--no-psqlrc</option>) option when restoring a database ...
>
> to provide clarity about what the switch does.
I agree to it and fixed the patch.
--
Regards,
Shinya Kato
NTT DATA GROUP CORPORATION
Attachments:
v3-0001-doc-Add-no-psqlrc-in-pg_dump-pg_dumpall-docs.patch (text/x-diff, 6.6K) download
From 664f89f8467f356024fe6e4e4ac0369b65238d41 Mon Sep 17 00:00:00 2001
From: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
Date: Wed, 22 Jan 2025 21:53:19 +0900
Subject: [PATCH v3] doc: Add --no-psqlrc in pg_dump pg_dumpall docs
---
doc/src/sgml/backup.sgml | 16 +++++++++-------
doc/src/sgml/ref/pg_dump.sgml | 10 +++++++++-
doc/src/sgml/ref/pg_dumpall.sgml | 14 ++++++++++++--
3 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index e4e4c56cf1..dd38a8b220 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -106,10 +106,10 @@ pg_dump <replaceable class="parameter">dbname</replaceable> > <replaceable cl
<para>
Text files created by <application>pg_dump</application> are intended to
- be read in by the <application>psql</application> program. The
- general command form to restore a dump is
+ be read in by the <application>psql</application> program using its default
+ settings. The general command form to restore a dump is
<synopsis>
-psql <replaceable class="parameter">dbname</replaceable> < <replaceable class="parameter">dumpfile</replaceable>
+psql -X <replaceable class="parameter">dbname</replaceable> < <replaceable class="parameter">dumpfile</replaceable>
</synopsis>
where <replaceable class="parameter">dumpfile</replaceable> is the
file output by the <application>pg_dump</application> command. The database <replaceable
@@ -117,7 +117,9 @@ psql <replaceable class="parameter">dbname</replaceable> < <replaceable class
command, so you must create it yourself from <literal>template0</literal>
before executing <application>psql</application> (e.g., with
<literal>createdb -T template0 <replaceable
- class="parameter">dbname</replaceable></literal>). <application>psql</application>
+ class="parameter">dbname</replaceable></literal>). To use
+ <application>psql</application> with its default settings, use the
+ <option>-X</option> (<option>--no-psqlrc</option>) option. <application>psql</application>
supports options similar to <application>pg_dump</application> for specifying
the database server to connect to and the user name to use. See
the <xref linkend="app-psql"/> reference page for more information.
@@ -141,7 +143,7 @@ psql <replaceable class="parameter">dbname</replaceable> < <replaceable class
behavior and have <application>psql</application> exit with an
exit status of 3 if an SQL error occurs:
<programlisting>
-psql --set ON_ERROR_STOP=on <replaceable>dbname</replaceable> < <replaceable>dumpfile</replaceable>
+psql -X --set ON_ERROR_STOP=on <replaceable>dbname</replaceable> < <replaceable>dumpfile</replaceable>
</programlisting>
Either way, you will only have a partially restored database.
Alternatively, you can specify that the whole dump should be
@@ -160,7 +162,7 @@ psql --set ON_ERROR_STOP=on <replaceable>dbname</replaceable> < <replaceable>
write to or read from pipes makes it possible to dump a database
directly from one server to another, for example:
<programlisting>
-pg_dump -h <replaceable>host1</replaceable> <replaceable>dbname</replaceable> | psql -h <replaceable>host2</replaceable> <replaceable>dbname</replaceable>
+pg_dump -h <replaceable>host1</replaceable> <replaceable>dbname</replaceable> | psql -X -h <replaceable>host2</replaceable> <replaceable>dbname</replaceable>
</programlisting>
</para>
@@ -205,7 +207,7 @@ pg_dumpall > <replaceable>dumpfile</replaceable>
</synopsis>
The resulting dump can be restored with <application>psql</application>:
<synopsis>
-psql -f <replaceable class="parameter">dumpfile</replaceable> postgres
+psql -X -f <replaceable class="parameter">dumpfile</replaceable> postgres
</synopsis>
(Actually, you can specify any existing database name to start from,
but if you are loading into an empty cluster then <literal>postgres</literal>
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index d66e901f51..8b572cdec0 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -1636,6 +1636,14 @@ CREATE DATABASE foo WITH TEMPLATE template0;
option will be automatically enabled by the subscriber if the subscription
had been originally created with <literal>two_phase = true</literal> option.
</para>
+
+ <para>
+ It is generally recommended to use the <option>-X</option>
+ (<option>--no-psqlrc</option>) option when restoring a database from a
+ <application>pg_dump</application> script to ensure a clean restore process
+ and prevent potential conflicts with existing <application>psql</application>
+ configurations.
+ </para>
</refsect1>
<refsect1 id="pg-dump-examples" xreflabel="Examples">
@@ -1653,7 +1661,7 @@ CREATE DATABASE foo WITH TEMPLATE template0;
<literal>newdb</literal>:
<screen>
-<prompt>$</prompt> <userinput>psql -d newdb -f db.sql</userinput>
+<prompt>$</prompt> <userinput>psql -X -d newdb -f db.sql</userinput>
</screen>
</para>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 014f279258..bfbe305b08 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -817,6 +817,16 @@ exclude database <replaceable class="parameter">PATTERN</replaceable>
database creation will fail for databases in non-default
locations.
</para>
+
+ <para>
+ It is generally recommended to use the <option>-X</option>
+ (<option>--no-psqlrc</option>) option when restoring a database from a
+ <application>pg_dumpall</application> script to ensure a clean restore
+ process and prevent potential conflicts with existing
+ <application>psql</application> configurations. Additionally,
+ because the script can include <application>psql</application> meta-commands,
+ it may be incompatible with clients other than <application>psql</application>.
+ </para>
</refsect1>
@@ -833,9 +843,9 @@ exclude database <replaceable class="parameter">PATTERN</replaceable>
<para>
To restore database(s) from this file, you can use:
<screen>
-<prompt>$</prompt> <userinput>psql -f db.out postgres</userinput>
+<prompt>$</prompt> <userinput>psql -X -f db.out -d postgres</userinput>
</screen>
- It is not important to which database you connect here since the
+ It is not important which database you connect to here since the
script file created by <application>pg_dumpall</application> will
contain the appropriate commands to create and connect to the saved
databases. An exception is that if you specified <option>--clean</option>,
--
2.39.3
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2025-01-22 21:33 Robert Treat <rob@xzilla.net>
parent: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
0 siblings, 1 reply; 15+ messages in thread
From: Robert Treat @ 2025-01-22 21:33 UTC (permalink / raw)
To: Shinya Kato <Shinya11.Kato@oss.nttdata.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Yugo Nagata <nagata@sraoss.co.jp>; David G. Johnston <david.g.johnston@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Wed, Jan 22, 2025 at 8:02 AM Shinya Kato
<Shinya11.Kato@oss.nttdata.com> wrote:
>
> Hi, thank you for the reviews.
>
> On 2025-01-18 00:45, Robert Treat wrote:
> > This looks pretty good to me. I think there are a couple of minor
> > grammar changes that could be made, and I think the pg_dumpall section
> > could use a couple tweaks, specifically 1) not all incantations of
> > pg_dumpall emit psql meta commands (-g comes to mind quickly) and ISTR
> > some non-psql clients honor psql meta commands, so I would lessen the
> > language around incompatibility, and 2) I think adding an explicit -f
> > for the database name in the pg_dumpall is clearer, and mirrors the
> > pg_dump example.
>
> Thanks, I had no reason to oppose it, so I reflected that in the patch.
>
> On 2025-01-18 05:11, Tom Lane wrote:
> > Robert Treat <rob@xzilla.net> writes:
> >> suggested diffs attached, let me know if you would like a consolidated
> >> patch
> >
> > Sadly, the cfbot is now confused since it doesn't understand the idea
> > of an incremental patch. Somebody please post a consolidated patch.
>
> I've attached new consolidated patch.
>
> > For myself, I'd suggest writing the examples with -X not --no-psqlrc.
> > -X is shorter, it's more likely to be what people would actually
> > type, and it's not jarringly inconsistent with the adjacent use
> > of -h and other short-form switches. We can write the added
> > paragraphs like
> >
> > It is generally recommended to use the <option>-X</option>
> > (<option>--no-psqlrc</option>) option when restoring a database ...
> >
> > to provide clarity about what the switch does.
>
> I agree to it and fixed the patch.
>
LGTM
Robert Treat
https://xzilla.net
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2025-01-25 17:45 Tom Lane <tgl@sss.pgh.pa.us>
parent: Robert Treat <rob@xzilla.net>
0 siblings, 1 reply; 15+ messages in thread
From: Tom Lane @ 2025-01-25 17:45 UTC (permalink / raw)
To: Robert Treat <rob@xzilla.net>; +Cc: Shinya Kato <Shinya11.Kato@oss.nttdata.com>; Yugo Nagata <nagata@sraoss.co.jp>; David G. Johnston <david.g.johnston@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Robert Treat <rob@xzilla.net> writes:
> On Wed, Jan 22, 2025 at 8:02 AM Shinya Kato
> <Shinya11.Kato@oss.nttdata.com> wrote:
>> I agree to it and fixed the patch.
> LGTM
LGTM too. Pushed with a couple of very minor tweaks.
regards, tom lane
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Set AUTOCOMMIT to on in script output by pg_dump
@ 2025-01-27 00:23 Shinya Kato <Shinya11.Kato@oss.nttdata.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 15+ messages in thread
From: Shinya Kato @ 2025-01-27 00:23 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Robert Treat <rob@xzilla.net>; Yugo Nagata <nagata@sraoss.co.jp>; David G. Johnston <david.g.johnston@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On 2025-01-26 02:45, Tom Lane wrote:
> Robert Treat <rob@xzilla.net> writes:
>> On Wed, Jan 22, 2025 at 8:02 AM Shinya Kato
>> <Shinya11.Kato@oss.nttdata.com> wrote:
>>> I agree to it and fixed the patch.
>
>> LGTM
>
> LGTM too. Pushed with a couple of very minor tweaks.
>
> regards, tom lane
Thank you for pushing!
--
Regards,
Shinya Kato
NTT DATA GROUP CORPORATION
^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2025-01-27 00:23 UTC | newest]
Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-10-09 02:10 Set AUTOCOMMIT to on in script output by pg_dump Shinya Kato <Shinya11.Kato@oss.nttdata.com>
2024-10-09 03:03 ` Yugo Nagata <nagata@sraoss.co.jp>
2024-10-09 03:53 ` David G. Johnston <david.g.johnston@gmail.com>
2024-10-09 03:58 ` Tatsuo Ishii <ishii@postgresql.org>
2024-10-09 04:10 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-10-09 04:37 ` David G. Johnston <david.g.johnston@gmail.com>
2024-10-09 06:15 ` Yugo Nagata <nagata@sraoss.co.jp>
2024-10-10 05:56 ` Shinya Kato <Shinya11.Kato@oss.nttdata.com>
2024-10-17 00:32 ` Shinya Kato <Shinya11.Kato@oss.nttdata.com>
2025-01-17 15:45 ` Robert Treat <rob@xzilla.net>
2025-01-17 20:11 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-01-22 13:01 ` Shinya Kato <Shinya11.Kato@oss.nttdata.com>
2025-01-22 21:33 ` Robert Treat <rob@xzilla.net>
2025-01-25 17:45 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-01-27 00:23 ` Shinya Kato <Shinya11.Kato@oss.nttdata.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox