ON CONFLICT DO NOTHING on pg_dump

Started by Surafel Temesgenover 7 years ago15 messages

surafel3000@gmail.com

over 7 years ago

1 attachment(s)

Hello,

Sometimes I have to maintain two similar database and I have to update one
from the other and notice having the option to add ON CONFLICT DO NOTHING
clause to INSERT command in the dump data will allows pg_restore to be done
with free of ignore error.

The attache patch add --on-conflect-do-nothing option to pg_dump in order
to do the above.

regards

Surafel

Attachments:

pg_dump_onConflect_v1.pachapplication/octet-stream; name=pg_dump_onConflect_v1.pachDownload

diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 50809b4844..c929dd855b 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -789,6 +789,16 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--on-conflect-do-nothing</option></term>
+      <listitem>
+       <para>
+        add ON CONFLICT DO NOTHING clause in the INSERT commands.
+        This option is not valid unless <option>--inserts</option> is also specified.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>--lock-wait-timeout=<replaceable class="parameter">timeout</replaceable></option></term>
       <listitem>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 5d6fe9b87d..b0fb0e4cd4 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -326,6 +326,16 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--on-conflect-do-nothing</option></term>
+      <listitem>
+       <para>
+        add ON CONFLICT DO NOTHING clause in the INSERT commands.
+        This option is not valid unless <option>--inserts</option> is also specified.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>--lock-wait-timeout=<replaceable class="parameter">timeout</replaceable></option></term>
       <listitem>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index ceedd481fb..42cf441aaf 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -172,6 +172,7 @@ typedef struct _dumpOptions
 	char	   *outputSuperuser;
 
 	int			sequence_data;	/* dump sequence data even in schema-only mode */
+	int			do_nothing;
 } DumpOptions;
 
 /*
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index d6ceb72c05..17fb08ee3c 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -357,6 +357,7 @@ main(int argc, char **argv)
 		{"column-inserts", no_argument, &dopt.column_inserts, 1},
 		{"disable-dollar-quoting", no_argument, &dopt.disable_dollar_quoting, 1},
 		{"disable-triggers", no_argument, &dopt.disable_triggers, 1},
+		{"on-conflect-do-nothing", no_argument, &dopt.do_nothing, 1},
 		{"enable-row-security", no_argument, &dopt.enable_row_security, 1},
 		{"exclude-table-data", required_argument, NULL, 4},
 		{"if-exists", no_argument, &dopt.if_exists, 1},
@@ -619,6 +620,9 @@ main(int argc, char **argv)
 	if (dopt.if_exists && !dopt.outputClean)
 		exit_horribly(NULL, "option --if-exists requires option -c/--clean\n");
 
+	if (dopt.do_nothing && !dopt.dump_inserts)
+		exit_horribly(NULL, "option --on-conflect-do-nothing requires option --inserts\n");
+
 	/* Identify archive format to emit */
 	archiveFormat = parseArchiveFormat(format, &archiveMode);
 
@@ -981,6 +985,7 @@ help(const char *progname)
 	printf(_("  --exclude-table-data=TABLE   do NOT dump data for the named table(s)\n"));
 	printf(_("  --if-exists                  use IF EXISTS when dropping objects\n"));
 	printf(_("  --inserts                    dump data as INSERT commands, rather than COPY\n"));
+	printf(_("  --on-conflect-do-nothing     dump data as INSERT commands with on conflect do nothing\n"));
 	printf(_("  --no-comments                do not dump comments\n"));
 	printf(_("  --no-publications            do not dump publications\n"));
 	printf(_("  --no-security-labels         do not dump security label assignments\n"));
@@ -2047,9 +2052,13 @@ dumpTableData_insert(Archive *fout, void *dcontext)
 						break;
 				}
 			}
-			archputs(");\n", fout);
+
+			if (!dopt->do_nothing)
+				archputs(");\n", fout);
+			else
+				archputs(") ON CONFLICT DO NOTHING;\n", fout);
+
 		}
