Hi,
I took a quick look at the patch today. There was some minor bitrot
requiring a rebase, so I attach the rebased patch as v3.
The separate 0002 part contains some minor fixes - a couple
typos/rewording in the docs (would be good if a native speaker looked at
it, thought), and a slightly reworked chunk of code from pg_dump.c. The
change is more a matter of personal preference than correctness - it
just seems simpler this way, but ymmv. And I disliked that the comment
said "If we have to export only the functions .." but the if condition
was actually the exact opposite of that.
The main question I have is whether this should include procedures. I'd
probably argue procedures should be considered different from functions
(i.e. requiring a separate --procedures-only option), because it pretty
much is meant to be a separate object type. We don't allow calling DROP
FUNCTION on a procedure, etc. It'd be silly to introduce an unnecessary
ambiguity in pg_dump and have to deal with it sometime later.
I wonder if we should allow naming a function to dump, similarly to how
--table works for tables, for example.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
>From 1acb9eb47277e6c3cc7d46f1fb5bef0cf669cad6 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <to...@2ndquadrant.com>
Date: Fri, 9 Jul 2021 22:47:23 +0200
Subject: [PATCH 1/2] v3
---
doc/src/sgml/ref/pg_dump.sgml | 39 ++++++++++++++++--
src/bin/pg_dump/pg_backup.h | 1 +
src/bin/pg_dump/pg_dump.c | 45 +++++++++++++++++++--
src/test/modules/test_pg_dump/t/001_base.pl | 9 +++++
4 files changed, 86 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index a6c0788592..fcc3fc663c 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -123,6 +123,11 @@ PostgreSQL documentation
Table data, large objects, and sequence values are dumped.
</para>
+ <para>
+ This option cannot be used together with the <option>--schema-only</option>,
+ or <option>--functions-only</option> option.
+ </para>
+
<para>
This option is similar to, but for historical reasons not identical
to, specifying <option>--section=data</option>.
@@ -136,13 +141,14 @@ PostgreSQL documentation
<listitem>
<para>
Include large objects in the dump. This is the default behavior
- except when <option>--schema</option>, <option>--table</option>, or
- <option>--schema-only</option> is specified. The <option>-b</option>
+ except when <option>--schema</option>, <option>--table</option>,
+ <option>--schema-only</option>, or <option>--functions-only</option>
+ is specified. The <option>-b</option>
switch is therefore only useful to add large objects to dumps
where a specific schema or table has been requested. Note that
blobs are considered data and therefore will be included when
<option>--data-only</option> is used, but not
- when <option>--schema-only</option> is.
+ when <option>--schema-only</option> or <option>--functions-only</option> are.
</para>
</listitem>
</varlistentry>
@@ -582,6 +588,11 @@ PostgreSQL documentation
be dumped.
</para>
+ <para>
+ This option cannot be used together with the <option>--functions-only</option>
+ option.
+ </para>
+
<note>
<para>
When <option>-t</option> is specified, <application>pg_dump</application>
@@ -790,7 +801,22 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
- <term><option>--if-exists</option></term>
+ <term><option>--functions-only</option></term>
+ <listitem>
+ <para>
+ Dmup only the stored functions and stored procedure definitions, not data, not
+ other objects definition.
+ </para>
+
+ <para>
+ This option is the same than <option>--schema-only</option> but restricted
+ to stored functions and stored procedures.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><option>--if-exists</option></term>
<listitem>
<para>
Use conditional commands (i.e., add an <literal>IF EXISTS</literal>
@@ -819,6 +845,11 @@ PostgreSQL documentation
The only exception is that an empty pattern is disallowed.
</para>
+ <para>
+ This option cannot be used together with the <option>--schema-only</option>,
+ or <option>--functions-only</option> option.
+ </para>
+
<note>
<para>
When <option>--include-foreign-data</option> is specified,
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 3c1cd858a8..e332d06317 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -153,6 +153,7 @@ typedef struct _dumpOptions
/* flags for various command-line long options */
int disable_dollar_quoting;
int column_inserts;
+ int functions_only;
int if_exists;
int no_comments;
int no_security_labels;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 321152151d..7837656085 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -380,6 +380,7 @@ main(int argc, char **argv)
{"enable-row-security", no_argument, &dopt.enable_row_security, 1},
{"exclude-table-data", required_argument, NULL, 4},
{"extra-float-digits", required_argument, NULL, 8},
+ {"functions-only", no_argument, &dopt.functions_only, 1},
{"if-exists", no_argument, &dopt.if_exists, 1},
{"inserts", no_argument, NULL, 9},
{"lock-wait-timeout", required_argument, NULL, 2},
@@ -666,9 +667,24 @@ main(int argc, char **argv)
exit_nicely(1);
}
+ if (dopt.dataOnly && dopt.functions_only)
+ {
+ pg_log_error("options --functions-only and -a/--data-only cannot be used together");
+ exit_nicely(1);
+ }
+
+ if (table_include_patterns.head != NULL && dopt.functions_only)
+ {
+ pg_log_error("options --functions-only and -t/--table cannot be used together");
+ exit_nicely(1);
+ }
+
if (dopt.schemaOnly && foreign_servers_include_patterns.head != NULL)
fatal("options -s/--schema-only and --include-foreign-data cannot be used together");
+ if (dopt.functions_only && foreign_servers_include_patterns.head != NULL)
+ fatal("options --functions-only and --include-foreign-data cannot be used together");
+
if (numWorkers > 1 && foreign_servers_include_patterns.head != NULL)
fatal("option --include-foreign-data is not supported with parallel backup");
@@ -862,8 +878,12 @@ main(int argc, char **argv)
*
* -s means "schema only" and blobs are data, not schema, so we never
* include blobs when -s is used.
+ *
+ * --functions-only means no data and blobs are data, so we never include
+ * blobs when --functions-only is used.
+ *
*/
- if (dopt.include_everything && !dopt.schemaOnly && !dopt.dontOutputBlobs)
+ if (dopt.include_everything && !dopt.schemaOnly && !dopt.functions_only && !dopt.dontOutputBlobs)
dopt.outputBlobs = true;
/*
@@ -941,9 +961,26 @@ main(int argc, char **argv)
if (dopt.outputCreateDB)
dumpDatabase(fout);
- /* Now the rearrangeable objects. */
- for (i = 0; i < numObjs; i++)
- dumpDumpableObject(fout, dobjs[i]);
+ /* If we have to export only the functions, we won't need to export
+ * everything
+ */
+ if (!dopt.functions_only)
+ {
+ /* Now the rearrangeable objects. */
+ for (i = 0; i < numObjs; i++)
+ dumpDumpableObject(fout, dobjs[i]);
+ }
+ else
+ {
+ /* We'd like to dump functions only, in that case */
+ for (i = 0; i < numObjs; i++)
+ {
+ if (dobjs[i]->objType == DO_FUNC)
+ {
+ dumpFunc(fout, (const FuncInfo *) dobjs[i]);
+ }
+ }
+ }
/*
* Set up options info to ensure we dump what we want.
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.pl
index 8511da5169..87d10dbdd8 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -152,6 +152,12 @@ my %pgdump_runs = (
"--file=$tempdir/extension_schema.sql", 'postgres',
],
},
+ functions_only => {
+ dump_cmd => [
+ 'pg_dump', '--no-sync', "--file=$tempdir/functions_only.sql",
+ '--functions-only', 'postgres',
+ ],
+ },
pg_dumpall_globals => {
dump_cmd => [
'pg_dumpall', '--no-sync',
@@ -421,6 +427,7 @@ my %tests = (
/xm,
like => {
%full_runs,
+ functions_only => 1,
schema_only => 1,
section_pre_data => 1,
},
@@ -673,6 +680,7 @@ my %tests = (
section_pre_data => 1,
# Excludes this schema as extension is not listed.
without_extension_explicit_schema => 1,
+ functions_only => 1,
},
},
@@ -689,6 +697,7 @@ my %tests = (
section_pre_data => 1,
# Excludes this schema as extension is not listed.
without_extension_explicit_schema => 1,
+ functions_only => 1,
},
},
--
2.31.1
>From d08b2a0d554bf4975818401c8e77a911700fb36e Mon Sep 17 00:00:00 2001
From: Tomas Vondra <to...@2ndquadrant.com>
Date: Fri, 9 Jul 2021 23:37:06 +0200
Subject: [PATCH 2/2] minor review tweaks
---
doc/src/sgml/ref/pg_dump.sgml | 4 ++--
src/bin/pg_dump/pg_dump.c | 25 +++++++------------------
2 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index fcc3fc663c..8ce160027e 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -804,8 +804,8 @@ PostgreSQL documentation
<term><option>--functions-only</option></term>
<listitem>
<para>
- Dmup only the stored functions and stored procedure definitions, not data, not
- other objects definition.
+ Dump only the stored function and stored procedure definitions, not data or
+ definitions of other objects.
</para>
<para>
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 7837656085..eb7a5b05ef 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -880,7 +880,7 @@ main(int argc, char **argv)
* include blobs when -s is used.
*
* --functions-only means no data and blobs are data, so we never include
- * blobs when --functions-only is used.
+ * blobs when --functions-only is used.
*
*/
if (dopt.include_everything && !dopt.schemaOnly && !dopt.functions_only && !dopt.dontOutputBlobs)
@@ -961,25 +961,14 @@ main(int argc, char **argv)
if (dopt.outputCreateDB)
dumpDatabase(fout);
- /* If we have to export only the functions, we won't need to export
- * everything
- */
- if (!dopt.functions_only)
+ /* Now the rearrangeable objects. */
+ for (i = 0; i < numObjs; i++)
{
- /* Now the rearrangeable objects. */
- for (i = 0; i < numObjs; i++)
+ /* Without the --functions-only option, just dump everything. */
+ if (!dopt.functions_only)
dumpDumpableObject(fout, dobjs[i]);
- }
- else
- {
- /* We'd like to dump functions only, in that case */
- for (i = 0; i < numObjs; i++)
- {
- if (dobjs[i]->objType == DO_FUNC)
- {
- dumpFunc(fout, (const FuncInfo *) dobjs[i]);
- }
- }
+ else if (dobjs[i]->objType == DO_FUNC)
+ dumpFunc(fout, (const FuncInfo *) dobjs[i]);
}
/*
--
2.31.1
