All, * Stephen Frost (sfr...@snowman.net) wrote: > While testing pg_dump, I discovered that there seems to be an issue when > it comes to TRANSFORMs.
[...] As pointed out by Peter E, this also impacts CASTs. Attached is a patch which addresses both by simply also pulling any functions which are referenced from pg_cast or pg_transform when they have OIDs at or after FirstNormalObjectId. I also modified dumpCast() and dumpTransform() to complain loudly if they're unable to dump out the cast or transform due to not finding the function definition(s) necessary. This'll need to be back-patched to 9.5 for the pg_transform look up and all the way for pg_cast, though I don't expect that to be too difficult. We don't do anything else with FirstNormalObjectId in SQL code in pg_dump, though we obviously use it all over the place in the actual code based on the OIDs returned from the database. Still, does anyone see an issue with using it in a query? Without it, we end grabbing the info for 100+ or so functions in a default install that we don't need, which isn't horrible, but there were concerns raised before about pg_dump performance for very small databases. This also adds in regression tests to pg_dump for casts and transforms and the pg_upgrade testing with the regression database will now actually test the dump/restore of transforms (which it didn't previously because the transforms weren't dumped). Thoughts? Thanks! Stephen
From 3eaa8d32170b69e58b5780c0aa92b05d10869389 Mon Sep 17 00:00:00 2001 From: Stephen Frost <sfr...@snowman.net> Date: Wed, 7 Dec 2016 14:59:44 -0500 Subject: [PATCH] Fix dumping of casts and transforms using built-in functions In pg_dump.c dumpCast() and dumpTransform(), we would happily ignore the cast or transform if it happened to use a built-in function because we weren't including the information about built-in functions when querying pg_proc from getFuncs(). Modify the query in getFuncs() to also gather information about functions which are used by user-defined casts and transforms (where "user-defined" means "has an OID >= FirstNormalObjectId"). This also adds to the TAP regression tests for 9.6 and master to cover these types of objects. Back-patch all the way for casts, back to 9.5 for transforms. Discussion: https://www.postgresql.org/message-id/flat/20160504183952.GE10850%40tamriel.snowman.net --- src/bin/pg_dump/pg_dump.c | 33 +++++++++++++--- src/bin/pg_dump/t/002_pg_dump.pl | 85 +++++++++++++++++++++++++--------------- 2 files changed, 80 insertions(+), 38 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 42873bb..530500c 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -4721,12 +4721,21 @@ getFuncs(Archive *fout, int *numFuncs) "\n AND (" "\n pronamespace != " "(SELECT oid FROM pg_namespace " - "WHERE nspname = 'pg_catalog')", + "WHERE nspname = 'pg_catalog')" + "\n OR EXISTS (SELECT 1 FROM pg_cast" + "\n WHERE pg_cast.oid >= %u " + "\n AND p.oid = pg_cast.castfunc)" + "\n OR EXISTS (SELECT 1 FROM pg_transform" + "\n WHERE pg_transform.oid >= %u AND " + "\n (p.oid = pg_transform.trffromsql" + "\n OR p.oid = pg_transform.trftosql))", acl_subquery->data, racl_subquery->data, initacl_subquery->data, initracl_subquery->data, - username_subquery); + username_subquery, + FirstNormalObjectId, + FirstNormalObjectId); if (dopt->binary_upgrade) appendPQExpBufferStr(query, "\n OR EXISTS(SELECT 1 FROM pg_depend WHERE " @@ -4764,7 +4773,16 @@ getFuncs(Archive *fout, int *numFuncs) "\n AND (" "\n pronamespace != " "(SELECT oid FROM pg_namespace " - "WHERE nspname = 'pg_catalog')"); + "WHERE nspname = 'pg_catalog')" + "\n OR EXISTS (SELECT 1 FROM pg_cast" + "\n WHERE p.oid = pg_cast.castfunc)"); + + if (fout->remoteVersion >= 90500) + appendPQExpBufferStr(query, + "\n OR EXISTS (SELECT 1 FROM pg_transform" + "\n WHERE p.oid = pg_transform.trffromsql" + "\n OR p.oid = pg_transform.trftosql)"); + if (dopt->binary_upgrade && fout->remoteVersion >= 90100) appendPQExpBufferStr(query, "\n OR EXISTS(SELECT 1 FROM pg_depend WHERE " @@ -10828,7 +10846,8 @@ dumpCast(Archive *fout, CastInfo *cast) { funcInfo = findFuncByOid(cast->castfunc); if (funcInfo == NULL) - return; + exit_horribly(NULL, "unable to find function definition for OID %u", + cast->castfunc); } /* @@ -10937,13 +10956,15 @@ dumpTransform(Archive *fout, TransformInfo *transform) { fromsqlFuncInfo = findFuncByOid(transform->trffromsql); if (fromsqlFuncInfo == NULL) - return; + exit_horribly(NULL, "unable to find function definition for OID %u", + transform->trffromsql); } if (OidIsValid(transform->trftosql)) { tosqlFuncInfo = findFuncByOid(transform->trftosql); if (tosqlFuncInfo == NULL) - return; + exit_horribly(NULL, "unable to find function definition for OID %u", + transform->trftosql); } /* Make sure we are in proper schema (needed for getFormattedTypeName) */ diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index f895522..59191cc 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -1108,6 +1108,33 @@ my %tests = ( section_post_data => 1, test_schema_plus_blobs => 1, }, }, + 'CREATE CAST FOR timestamptz' => { + create_order => 51, + create_sql => 'CREATE CAST (timestamptz AS interval) WITH FUNCTION age(timestamptz) AS ASSIGNMENT;', + regexp => qr/CREATE CAST \(timestamp with time zone AS interval\) WITH FUNCTION pg_catalog\.age\(timestamp with time zone\) AS ASSIGNMENT;/m, + like => { + binary_upgrade => 1, + clean => 1, + clean_if_exists => 1, + createdb => 1, + defaults => 1, + exclude_dump_test_schema => 1, + exclude_test_table => 1, + exclude_test_table_data => 1, + no_blobs => 1, + no_privs => 1, + no_owner => 1, + pg_dumpall_dbprivs => 1, + schema_only => 1, + section_pre_data => 1, + }, + unlike => { + only_dump_test_schema => 1, + only_dump_test_table => 1, + pg_dumpall_globals => 1, + section_post_data => 1, + test_schema_plus_blobs => 1, }, }, + 'CREATE DATABASE postgres' => { all_runs => 1, regexp => qr/^ @@ -1856,38 +1883,32 @@ my %tests = ( section_post_data => 1, test_schema_plus_blobs => 1, }, }, -####################################### - # Currently broken. -####################################### -# -# 'CREATE TRANSFORM FOR int' => { -# create_order => 34, -# create_sql => 'CREATE TRANSFORM FOR int LANGUAGE SQL (FROM SQL WITH FUNCTION varchar_transform(internal), TO SQL WITH FUNCTION int4recv(internal));', -# regexp => qr/CREATE TRANSFORM FOR int LANGUAGE SQL \(FROM SQL WITH FUNCTION varchar_transform\(internal\), TO SQL WITH FUNCTION int4recv\(internal\)\);/m, -# like => { -# binary_upgrade => 1, -# clean => 1, -# clean_if_exists => 1, -# createdb => 1, -# defaults => 1, -# exclude_dump_test_schema => 1, -# exclude_test_table => 1, -# exclude_test_table_data => 1, -# no_blobs => 1, -# no_privs => 1, -# no_owner => 1, -# pg_dumpall_dbprivs => 1, -# schema_only => 1, -# section_post_data => 1, -# }, -# unlike => { -# section_pre_data => 1, -# only_dump_test_schema => 1, -# only_dump_test_table => 1, -# pg_dumpall_globals => 1, -# test_schema_plus_blobs => 1, -# }, -# }, + 'CREATE TRANSFORM FOR int' => { + create_order => 34, + create_sql => 'CREATE TRANSFORM FOR int LANGUAGE SQL (FROM SQL WITH FUNCTION varchar_transform(internal), TO SQL WITH FUNCTION int4recv(internal));', + regexp => qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog\.varchar_transform\(internal\), TO SQL WITH FUNCTION pg_catalog\.int4recv\(internal\)\);/m, + like => { + binary_upgrade => 1, + clean => 1, + clean_if_exists => 1, + createdb => 1, + defaults => 1, + exclude_dump_test_schema => 1, + exclude_test_table => 1, + exclude_test_table_data => 1, + no_blobs => 1, + no_privs => 1, + no_owner => 1, + pg_dumpall_dbprivs => 1, + schema_only => 1, + section_pre_data => 1, + }, + unlike => { + only_dump_test_schema => 1, + only_dump_test_table => 1, + pg_dumpall_globals => 1, + section_post_data => 1, + test_schema_plus_blobs => 1, }, }, 'CREATE LANGUAGE pltestlang' => { all_runs => 1, -- 2.7.4
signature.asc
Description: Digital signature
