From 23c52476b4cfd007543943d6d1c67baeb87f75af Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 12 Aug 2020 09:51:40 +0200 Subject: [PATCH v1 2/2] pg_dump: Use pg_get_functiondef() Instead of having pg_dump assemble CREATE FUNCTION commands from the pieces, use the existing pg_get_functiondef(). This should save pg_dump maintenance effort in the future. pg_get_functiondef() was meant for psql's \ef command, so its defaults are slightly different from what pg_dump would like, so add a few optional parameters for tweaking the behavior. --- doc/src/sgml/func.sgml | 17 ++++-- src/backend/catalog/system_views.sql | 4 ++ src/backend/utils/adt/ruleutils.c | 63 ++++++++++++++++----- src/bin/pg_dump/pg_dump.c | 41 +++++++++++++- src/bin/pg_dump/t/002_pg_dump.pl | 42 ++++++++------ src/include/catalog/pg_proc.dat | 2 +- src/test/modules/test_pg_dump/t/001_base.pl | 3 +- 7 files changed, 132 insertions(+), 40 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index f766c1bc67..4f0f060d51 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -22076,16 +22076,25 @@ System Catalog Information Functions pg_get_functiondef - pg_get_functiondef ( func oid ) + pg_get_functiondef ( func oid , or_replace boolean, lavish_quoting boolean ) text Reconstructs the creating command for a function or procedure. (This is a decompiled reconstruction, not the original text of the command.) - The result is a complete CREATE OR REPLACE FUNCTION - or CREATE OR REPLACE PROCEDURE statement. - + The result is a complete CREATE FUNCTION + or CREATE PROCEDURE statement. + + + If or_replace is true (the default), then + CREATE OR REPLACE commands are generated. If + lavish_quoting is true (the default), then the + created command text will use dollar-quoting in a way that is unlikely + to clash with any function body. This is useful when the fetched + command is meant to be edited by the user. Otherwise, it will try to + generate compact quoting adequate for the current function body. + diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 8625cbeab6..823ca7b38a 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1203,6 +1203,10 @@ CREATE FUNCTION ts_debug(IN document text, -- to get filled in.) -- +CREATE OR REPLACE FUNCTION + pg_get_functiondef(func oid, or_replace boolean DEFAULT true, lavish_quoting boolean DEFAULT true) + RETURNS text STRICT STABLE LANGUAGE internal AS 'pg_get_functiondef'; + CREATE OR REPLACE FUNCTION pg_start_backup(label text, fast boolean DEFAULT false, exclusive boolean DEFAULT true) RETURNS pg_lsn STRICT VOLATILE LANGUAGE internal AS 'pg_start_backup' diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 877f99cba3..b1554e351d 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -2590,6 +2590,38 @@ pg_get_serial_sequence(PG_FUNCTION_ARGS) } +static void +appendStringLiteralDQ(StringInfo buf, const char *str, const char *dqprefix) +{ + static const char suffixes[] = "_XXXXXXX"; + int nextchar = 0; + StringInfo delimBuf = makeStringInfo(); + + /* start with $ + dqprefix if not NULL */ + appendStringInfoChar(delimBuf, '$'); + if (dqprefix) + appendStringInfoString(delimBuf, dqprefix); + + /* + * Make sure we choose a delimiter which (without the trailing $) is not + * present in the string being quoted. We don't check with the trailing $ + * because a string ending in $foo must not be quoted with $foo$. + */ + while (strstr(str, delimBuf->data) != NULL) + { + appendStringInfoChar(delimBuf, suffixes[nextchar++]); + nextchar %= sizeof(suffixes) - 1; + } + + /* add trailing $ */ + appendStringInfoChar(delimBuf, '$'); + + /* quote it and we are all done */ + appendStringInfoString(buf, delimBuf->data); + appendStringInfoString(buf, str); + appendStringInfoString(buf, delimBuf->data); +} + /* * pg_get_functiondef * Returns the complete "CREATE OR REPLACE FUNCTION ..." statement for @@ -2604,13 +2636,14 @@ Datum pg_get_functiondef(PG_FUNCTION_ARGS) { Oid funcid = PG_GETARG_OID(0); + bool or_replace = PG_GETARG_BOOL(1); + bool lavish_quoting = PG_GETARG_BOOL(2); StringInfoData buf; - StringInfoData dq; HeapTuple proctup; Form_pg_proc proc; bool isfunction; Datum tmp; - bool isnull; + bool isnull, isnull2; const char *prosrc; const char *name; const char *nsp; @@ -2639,7 +2672,8 @@ pg_get_functiondef(PG_FUNCTION_ARGS) * replaced. */ nsp = get_namespace_name(proc->pronamespace); - appendStringInfo(&buf, "CREATE OR REPLACE %s %s(", + appendStringInfo(&buf, "CREATE %s%s %s(", + or_replace ? "OR REPLACE " : "", isfunction ? "FUNCTION" : "PROCEDURE", quote_qualified_identifier(nsp, name)); (void) print_function_arguments(&buf, proctup, false, true); @@ -2809,8 +2843,8 @@ pg_get_functiondef(PG_FUNCTION_ARGS) appendStringInfoString(&buf, ", "); /* assume prosrc isn't null */ } - tmp = SysCacheGetAttr(PROCOID, proctup, Anum_pg_proc_prosrc, &isnull); - if (isnull) + tmp = SysCacheGetAttr(PROCOID, proctup, Anum_pg_proc_prosrc, &isnull2); + if (isnull2) elog(ERROR, "null prosrc"); prosrc = TextDatumGetCString(tmp); @@ -2821,16 +2855,15 @@ pg_get_functiondef(PG_FUNCTION_ARGS) * shouldn't use a short delimiter that he might easily create a conflict * with. Hence prefer "$function$"/"$procedure$", but extend if needed. */ - initStringInfo(&dq); - appendStringInfoChar(&dq, '$'); - appendStringInfoString(&dq, (isfunction ? "function" : "procedure")); - while (strstr(prosrc, dq.data) != NULL) - appendStringInfoChar(&dq, 'x'); - appendStringInfoChar(&dq, '$'); - - appendBinaryStringInfo(&buf, dq.data, dq.len); - appendStringInfoString(&buf, prosrc); - appendBinaryStringInfo(&buf, dq.data, dq.len); + if (isnull) + appendStringLiteralDQ(&buf, prosrc, (lavish_quoting ? (isfunction ? "function" : "procedure") : NULL)); + else + { + if (strchr(prosrc, '\'') == NULL && strchr(prosrc, '\\') == NULL) + simple_quote_literal(&buf, prosrc); + else + appendStringLiteralDQ(&buf, prosrc, NULL); + } ReleaseSysCache(proctup); diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 9c8436dde6..1f50563383 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -11764,6 +11764,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo) PQExpBuffer q; PQExpBuffer delqry; PQExpBuffer asPart; + bool use_functiondef; PGresult *res; char *funcsig; /* identity signature */ char *funcfullsig = NULL; /* full signature */ @@ -11808,6 +11809,20 @@ dumpFunc(Archive *fout, FuncInfo *finfo) delqry = createPQExpBuffer(); asPart = createPQExpBuffer(); + if (fout->remoteVersion >= 140000) + { + use_functiondef = true; + + appendPQExpBuffer(query, + "SELECT\n" + "pg_get_functiondef(oid, false, false) AS functiondef,\n" + "prokind,\n" + "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs\n"); + } + else + { + use_functiondef = false; + /* Fetch function-specific details */ appendPQExpBuffer(query, "SELECT\n" @@ -11886,6 +11901,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo) else appendPQExpBuffer(query, "'-' AS prosupport\n"); + } appendPQExpBuffer(query, "FROM pg_catalog.pg_proc " @@ -11894,6 +11910,13 @@ dumpFunc(Archive *fout, FuncInfo *finfo) res = ExecuteSqlQueryForSingleRow(fout, query->data); + if (use_functiondef) + { + funciargs = PQgetvalue(res, 0, PQfnumber(res, "funciargs")); + prokind = PQgetvalue(res, 0, PQfnumber(res, "prokind")); + } + else + { proretset = PQgetvalue(res, 0, PQfnumber(res, "proretset")); prosrc = PQgetvalue(res, 0, PQfnumber(res, "prosrc")); probin = PQgetvalue(res, 0, PQfnumber(res, "probin")); @@ -12022,8 +12045,13 @@ dumpFunc(Archive *fout, FuncInfo *finfo) nconfigitems = 0; } } + } - if (funcargs) + if (use_functiondef) + { + funcsig = format_function_arguments(finfo, funciargs, false); + } + else if (funcargs) { /* 8.4 or later; we rely on server-side code for most of the work */ funcfullsig = format_function_arguments(finfo, funcargs, false); @@ -12047,6 +12075,12 @@ dumpFunc(Archive *fout, FuncInfo *finfo) fmtId(finfo->dobj.namespace->dobj.name), funcsig); + if (use_functiondef) + { + appendPQExpBuffer(q, "%s", PQgetvalue(res, 0, PQfnumber(res, "functiondef"))); + } + else + { appendPQExpBuffer(q, "CREATE %s %s.%s", keyword, fmtId(finfo->dobj.namespace->dobj.name), @@ -12198,7 +12232,10 @@ dumpFunc(Archive *fout, FuncInfo *finfo) appendStringLiteralAH(q, pos, fout); } - appendPQExpBuffer(q, "\n %s;\n", asPart->data); + appendPQExpBuffer(q, "\n %s", asPart->data); + } + + appendPQExpBuffer(q, ";\n"); append_depends_on_extension(fout, q, &finfo->dobj, "pg_catalog.pg_proc", keyword, diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index ec63662060..c99408dc0d 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -1495,10 +1495,10 @@ RETURNS LANGUAGE_HANDLER AS \'$libdir/plpgsql\', \'plpgsql_call_handler\' LANGUAGE C;', regexp => qr/^ - \QCREATE FUNCTION dump_test.pltestlang_call_handler() \E - \QRETURNS language_handler\E + \QCREATE FUNCTION dump_test.pltestlang_call_handler()\E + \n\s+\QRETURNS language_handler\E \n\s+\QLANGUAGE c\E - \n\s+AS\ \'\$ + \n\s*AS\ \'\$ \Qlibdir\/plpgsql', 'plpgsql_call_handler';\E /xm, like => @@ -1512,9 +1512,10 @@ RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN RETURN NULL; END;$$;', regexp => qr/^ - \QCREATE FUNCTION dump_test.trigger_func() RETURNS trigger\E + \QCREATE FUNCTION dump_test.trigger_func()\E + \n\s+\QRETURNS trigger\E \n\s+\QLANGUAGE plpgsql\E - \n\s+AS\ \$\$ + \n\s*AS\ \$\$ \Q BEGIN RETURN NULL; END;\E \$\$;/xm, like => @@ -1528,9 +1529,10 @@ RETURNS event_trigger LANGUAGE plpgsql AS $$ BEGIN RETURN; END;$$;', regexp => qr/^ - \QCREATE FUNCTION dump_test.event_trigger_func() RETURNS event_trigger\E + \QCREATE FUNCTION dump_test.event_trigger_func()\E + \n\s+\QRETURNS event_trigger\E \n\s+\QLANGUAGE plpgsql\E - \n\s+AS\ \$\$ + \n\s*AS\ \$\$ \Q BEGIN RETURN; END;\E \$\$;/xm, like => @@ -1838,9 +1840,11 @@ RETURNS dump_test.int42 AS \'int4in\' LANGUAGE internal STRICT IMMUTABLE;', regexp => qr/^ - \QCREATE FUNCTION dump_test.int42_in(cstring) RETURNS dump_test.int42\E - \n\s+\QLANGUAGE internal IMMUTABLE STRICT\E - \n\s+AS\ \$\$int4in\$\$; + \QCREATE FUNCTION dump_test.int42_in(cstring)\E + \n\s+\QRETURNS dump_test.int42\E + \n\s+\QLANGUAGE internal\E + \n\s+\QIMMUTABLE STRICT\E + \n\s*AS\ \$\$int4in\$\$; /xm, like => { %full_runs, %dump_test_schema_runs, section_pre_data => 1, }, @@ -1853,9 +1857,11 @@ RETURNS cstring AS \'int4out\' LANGUAGE internal STRICT IMMUTABLE;', regexp => qr/^ - \QCREATE FUNCTION dump_test.int42_out(dump_test.int42) RETURNS cstring\E - \n\s+\QLANGUAGE internal IMMUTABLE STRICT\E - \n\s+AS\ \$\$int4out\$\$; + \QCREATE FUNCTION dump_test.int42_out(dump_test.int42)\E + \n\s+\QRETURNS cstring\E + \n\s+\QLANGUAGE internal\E + \n\s+\QIMMUTABLE STRICT\E + \n\s*AS\ \$\$int4out\$\$; /xm, like => { %full_runs, %dump_test_schema_runs, section_pre_data => 1, }, @@ -1867,9 +1873,11 @@ create_sql => 'CREATE FUNCTION dump_test.func_with_support() RETURNS int LANGUAGE sql AS $$ SELECT 1 $$ SUPPORT varchar_support;', regexp => qr/^ - \QCREATE FUNCTION dump_test.func_with_support() RETURNS integer\E - \n\s+\QLANGUAGE sql SUPPORT varchar_support\E - \n\s+AS\ \$\$\Q SELECT 1 \E\$\$; + \QCREATE FUNCTION dump_test.func_with_support()\E + \n\s+\QRETURNS integer\E + \n\s+\QLANGUAGE sql\E + \n\s+\QSUPPORT varchar_support\E + \n\s*AS\ \$\$\Q SELECT 1 \E\$\$; /xm, like => { %full_runs, %dump_test_schema_runs, section_pre_data => 1, }, @@ -1883,7 +1891,7 @@ regexp => qr/^ \QCREATE PROCEDURE dump_test.ptest1(a integer)\E \n\s+\QLANGUAGE sql\E - \n\s+AS\ \$\$\Q INSERT INTO dump_test.test_table (col1) VALUES (a) \E\$\$; + \n\s*AS\ \$\$\Q INSERT INTO dump_test.test_table (col1) VALUES (a) \E\$\$; /xm, like => { %full_runs, %dump_test_schema_runs, section_pre_data => 1, }, diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 082a11f270..fe4953114e 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -3660,7 +3660,7 @@ proargtypes => 'text text', prosrc => 'pg_get_serial_sequence' }, { oid => '2098', descr => 'definition of a function', proname => 'pg_get_functiondef', provolatile => 's', prorettype => 'text', - proargtypes => 'oid', prosrc => 'pg_get_functiondef' }, + proargtypes => 'oid bool bool', prosrc => 'pg_get_functiondef' }, { oid => '2162', descr => 'argument list of a function', proname => 'pg_get_function_arguments', provolatile => 's', prorettype => 'text', proargtypes => 'oid', 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 ae120a5ee3..760b7d06d5 100644 --- a/src/test/modules/test_pg_dump/t/001_base.pl +++ b/src/test/modules/test_pg_dump/t/001_base.pl @@ -490,7 +490,8 @@ 'CREATE FUNCTION regress_pg_dump_schema.test_func' => { regexp => qr/^ - \QCREATE FUNCTION regress_pg_dump_schema.test_func() RETURNS integer\E + \QCREATE FUNCTION regress_pg_dump_schema.test_func()\E + \n\s+\QRETURNS integer\E \n\s+\QLANGUAGE sql\E \n/xm, like => { binary_upgrade => 1, }, -- 2.28.0