-
 		if (PQntuples(res) <= 0)
 		{
 			PQclear(res);

Ideriha, Takeshi

ideriha.takeshi@jp.fujitsu.com

over 7 years ago

In reply to: Surafel Temesgen (#1)

RE: ON CONFLICT DO NOTHING on pg_dump

From: Surafel Temesgen [mailto:surafel3000@gmail.com]
Subject: ON CONFLICT DO NOTHING on pg_dump

Sometimes I have to maintain two similar database and I have to update one from the other and notice having the option to add ON CONFLICT DO NOTHING clause to >INSERT command in the dump data will allows pg_restore to be done with free of ignore error.

Hi,
I feel like that on-conflict-do-nothing support is useful especially coupled with --data-only option.
Only the difference of data can be restored.

The attache patch add --on-conflect-do-nothing option to pg_dump in order to do the above.

The followings are some comments.

+ <term><option>--on-conflect-do-nothing</option></term>
Here's a typo: conflect -> conflict. This typo also applies to pg_dump.c

printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
+ printf(_(" --on-conflect-do-nothing dump data as INSERT commands with on conflect do nothing\n"));
printf(_(" --no-comments do not dump comments\n"));

The output of help should be in alphabetical order according to the convention. So changing the order seems logical.
Please apply my review to the documentation as well.
By the way, 4d6a854 breaks the patch on this point.

+        This option is not valid unless <option>--inserts</option> is also specified.
+       </para>

+       if (dopt.do_nothing && !dopt.dump_inserts)
+               exit_horribly(NULL, "option --on-conflect-do-nothing requires option --inserts\n");

How about mentioning --column-inserts? --on-conflict-do-nothing with --column-inserts should work.

Do you have any plan to support on-conlict-do-update? Supporting this seems to me complicated and take much time so I don't mind not implementing this.

What do you think about adding some test cases?
command_fails_like() at 001_basic.pl checks command fail pattern with invalid comnibation of option.
And 002_pg_dump.pl checks the feature iteself.

Regards,
Takeshi Ideriha

Nico Williams

nico@cryptonector.com

over 7 years ago

In reply to: Ideriha, Takeshi (#2)

Re: ON CONFLICT DO NOTHING on pg_dump

On Tue, Jun 12, 2018 at 09:05:23AM +0000, Ideriha, Takeshi wrote:

From: Surafel Temesgen [mailto:surafel3000@gmail.com]
Subject: ON CONFLICT DO NOTHING on pg_dump

Sometimes I have to maintain two similar database and I have to update one from the other and notice having the option to add ON CONFLICT DO NOTHING clause to >INSERT command in the dump data will allows pg_restore to be done with free of ignore error.

Hi,
I feel like that on-conflict-do-nothing support is useful especially coupled with --data-only option.
Only the difference of data can be restored.

But that's additive-only. Only missing rows are restored this way, and
differences are not addressed.

If you want restore to restore data properly and concurrently (as
opposed to renaming a new database into place or whatever) then you'd
want a) MERGE, b) dump to generate MERGE statements. A concurrent data
restore operation would be rather neat.

Nico
--

Surafel Temesgen

surafel3000@gmail.com

over 7 years ago

In reply to: Ideriha, Takeshi (#2)

1 attachment(s)

Re: ON CONFLICT DO NOTHING on pg_dump

On Tue, Jun 12, 2018 at 12:05 PM, Ideriha, Takeshi <
ideriha.takeshi@jp.fujitsu.com> wrote:
thank you for the review

Hi,
I feel like that on-conflict-do-nothing support is useful especially
coupled with --data-only option.
Only the difference of data can be restored.

The attache patch add --on-conflect-do-nothing option to pg_dump in order

to do the above.

The followings are some comments.

+ <term><option>--on-conflect-do-nothing</option></term>
Here's a typo: conflect -> conflict. This typo also applies to pg_dump.c

printf(_(" --inserts dump data as INSERT
commands, rather than COPY\n"));
+ printf(_(" --on-conflect-do-nothing dump data as INSERT
commands with on conflect do nothing\n"));
printf(_(" --no-comments do not dump comments\n"));

The output of help should be in alphabetical order according to the
convention. So changing the order seems logical.
Please apply my review to the documentation as well.
By the way, 4d6a854 breaks the patch on this point.
+        This option is not valid unless <option>--inserts</option> is
also specified.
+       </para>
+       if (dopt.do_nothing && !dopt.dump_inserts)
+               exit_horribly(NULL, "option --on-conflect-do-nothing
requires option --inserts\n");
How about mentioning --column-inserts? --on-conflict-do-nothing with
--column-inserts should work.

fixed

Do you have any plan to support on-conlict-do-update? Supporting this
seems to me complicated and take much time so I don't mind not implementing
this.

i agree its complicated and i don't have a plan to implementing it.

What do you think about adding some test cases?
command_fails_like() at 001_basic.pl checks command fail pattern with
invalid comnibation of option.
And 002_pg_dump.pl checks the feature iteself.

thank you for pointing me that i add basic test and it seems to me the rest
of the test is covered by column_inserts test

Show quoted text

Regards,
Takeshi Ideriha

Attachments:

pg_dump_onConflect_v2.pachapplication/octet-stream; name=pg_dump_onConflect_v2.pachDownload

diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 50809b4844..a36fd20328 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -894,6 +894,17 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--on-conflict-do-nothing</option></term>
+      <listitem>
+       <para>
+        Add ON CONFLICT DO NOTHING clause in the INSERT commands.
+        This option is not valid unless <option>--inserts</option> or 
+        <option>--column-inserts</option> is also specified.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>--quote-all-identifiers</option></term>
       <listitem>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 5d6fe9b87d..ff710e7568 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -431,6 +431,17 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--on-conflict-do-nothing</option></term>
+      <listitem>
+       <para>
+        Add ON CONFLICT DO NOTHING clause in the INSERT commands.
+        This option is not valid unless <option>--inserts</option> or 
+        <option>--column-inserts</option> is also specified.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>--quote-all-identifiers</option></term>
       <listitem>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index ceedd481fb..42cf441aaf 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -172,6 +172,7 @@ typedef struct _dumpOptions
 	char	   *outputSuperuser;
 
 	int			sequence_data;	/* dump sequence data even in schema-only mode */
+	int			do_nothing;
 } DumpOptions;
 
 /*
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index d6ceb72c05..facece2e6f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -378,6 +378,7 @@ main(int argc, char **argv)
 		{"no-unlogged-table-data", no_argument, &dopt.no_unlogged_table_data, 1},
 		{"no-subscriptions", no_argument, &dopt.no_subscriptions, 1},
 		{"no-sync", no_argument, NULL, 7},
+		{"on-conflict-do-nothing", no_argument, &dopt.do_nothing, 1},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -619,6 +620,9 @@ main(int argc, char **argv)
 	if (dopt.if_exists && !dopt.outputClean)
 		exit_horribly(NULL, "option --if-exists requires option -c/--clean\n");
 
+	if (dopt.do_nothing && !(dopt.dump_inserts || dopt.column_inserts))
+		exit_horribly(NULL, "option --on-conflict-do-nothing requires option --inserts or --column-inserts\n");
+
 	/* Identify archive format to emit */
 	archiveFormat = parseArchiveFormat(format, &archiveMode);
 
@@ -988,6 +992,7 @@ help(const char *progname)
 	printf(_("  --no-synchronized-snapshots  do not use synchronized snapshots in parallel jobs\n"));
 	printf(_("  --no-tablespaces             do not dump tablespace assignments\n"));
 	printf(_("  --no-unlogged-table-data     do not dump unlogged table data\n"));
+	printf(_("  --on-conflict-do-nothing     dump data as INSERT commands with ON CONFLICT DO NOTHING \n"));
 	printf(_("  --quote-all-identifiers      quote all identifiers, even if not key words\n"));
 	printf(_("  --load-via-partition-root    load partitions via the root table\n"));
 	printf(_("  --section=SECTION            dump named section (pre-data, data, or post-data)\n"));
@@ -2047,7 +2052,11 @@ dumpTableData_insert(Archive *fout, void *dcontext)
 						break;
 				}
 			}
-			archputs(");\n", fout);
+
+			if (!dopt->do_nothing)
+				archputs(");\n", fout);
+			else
+				archputs(") ON CONFLICT DO NOTHING;\n", fout);
 		}
 
 		if (PQntuples(res) <= 0)
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 55de7449b4..6574ac479c 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -78,6 +78,7 @@ static int	no_unlogged_table_data = 0;
 static int	no_role_passwords = 0;
 static int	server_version;
 static int	load_via_partition_root = 0;
+static int	on_conflict_do_nothing = 0;
 
 static char role_catalog[10];
 #define PG_AUTHID "pg_authid"
@@ -137,6 +138,7 @@ main(int argc, char *argv[])
 		{"no-subscriptions", no_argument, &no_subscriptions, 1},
 		{"no-sync", no_argument, NULL, 4},
 		{"no-unlogged-table-data", no_argument, &no_unlogged_table_data, 1},
+		{"on-conflict-do-nothing", no_argument, &on_conflict_do_nothing, 1},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -367,6 +369,12 @@ main(int argc, char *argv[])
 		exit_nicely(1);
 	}
 
+	if (on_conflict_do_nothing && !(inserts || column_inserts))
+	{
+		fprintf(stderr, _("%s: option --on-conflict-do-nothing requires option --inserts or --column-inserts\n"),
+				progname);
+		exit_nicely(1);
+	}
 	/*
 	 * If password values are not required in the dump, switch to using
 	 * pg_roles which is equally useful, just more likely to have unrestricted
@@ -390,6 +398,8 @@ main(int argc, char *argv[])
 		appendPQExpBufferStr(pgdumpopts, " --inserts");
 	if (no_tablespaces)
 		appendPQExpBufferStr(pgdumpopts, " --no-tablespaces");
+	if (on_conflict_do_nothing)
+		appendPQExpBufferStr(pgdumpopts, " --on-conflict-do-nothing");
 	if (quote_all_identifiers)
 		appendPQExpBufferStr(pgdumpopts, " --quote-all-identifiers");
 	if (load_via_partition_root)
@@ -620,6 +630,7 @@ help(void)
 	printf(_("  --no-sync                    do not wait for changes to be written safely to disk\n"));
 	printf(_("  --no-tablespaces             do not dump tablespace assignments\n"));
 	printf(_("  --no-unlogged-table-data     do not dump unlogged table data\n"));
+	printf(_("  --on-conflict-do-nothing     dump data as INSERT commands with ON CONFLICT DO NOTHING \n"));
 	printf(_("  --quote-all-identifiers      quote all identifiers, even if not key words\n"));
 	printf(_("  --load-via-partition-root    load partitions via the root table\n"));
 	printf(_("  --use-set-session-authorization\n"
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 8be5770ba4..8eaf556df6 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -4,7 +4,7 @@ use warnings;
 use Config;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 68;
+use Test::More tests => 72;
 
 my $tempdir       = TestLib::tempdir;
 my $tempdir_short = TestLib::tempdir_short;
@@ -122,6 +122,11 @@ command_fails_like(
 	qr/\Qpg_restore: unrecognized archive format "garbage";\E/,
 	'pg_dump: unrecognized archive format');
 
+command_fails_like(
+	[ 'pg_dump', '--on-conflict-do-nothing' ],
+	qr/\Qpg_dump: option --on-conflict-do-nothing requires option --inserts or --column-inserts\E/,
+	'pg_dump: option --on-conflict-do-nothing requires option --inserts or --column-inserts');
+
 # pg_dumpall command-line argument checks
 command_fails_like(
 	[ 'pg_dumpall', '-g', '-r' ],
@@ -145,3 +150,9 @@ command_fails_like(
 	[ 'pg_dumpall', '--if-exists' ],
 	qr/\Qpg_dumpall: option --if-exists requires option -c\/--clean\E/,
 	'pg_dumpall: option --if-exists requires option -c/--clean');
+
+command_fails_like(
+	[ 'pg_dumpall', '--on-conflict-do-nothing' ],
+	qr/\Qpg_dumpall: option --on-conflict-do-nothing requires option --inserts or --column-inserts\E/,
+	'pg_dumpall: option --on-conflict-do-nothing requires option --inserts or --column-inserts'
+);

Ideriha, Takeshi

ideriha.takeshi@jp.fujitsu.com

over 7 years ago

In reply to: Nico Williams (#3)

RE: ON CONFLICT DO NOTHING on pg_dump

-----Original Message-----
From: Nico Williams [mailto:nico@cryptonector.com]
On Tue, Jun 12, 2018 at 09:05:23AM +0000, Ideriha, Takeshi wrote:

From: Surafel Temesgen [mailto:surafel3000@gmail.com]
Subject: ON CONFLICT DO NOTHING on pg_dump

Sometimes I have to maintain two similar database and I have to update one from

the other and notice having the option to add ON CONFLICT DO NOTHING clause to

INSERT command in the dump data will allows pg_restore to be done with free of

ignore error.

Hi,
I feel like that on-conflict-do-nothing support is useful especially coupled with

--data-only option.

Only the difference of data can be restored.

But that's additive-only. Only missing rows are restored this way, and differences are
not addressed.

If you want restore to restore data properly and concurrently (as opposed to renaming
a new database into place or whatever) then you'd want a) MERGE, b) dump to
generate MERGE statements. A concurrent data restore operation would be rather
neat.

I agree with you though supporting MERGE or ON-CONFLICT-DO-UPDATE seems hard work.
Only ON-CONCLICT-DO-NOTHING use case may be narrow.

--
Takeshi

Ideriha, Takeshi

ideriha.takeshi@jp.fujitsu.com

over 7 years ago

In reply to: Surafel Temesgen (#4)

RE: ON CONFLICT DO NOTHING on pg_dump

Hi,

-----Original Message-----
From: Surafel Temesgen [mailto:surafel3000@gmail.com]
thank you for the review

Do you have any plan to support on-conlict-do-update? Supporting this seems
to me complicated and take much time so I don't mind not implementing this.

i agree its complicated and i don't have a plan to implementing it.

Sure.

thank you for pointing me that i add basic test and it seems to me the rest of the test
is covered by column_inserts test

Thank you for updating.
Just one comment for the code.

+ if (dopt.do_nothing && !(dopt.dump_inserts || dopt.column_inserts))
I think you can remove dopt.column_inserts here because at this point dopt.dump_inserts has
already turned true if --column-inserts is specified.

But I'm just inclined to think that use case of this feature is vague.
Could you specify more concrete use case that is practical for many users?
(It would lead to more attention to this patch.)
For example, is it useful to backup just before making a big change to DB like delete tables?

===
Takeshi

Dilip Kumar

dilipbalaut@gmail.com

over 7 years ago

In reply to: Surafel Temesgen (#4)

Re: ON CONFLICT DO NOTHING on pg_dump

On Thu, Jun 14, 2018 at 4:09 PM, Surafel Temesgen <surafel3000@gmail.com> wrote:

thank you for pointing me that i add basic test and it seems to me the rest
of the test is covered by column_inserts test

@@ -172,6 +172,7 @@ typedef struct _dumpOptions
char *outputSuperuser;

int sequence_data; /* dump sequence data even in schema-only mode */
+ int do_nothing;
} DumpOptions;

The new structure member appears out of place, can you move up along
with other "command-line long options" ?

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Surafel Temesgen

surafel3000@gmail.com

over 7 years ago

In reply to: Dilip Kumar (#7)

1 attachment(s)

Re: ON CONFLICT DO NOTHING on pg_dump

On Sat, Jun 16, 2018 at 11:36 AM, Dilip Kumar <dilipbalaut@gmail.com> wrote:

@@ -172,6 +172,7 @@ typedef struct _dumpOptions
char *outputSuperuser;

int sequence_data; /* dump sequence data even in schema-only mode */
+ int do_nothing;
} DumpOptions;

The new structure member appears out of place, can you move up along
with other "command-line long options" ?

Done

regards Surafel

Attachments:

pg_dump_onConflect_v3.pachapplication/octet-stream; name=pg_dump_onConflect_v3.pachDownload

diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index f402d46b0c..ff4ebb9121 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -909,6 +909,17 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--on-conflict-do-nothing</option></term>
+      <listitem>
+       <para>
+        Add ON CONFLICT DO NOTHING clause in the INSERT commands.
+        This option is not valid unless <option>--inserts</option> or 
+        <option>--column-inserts</option> is also specified.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>--quote-all-identifiers</option></term>
       <listitem>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 22cb790703..df26927416 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -446,6 +446,17 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--on-conflict-do-nothing</option></term>
+      <listitem>
+       <para>
+        Add ON CONFLICT DO NOTHING clause in the INSERT commands.
+        This option is not valid unless <option>--inserts</option> or 
+        <option>--column-inserts</option> is also specified.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>--quote-all-identifiers</option></term>
       <listitem>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index ceedd481fb..449826a7de 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -160,6 +160,7 @@ typedef struct _dumpOptions
 	int			use_setsessauth;
 	int			enable_row_security;
 	int			load_via_partition_root;
+	int			do_nothing;
 
 	/* default, if no "inclusion" switches appear, is to dump everything */
 	bool		include_everything;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index ea2f022eee..11f2ee17bb 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -378,6 +378,7 @@ main(int argc, char **argv)
 		{"no-unlogged-table-data", no_argument, &dopt.no_unlogged_table_data, 1},
 		{"no-subscriptions", no_argument, &dopt.no_subscriptions, 1},
 		{"no-sync", no_argument, NULL, 7},
+		{"on-conflict-do-nothing", no_argument, &dopt.do_nothing, 1},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -619,6 +620,9 @@ main(int argc, char **argv)
 	if (dopt.if_exists && !dopt.outputClean)
 		exit_horribly(NULL, "option --if-exists requires option -c/--clean\n");
 
+	if (dopt.do_nothing && !(dopt.dump_inserts || dopt.column_inserts))
+		exit_horribly(NULL, "option --on-conflict-do-nothing requires option --inserts or --column-inserts\n");
+
 	/* Identify archive format to emit */
 	archiveFormat = parseArchiveFormat(format, &archiveMode);
 
@@ -989,6 +993,7 @@ help(const char *progname)
 	printf(_("  --no-synchronized-snapshots  do not use synchronized snapshots in parallel jobs\n"));
 	printf(_("  --no-tablespaces             do not dump tablespace assignments\n"));
 	printf(_("  --no-unlogged-table-data     do not dump unlogged table data\n"));
+	printf(_("  --on-conflict-do-nothing     dump data as INSERT commands with ON CONFLICT DO NOTHING \n"));
 	printf(_("  --quote-all-identifiers      quote all identifiers, even if not key words\n"));
 	printf(_("  --section=SECTION            dump named section (pre-data, data, or post-data)\n"));
 	printf(_("  --serializable-deferrable    wait until the dump can run without anomalies\n"));
@@ -2047,7 +2052,11 @@ dumpTableData_insert(Archive *fout, void *dcontext)
 						break;
 				}
 			}
