From: Stephen Frost <sfrost@snowman.net>
To: pgsql-hackers@postgresql.org
Subject: Re: pg_dump vs. TRANSFORMs
Date: Wed, 7 Dec 2016 15:09:15 -0500
Message-ID: <20161207200915.GZ23417@tamriel.snowman.net> (raw)
In-Reply-To: <20160504183952.GE10850@tamriel.snowman.net>
References: <20160504183952.GE10850@tamriel.snowman.net>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
All,
* Stephen Frost (sfrost@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
Attachments:fix_pg_dump_cast_transform_v1.patch (text/x-diff, 7.1K) download
From 3eaa8d32170b69e58b5780c0aa92b05d10869389 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@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.cindex 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.plindex 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 (application/pgp-signature, 818B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: sfrost@snowman.net
Subject: Re: pg_dump vs. TRANSFORMs
In-Reply-To: <20161207200915.GZ23417@tamriel.snowman.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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