-			archputs(");\n", fout);
+
+			if (!dopt->do_nothing)
+				archputs(");\n", fout);
+			else
+				archputs(") ON CONFLICT DO NOTHING;\n", fout);
 		}
 
 		if (PQntuples(res) <= 0)
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 54db0cd174..1be145a5b1 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -78,6 +78,7 @@ static int	no_unlogged_table_data = 0;
 static int	no_role_passwords = 0;
 static int	server_version;
 static int	load_via_partition_root = 0;
+static int	on_conflict_do_nothing = 0;
 
 static char role_catalog[10];
 #define PG_AUTHID "pg_authid"
@@ -137,6 +138,7 @@ main(int argc, char *argv[])
 		{"no-subscriptions", no_argument, &no_subscriptions, 1},
 		{"no-sync", no_argument, NULL, 4},
 		{"no-unlogged-table-data", no_argument, &no_unlogged_table_data, 1},
+		{"on-conflict-do-nothing", no_argument, &on_conflict_do_nothing, 1},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -367,6 +369,12 @@ main(int argc, char *argv[])
 		exit_nicely(1);
 	}
 
+	if (on_conflict_do_nothing && !(inserts || column_inserts))
+	{
+		fprintf(stderr, _("%s: option --on-conflict-do-nothing requires option --inserts or --column-inserts\n"),
+				progname);
+		exit_nicely(1);
+	}
 	/*
 	 * If password values are not required in the dump, switch to using
 	 * pg_roles which is equally useful, just more likely to have unrestricted
@@ -390,6 +398,8 @@ main(int argc, char *argv[])
 		appendPQExpBufferStr(pgdumpopts, " --inserts");
 	if (no_tablespaces)
 		appendPQExpBufferStr(pgdumpopts, " --no-tablespaces");
+	if (on_conflict_do_nothing)
+		appendPQExpBufferStr(pgdumpopts, " --on-conflict-do-nothing");
 	if (quote_all_identifiers)
 		appendPQExpBufferStr(pgdumpopts, " --quote-all-identifiers");
 	if (load_via_partition_root)
@@ -621,6 +631,7 @@ help(void)
 	printf(_("  --no-sync                    do not wait for changes to be written safely to disk\n"));
 	printf(_("  --no-tablespaces             do not dump tablespace assignments\n"));
 	printf(_("  --no-unlogged-table-data     do not dump unlogged table data\n"));
+	printf(_("  --on-conflict-do-nothing     dump data as INSERT commands with ON CONFLICT DO NOTHING \n"));
 	printf(_("  --quote-all-identifiers      quote all identifiers, even if not key words\n"));
 	printf(_("  --use-set-session-authorization\n"
 			 "                               use SET SESSION AUTHORIZATION commands instead of\n"
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 8be5770ba4..8eaf556df6 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -4,7 +4,7 @@ use warnings;
 use Config;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 68;
+use Test::More tests => 72;
 
 my $tempdir       = TestLib::tempdir;
 my $tempdir_short = TestLib::tempdir_short;
@@ -122,6 +122,11 @@ command_fails_like(
 	qr/\Qpg_restore: unrecognized archive format "garbage";\E/,
 	'pg_dump: unrecognized archive format');
 
+command_fails_like(
+	[ 'pg_dump', '--on-conflict-do-nothing' ],
+	qr/\Qpg_dump: option --on-conflict-do-nothing requires option --inserts or --column-inserts\E/,
+	'pg_dump: option --on-conflict-do-nothing requires option --inserts or --column-inserts');
+
 # pg_dumpall command-line argument checks
 command_fails_like(
 	[ 'pg_dumpall', '-g', '-r' ],
@@ -145,3 +150,9 @@ command_fails_like(
 	[ 'pg_dumpall', '--if-exists' ],
 	qr/\Qpg_dumpall: option --if-exists requires option -c\/--clean\E/,
 	'pg_dumpall: option --if-exists requires option -c/--clean');
+
+command_fails_like(
+	[ 'pg_dumpall', '--on-conflict-do-nothing' ],
+	qr/\Qpg_dumpall: option --on-conflict-do-nothing requires option --inserts or --column-inserts\E/,
+	'pg_dumpall: option --on-conflict-do-nothing requires option --inserts or --column-inserts'
+);

Nico Williams

nico@cryptonector.com

over 7 years ago

In reply to: Ideriha, Takeshi (#5)

Re: ON CONFLICT DO NOTHING on pg_dump

On Fri, Jun 15, 2018 at 02:20:21AM +0000, Ideriha, Takeshi wrote:

From: Nico Williams [mailto:nico@cryptonector.com]
On Tue, Jun 12, 2018 at 09:05:23AM +0000, Ideriha, Takeshi wrote:

Only the difference of data can be restored.

But that's additive-only. Only missing rows are restored this way, and differences are
not addressed.

If you want restore to restore data properly and concurrently (as opposed to renaming
a new database into place or whatever) then you'd want a) MERGE, b) dump to
generate MERGE statements. A concurrent data restore operation would be rather
neat.

I agree with you though supporting MERGE or ON-CONFLICT-DO-UPDATE seems hard work.
Only ON-CONCLICT-DO-NOTHING use case may be narrow.

Is it narrow, or is it just easy enough to add quickly?

And by the way, you don't need MERGE. You can just generate INSERT/
UPDATE/DELETE statements -- MERGE is mainly an optimization on that, and
could wait until PG has a MERGE.

Nico
--

#10

Ideriha, Takeshi

ideriha.takeshi@jp.fujitsu.com

over 7 years ago

In reply to: Nico Williams (#9)

RE: ON CONFLICT DO NOTHING on pg_dump

I agree with you though supporting MERGE or ON-CONFLICT-DO-UPDATE seems

hard work.

Only ON-CONCLICT-DO-NOTHING use case may be narrow.

Is it narrow, or is it just easy enough to add quickly?

Sorry for late replay.
I read your comment and rethought about it.
What I meant by "narrow" is that the number of people who use this new feature seems limited to me.
And I had a wrong impression that small improvements are always rejected by hackers.
But that's only the case for small improvements with huge source code modification. In short, cost/benefit ratio is low case.

This patch have some benefits with source code change is small.
So I just wait for other people reviews.

And by the way, you don't need MERGE. You can just generate INSERT/
UPDATE/DELETE statements -- MERGE is mainly an optimization on that, and could
wait until PG has a MERGE.

Oh, thank you for clarifying this. Now I understand it.

Best regards,
Takeshi Ideriha

#11

Ideriha, Takeshi

ideriha.takeshi@jp.fujitsu.com

over 7 years ago

In reply to: Surafel Temesgen (#8)

RE: ON CONFLICT DO NOTHING on pg_dump

Hi,

The new structure member appears out of place, can you move up along
with other "command-line long options" ?

Done

I did regression tests (make check-world) and
checked manually pg_dump --on-conflict-do-nothing works properly.
Also it seems to me the code has no problem.
This feature has advantage to some users with small code change.

So I marked it as 'Ready for committer'.
I'd like to wait and see committers opinions.

Regards,
Takeshi Ideriha

#12

Thomas Munro

thomas.munro@enterprisedb.com

over 7 years ago

In reply to: Ideriha, Takeshi (#11)

1 attachment(s)

Re: ON CONFLICT DO NOTHING on pg_dump

On Wed, Jul 11, 2018 at 2:20 PM, Ideriha, Takeshi
<ideriha.takeshi@jp.fujitsu.com> wrote:

I did regression tests (make check-world) and
checked manually pg_dump --on-conflict-do-nothing works properly.
Also it seems to me the code has no problem.
This feature has advantage to some users with small code change.

So I marked it as 'Ready for committer'.
I'd like to wait and see committers opinions.

Yeah, it's not an earth-shattering feature, but it might occasionally
be useful and isn't complicated.

+ Add ON CONFLICT DO NOTHING clause in the INSERT commands.

I think this would be better as: Add <literal>ON CONFLICT DO
NOTHING</literal> to <command>INSERT</command> commands.

+ printf(_(" --on-conflict-do-nothing dump data as INSERT
commands with ON CONFLICT DO NOTHING \n"));

That's slightly misleading... let's just use the same wording again,
eg "add ON CONFLICT DO NOTHING to INSERT commands".

{"no-unlogged-table-data", no_argument,
&no_unlogged_table_data, 1},
+ {"on-conflict-do-nothing", no_argument,
&on_conflict_do_nothing, 1},

I was tempted to say that this should be in alphabetical order, but
then I noticed that these are only *almost* in alphabetical order, not
quite. Oh well.

Here's the version I'd like to commit, if there are no objections.

--
Thomas Munro
http://www.enterprisedb.com

Attachments:

0001-Add-pg_dump-on-conflict-do-nothing-option-v4.patchapplication/octet-stream; name=0001-Add-pg_dump-on-conflict-do-nothing-option-v4.patchDownload

From 997672c92c23de2c0575aff9e753c41741a32d79 Mon Sep 17 00:00:00 2001
From: Thomas Munro <tmunro@postgresql.org>
Date: Thu, 12 Jul 2018 22:36:55 +1200
Subject: [PATCH] Add pg_dump --on-conflict-do-nothing option.

When dumping INSERT statements, provide a way to add ON CONFLICT DO NOTHING.

Author: Surafel Temesgen
Reviewed-by: Takeshi Ideriha, Nico Williams, Dilip Kumar
Discussion: https://postgr.es/m/CALAY4q-PQ9cOEzs2%2BQHK5ObfF_4QbmBaYXbZx6BGGN66Q-n8FA%40mail.gmail.com
---
 doc/src/sgml/ref/pg_dump.sgml    | 12 ++++++++++++
 doc/src/sgml/ref/pg_dumpall.sgml | 12 ++++++++++++
 src/bin/pg_dump/pg_backup.h      |  1 +
 src/bin/pg_dump/pg_dump.c        | 11 ++++++++++-
 src/bin/pg_dump/pg_dumpall.c     | 11 +++++++++++
 src/bin/pg_dump/t/001_basic.pl   | 13 ++++++++++++-
 6 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index f402d46b0c..8286b2dddd 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -909,6 +909,18 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--on-conflict-do-nothing</option></term>
+      <listitem>
+       <para>
+        Add <literal>ON CONFLICT DO NOTHING</literal> to
+        <command>INSERT</command> commands.
+        This option is not valid unless <option>--inserts</option> or 
+        <option>--column-inserts</option> is also specified.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>--quote-all-identifiers</option></term>
       <listitem>
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 22cb790703..94d76c30db 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -446,6 +446,18 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--on-conflict-do-nothing</option></term>
+      <listitem>
+       <para>
+        Add <literal>ON CONFLICT DO NOTHING</literal> to
+        <command>INSERT</command> commands.
+        This option is not valid unless <option>--inserts</option> or 
+        <option>--column-inserts</option> is also specified.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>--quote-all-identifiers</option></term>
       <listitem>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index ceedd481fb..42cf441aaf 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -172,6 +172,7 @@ typedef struct _dumpOptions
 	char	   *outputSuperuser;
 
 	int			sequence_data;	/* dump sequence data even in schema-only mode */
+	int			do_nothing;
 } DumpOptions;
 
 /*
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 463639208d..98374c2127 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -378,6 +378,7 @@ main(int argc, char **argv)
 		{"no-unlogged-table-data", no_argument, &dopt.no_unlogged_table_data, 1},
 		{"no-subscriptions", no_argument, &dopt.no_subscriptions, 1},
 		{"no-sync", no_argument, NULL, 7},
+		{"on-conflict-do-nothing", no_argument, &dopt.do_nothing, 1},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -619,6 +620,9 @@ main(int argc, char **argv)
 	if (dopt.if_exists && !dopt.outputClean)
 		exit_horribly(NULL, "option --if-exists requires option -c/--clean\n");
 
+	if (dopt.do_nothing && !(dopt.dump_inserts || dopt.column_inserts))
+		exit_horribly(NULL, "option --on-conflict-do-nothing requires option --inserts or --column-inserts\n");
+
 	/* Identify archive format to emit */
 	archiveFormat = parseArchiveFormat(format, &archiveMode);
 
@@ -989,6 +993,7 @@ help(const char *progname)
 	printf(_("  --no-synchronized-snapshots  do not use synchronized snapshots in parallel jobs\n"));
 	printf(_("  --no-tablespaces             do not dump tablespace assignments\n"));
 	printf(_("  --no-unlogged-table-data     do not dump unlogged table data\n"));
+	printf(_("  --on-conflict-do-nothing     dump data as INSERT commands with ON CONFLICT DO NOTHING \n"));
 	printf(_("  --quote-all-identifiers      quote all identifiers, even if not key words\n"));
 	printf(_("  --section=SECTION            dump named section (pre-data, data, or post-data)\n"));
 	printf(_("  --serializable-deferrable    wait until the dump can run without anomalies\n"));
@@ -2047,7 +2052,11 @@ dumpTableData_insert(Archive *fout, void *dcontext)
 						break;
 				}
 			}
-			archputs(");\n", fout);
+
+			if (!dopt->do_nothing)
+				archputs(");\n", fout);
+			else
+				archputs(") ON CONFLICT DO NOTHING;\n", fout);
 		}
 
 		if (PQntuples(res) <= 0)
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 54db0cd174..d730b477b2 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -78,6 +78,7 @@ static int	no_unlogged_table_data = 0;
 static int	no_role_passwords = 0;
 static int	server_version;
 static int	load_via_partition_root = 0;
+static int	on_conflict_do_nothing = 0;
 
 static char role_catalog[10];
 #define PG_AUTHID "pg_authid"
@@ -137,6 +138,7 @@ main(int argc, char *argv[])
 		{"no-subscriptions", no_argument, &no_subscriptions, 1},
 		{"no-sync", no_argument, NULL, 4},
 		{"no-unlogged-table-data", no_argument, &no_unlogged_table_data, 1},
+		{"on-conflict-do-nothing", no_argument, &on_conflict_do_nothing, 1},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -367,6 +369,12 @@ main(int argc, char *argv[])
 		exit_nicely(1);
 	}
 
+	if (on_conflict_do_nothing && !(inserts || column_inserts))
+	{
+		fprintf(stderr, _("%s: option --on-conflict-do-nothing requires option --inserts or --column-inserts\n"),
+				progname);
+		exit_nicely(1);
+	}
 	/*
 	 * If password values are not required in the dump, switch to using
 	 * pg_roles which is equally useful, just more likely to have unrestricted
@@ -390,6 +398,8 @@ main(int argc, char *argv[])
 		appendPQExpBufferStr(pgdumpopts, " --inserts");
 	if (no_tablespaces)
 		appendPQExpBufferStr(pgdumpopts, " --no-tablespaces");
+	if (on_conflict_do_nothing)
+		appendPQExpBufferStr(pgdumpopts, " --on-conflict-do-nothing");
 	if (quote_all_identifiers)
 		appendPQExpBufferStr(pgdumpopts, " --quote-all-identifiers");
 	if (load_via_partition_root)
@@ -621,6 +631,7 @@ help(void)
 	printf(_("  --no-sync                    do not wait for changes to be written safely to disk\n"));
 	printf(_("  --no-tablespaces             do not dump tablespace assignments\n"));
 	printf(_("  --no-unlogged-table-data     do not dump unlogged table data\n"));
+	printf(_("  --on-conflict-do-nothing     add ON CONFLICT DO NOTHING to INSERT commands\n"));
 	printf(_("  --quote-all-identifiers      quote all identifiers, even if not key words\n"));
 	printf(_("  --use-set-session-authorization\n"
 			 "                               use SET SESSION AUTHORIZATION commands instead of\n"
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 8be5770ba4..8eaf556df6 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -4,7 +4,7 @@ use warnings;
 use Config;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 68;
+use Test::More tests => 72;
 
 my $tempdir       = TestLib::tempdir;
 my $tempdir_short = TestLib::tempdir_short;
@@ -122,6 +122,11 @@ command_fails_like(
 	qr/\Qpg_restore: unrecognized archive format "garbage";\E/,
 	'pg_dump: unrecognized archive format');
 
+command_fails_like(
+	[ 'pg_dump', '--on-conflict-do-nothing' ],
+	qr/\Qpg_dump: option --on-conflict-do-nothing requires option --inserts or --column-inserts\E/,
+	'pg_dump: option --on-conflict-do-nothing requires option --inserts or --column-inserts');
+
 # pg_dumpall command-line argument checks
 command_fails_like(
 	[ 'pg_dumpall', '-g', '-r' ],
@@ -145,3 +150,9 @@ command_fails_like(
 	[ 'pg_dumpall', '--if-exists' ],
 	qr/\Qpg_dumpall: option --if-exists requires option -c\/--clean\E/,
 	'pg_dumpall: option --if-exists requires option -c/--clean');
+
+command_fails_like(
+	[ 'pg_dumpall', '--on-conflict-do-nothing' ],
+	qr/\Qpg_dumpall: option --on-conflict-do-nothing requires option --inserts or --column-inserts\E/,
+	'pg_dumpall: option --on-conflict-do-nothing requires option --inserts or --column-inserts'
+);
-- 
2.17.0

#13

Ideriha, Takeshi

ideriha.takeshi@jp.fujitsu.com

over 7 years ago

In reply to: Thomas Munro (#12)

RE: ON CONFLICT DO NOTHING on pg_dump

Hi, thanks for the revision.

+ Add ON CONFLICT DO NOTHING clause in the INSERT commands.

I think this would be better as: Add <literal>ON CONFLICT DO NOTHING</literal> to
<command>INSERT</command> commands.

Agreed.

+ printf(_(" --on-conflict-do-nothing dump data as INSERT
commands with ON CONFLICT DO NOTHING \n"));

That's slightly misleading... let's just use the same wording again, eg "add ON
CONFLICT DO NOTHING to INSERT commands".

Agreed. But you forgot fixing it at pg_dump.c.
So could you please fix this and commit it?

Regards,
Takeshi Ideriha

#14

Thomas Munro

thomas.munro@enterprisedb.com

over 7 years ago

In reply to: Ideriha, Takeshi (#13)

Re: ON CONFLICT DO NOTHING on pg_dump

On Fri, Jul 13, 2018 at 12:33 PM, Ideriha, Takeshi
<ideriha.takeshi@jp.fujitsu.com> wrote:

+ Add ON CONFLICT DO NOTHING clause in the INSERT commands.

I think this would be better as: Add <literal>ON CONFLICT DO NOTHING</literal> to
<command>INSERT</command> commands.

Agreed.

+ printf(_(" --on-conflict-do-nothing dump data as INSERT
commands with ON CONFLICT DO NOTHING \n"));

That's slightly misleading... let's just use the same wording again, eg "add ON
CONFLICT DO NOTHING to INSERT commands".

Agreed. But you forgot fixing it at pg_dump.c.
So could you please fix this and commit it?

Right, thanks.

I noticed one more thing: pg_dumpall.c doesn't really need to prohibit
--on-conflict-do-nothing without --insert. Its existing validation
rejects illegal combinations of the settings that are *not* passed on
to pg_dump. It seems OK to just pass those on and let pg_dump
complain. For example, if you say "pg_dumpall --data-only
--schema-only", it's pg_dump that complains, not pg_dumpall. I think
we should do the same thing here.

Pushed, with those changes.

Thanks for the patch and the reviews!

--
Thomas Munro
http://www.enterprisedb.com

#15

Ideriha, Takeshi

ideriha.takeshi@jp.fujitsu.com

over 7 years ago

In reply to: Thomas Munro (#14)

RE: ON CONFLICT DO NOTHING on pg_dump

I noticed one more thing: pg_dumpall.c doesn't really need to prohibit
--on-conflict-do-nothing without --insert. Its existing validation rejects illegal
combinations of the settings that are *not* passed on to pg_dump. It seems OK to
just pass those on and let pg_dump complain. For example, if you say "pg_dumpall
--data-only --schema-only", it's pg_dump that complains, not pg_dumpall. I think we
should do the same thing here.

Thank you for the clarification. I didn't give thought to pg_dumpall internally running pg_dump.

Pushed, with those changes.

Thanks!