pg_dump vs. TRANSFORMs
Greetings all,
While testing pg_dump, I discovered that there seems to be an issue when
it comes to TRANSFORMs. I'll be the first to admit that I'm not
terribly familiar with transforms, but I do know that if you create one
using functions from pg_catalog (as our regression tests do), the CREATE
TRANSFORM statement isn't dumped out and therefore we don't test the
dump/restore of transforms, even with our current pg_upgrade test
process.
In the regression tests, we do:
CREATE TRANSFORM FOR int LANGUAGE SQL (
FROM SQL WITH FUNCTION varchar_transform(internal),
TO SQL WITH FUNCTION int4recv(internal));
but if you do that and then run pg_dump, you don't see any mention of
any transforms in the dump file.
This is because there's a couple of checks towards the top of
dumpTransform():
/* Cannot dump if we don't have the transform functions' info */
if (OidIsValid(transform->trffromsql))
{
fromsqlFuncInfo = findFuncByOid(transform->trffromsql);
if (fromsqlFuncInfo == NULL)
return;
}
if (OidIsValid(transform->trftosql))
{
tosqlFuncInfo = findFuncByOid(transform->trftosql);
if (tosqlFuncInfo == NULL)
return;
}
These checks are looking for the functions used by the transform in the
list of functions that pg_dump has loaded, but in 9.5, we don't load any
of the function in pg_catalog, and even with my patches, we only dump
the functions in pg_catalog that have an ACL which has been changed from
the default.
Given that there are lots of functions in pg_catalog, we probably don't
want to just load them all, all the time, but what we should probably do
is grab any functions which are depended on by any transforms. Or we
could change dumpTransform() to actually issue a query to get the
function information which it needs when we've decided to dump a given
transform.
For my 2c, this is a bug that needs to be fixed and back-patched to 9.5.
I'm going to continue working on my pg_dump test suite for now, but I
can look at fixing this after PGCon if no one else fixes it first.
Thanks!
Stephen
On 5/4/16 2:39 PM, Stephen Frost wrote:
These checks are looking for the functions used by the transform in the
list of functions that pg_dump has loaded, but in 9.5, we don't load any
of the function in pg_catalog, and even with my patches, we only dump
the functions in pg_catalog that have an ACL which has been changed from
the default.
This issue is not specific to transforms. For example, if you create a
user-defined cast using a built-in function, the cast won't be dumped.
Obviously, this is a problem, but it needs a more general solution.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
On 5/4/16 2:39 PM, Stephen Frost wrote:
These checks are looking for the functions used by the transform in the
list of functions that pg_dump has loaded, but in 9.5, we don't load any
of the function in pg_catalog, and even with my patches, we only dump
the functions in pg_catalog that have an ACL which has been changed from
the default.This issue is not specific to transforms. For example, if you
create a user-defined cast using a built-in function, the cast won't
be dumped. Obviously, this is a problem, but it needs a more general
solution.
Ah, I hadn't gotten to casts yet with my test framework.
I'm certainly open to suggestions on how to address this, but I will
point out that we already have a number of cases where we issue
additional queries against the database from the dump*() functions when
we need to collect additional information. That's not wonderful because
it ends up being slow, but it does work.
Trying to figure out what functions we need at getFuncs() time isn't
terribly easy. On the other hand, it might not be *that* bad to just
pull in all functions, if we use the same technique that we use for
tables, and mark the ones we aren't dumping as not "interesting."
Thanks!
Stephen
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 5/4/16 2:39 PM, Stephen Frost wrote:
These checks are looking for the functions used by the transform in the
list of functions that pg_dump has loaded, but in 9.5, we don't load any
of the function in pg_catalog, and even with my patches, we only dump
the functions in pg_catalog that have an ACL which has been changed from
the default.
This issue is not specific to transforms. For example, if you create a
user-defined cast using a built-in function, the cast won't be dumped.
Obviously, this is a problem, but it needs a more general solution.
Actually, I believe it will be dumped. selectDumpableCast believes it
should dump casts with OID >= FirstNormalObjectId. That's a kluge no
doubt, but reasonably effective; looks like we've been doing that since
9.0.
pg_dump appears not to have a special-case selectDumpableTransform
routine. Instead it falls back on the generic selectDumpableObject
for transforms. That's a bad idea because the only useful bit of
knowledge selectDumpableObject has is to look at the containing
namespace ... and transforms don't have one, just as casts don't.
My recommendation is to clone that cast logic for transforms.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5/4/16 11:23 PM, Tom Lane wrote:
Actually, I believe it will be dumped. selectDumpableCast believes it
should dump casts with OID >= FirstNormalObjectId. That's a kluge no
doubt, but reasonably effective; looks like we've been doing that since
9.0.pg_dump appears not to have a special-case selectDumpableTransform
routine. Instead it falls back on the generic selectDumpableObject
for transforms. That's a bad idea because the only useful bit of
knowledge selectDumpableObject has is to look at the containing
namespace ... and transforms don't have one, just as casts don't.My recommendation is to clone that cast logic for transforms.
Hmm. The way I understand it is that Stephen wants to make dumping that
test case work. But note that that test case is bogus; it wouldn't
actually do anything useful in practice. There aren't any functions in
the system catalog that could be used for actual transforms. So making
these changes in pg_dump isn't really of much practical value. Perhaps
it would be easier to change the test case or adjust the testing procedure?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 5/4/16 2:39 PM, Stephen Frost wrote:
These checks are looking for the functions used by the transform in the
list of functions that pg_dump has loaded, but in 9.5, we don't load any
of the function in pg_catalog, and even with my patches, we only dump
the functions in pg_catalog that have an ACL which has been changed from
the default.This issue is not specific to transforms. For example, if you create a
user-defined cast using a built-in function, the cast won't be dumped.
Obviously, this is a problem, but it needs a more general solution.Actually, I believe it will be dumped. selectDumpableCast believes it
should dump casts with OID >= FirstNormalObjectId. That's a kluge no
doubt, but reasonably effective; looks like we've been doing that since
9.0.
No, it isn't dumped. An interesting example test case is this:
-----
create function pg_catalog.toint(varchar) returns integer
strict immutable language sql as
'select cast($1 as integer);';
create cast (varchar as integer) with function toint(varchar);
-----
Note that neither the function nor the cast is dumped, and this was with
9.5. This is because:
getFuncs() intentionally skips all functions in pg_catalog (unless they
are members of extensions...). dumpCast() attempts to find the function
referenced by the cast in the set of functions which getFuncs()
collected, and simply gives up and doesn't dump the cast if it can't
find the function.
For my 2c, this coding pattern:
--------------
funcInfo = findFuncByOid(cast->castfunc);
if (funcInfo == NULL)
return;
--------------
in pg_dump is really bad. Thankfully, it looks like that's only
happening in dumpCast() and dumpTransform(). We used to have a similar
issue, it looks like, with extensions, which was fixed in 89c29c0. It
looks like we need to either stop excluding the functions in pg_catalog
during getFuncs(), or add in more exceptions to the "don't collect info
about functions in pg_catalog" rule.
I'm also inclined to think that we should fix dumping of non-initdb
functions which have been created in pg_catalog. I'm not sure how far
to take that though, as we have a similar issue for most object types
when it comes to pg_catalog. Perhaps selectDumpableObject() should be
considering FirstNormalObjectId and constraining what component we dump
for from-initdb objects to only ACL, and pg_catalog, as a namespace,
should be set to dump all components (in HEAD, and just set to not dump
anything for from-initdb objects in back-branches, but everything for
user-defined objects).
pg_dump appears not to have a special-case selectDumpableTransform
routine. Instead it falls back on the generic selectDumpableObject
for transforms. That's a bad idea because the only useful bit of
knowledge selectDumpableObject has is to look at the containing
namespace ... and transforms don't have one, just as casts don't.My recommendation is to clone that cast logic for transforms.
We should do this also, if we don't change selectDumpableObject, but
we should do it because we might have from-initdb transforms one day
and the current logic would end up selecting those transforms to dump
out (though dumpTransform would end up skipping them currently because
they'd be referring to functions that we didn't get information about,
but hopefully we're going to fix that).
Thanks!
Stephen
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
On 5/4/16 11:23 PM, Tom Lane wrote:
Actually, I believe it will be dumped. selectDumpableCast believes it
should dump casts with OID >= FirstNormalObjectId. That's a kluge no
doubt, but reasonably effective; looks like we've been doing that since
9.0.pg_dump appears not to have a special-case selectDumpableTransform
routine. Instead it falls back on the generic selectDumpableObject
for transforms. That's a bad idea because the only useful bit of
knowledge selectDumpableObject has is to look at the containing
namespace ... and transforms don't have one, just as casts don't.My recommendation is to clone that cast logic for transforms.
Hmm. The way I understand it is that Stephen wants to make dumping
that test case work. But note that that test case is bogus; it
wouldn't actually do anything useful in practice. There aren't any
functions in the system catalog that could be used for actual
transforms. So making these changes in pg_dump isn't really of much
practical value. Perhaps it would be easier to change the test case
or adjust the testing procedure?
I strongly disagree with the idea that this is only an issue with the
testing system. What if we add functions in the future that are
created by initdb and *are* useful for transforms? What about casts?
There are a lot of functions in pg_catalog that a user might wish to use
to define their own cast. This also doesn't do anything about users
creating functions in pg_catalog.
Thanks!
Stephen
On 5/5/16 8:33 AM, Stephen Frost wrote:
I strongly disagree with the idea that this is only an issue with the
testing system. What if we add functions in the future that are
created by initdb and *are* useful for transforms? What about casts?
There are a lot of functions in pg_catalog that a user might wish to use
to define their own cast. This also doesn't do anything about users
creating functions in pg_catalog.
+1 to all of that. I know that I've at least created casts using
built-in functions during testing, and I think I might be doing it in
some of my extensions.
As for transforms, I suspect we're going to end up with some of those in
initdb in the future, because it's currently the only way you can
improve existing type transformations without breaking existing PL code.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532) mobile: 512-569-9461
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-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.patchtext/x-diff; charset=us-asciiDownload
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.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
Stephen Frost <sfrost@snowman.net> writes:
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.
Please do not hack the already-overcomplicated query in getFuncs without
bothering to adjust the long comment that describes what it's doing.
I have a vague feeling that the code for dumping casts and/or transforms
may have some assumptions that the underlying function is also being
dumped. Although maybe the assumption was really only what's fixed here,
ie that there be a DumpableObject for the function. Anyway, take a close
look for that.
Looks reasonable otherwise.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
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.Please do not hack the already-overcomplicated query in getFuncs without
bothering to adjust the long comment that describes what it's doing.
Right, just wanted to make sure no one had issue with this approach.
I have a vague feeling that the code for dumping casts and/or transforms
may have some assumptions that the underlying function is also being
dumped. Although maybe the assumption was really only what's fixed here,
ie that there be a DumpableObject for the function. Anyway, take a close
look for that.
I'll look around and see, though my hunch is that, at some point, we
were just pulling all functions and then an optimization was added to
exclude pg_catalog and no one noticed that it broke casts using built-in
functions.
Looks reasonable otherwise.
Ok, great, I'll get to work on building and testing all supported
versions of pg_dump vs. all server versions that I can reasonably get to
build on my current laptop, which I expect to be a matrix of 9.2-master
pg_dump against server versions ~7.4-master...
Thanks!
Stephen
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
I have a vague feeling that the code for dumping casts and/or transforms
may have some assumptions that the underlying function is also being
dumped. Although maybe the assumption was really only what's fixed here,
ie that there be a DumpableObject for the function. Anyway, take a close
look for that.
I'll look around and see, though my hunch is that, at some point, we
were just pulling all functions and then an optimization was added to
exclude pg_catalog and no one noticed that it broke casts using built-in
functions.
Nah, that's historical revisionism --- the exclusion for system functions
is very ancient. It certainly predates transforms altogether, and
probably predates the cast-dumping code in anything like its current form.
I think the expectation was that casts using built-in functions were
also built-in and so needn't be dumped. There may be a comment about it
somewhere, which would need to be revised now.
(Actually, the most likely way in which this would break things is if
it started causing built-in casts to get dumped ... have you checked?)
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Tom,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
I have a vague feeling that the code for dumping casts and/or transforms
may have some assumptions that the underlying function is also being
dumped. Although maybe the assumption was really only what's fixed here,
ie that there be a DumpableObject for the function. Anyway, take a close
look for that.I'll look around and see, though my hunch is that, at some point, we
were just pulling all functions and then an optimization was added to
exclude pg_catalog and no one noticed that it broke casts using built-in
functions.Nah, that's historical revisionism --- the exclusion for system functions
is very ancient. It certainly predates transforms altogether, and
probably predates the cast-dumping code in anything like its current form.
Oh, certainly it predates transforms entirely, but I completely expect
that code to have been copied from dumpCast().
I think the expectation was that casts using built-in functions were
also built-in and so needn't be dumped. There may be a comment about it
somewhere, which would need to be revised now.
I'll look and see.
(Actually, the most likely way in which this would break things is if
it started causing built-in casts to get dumped ... have you checked?)
I did and it doesn't cause built-in casts to be dumped because we have
the FirstNormalObjectId check in selectDumpableCast(). We don't have
that for transforms (it just uses the generic selectDumpableObject
routine), but we also don't have any built-in transforms.
Thanks!
Stephen
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
I have a vague feeling that the code for dumping casts and/or transforms
may have some assumptions that the underlying function is also being
dumped. Although maybe the assumption was really only what's fixed here,
ie that there be a DumpableObject for the function. Anyway, take a close
look for that.I'll look around and see, though my hunch is that, at some point, we
were just pulling all functions and then an optimization was added to
exclude pg_catalog and no one noticed that it broke casts using built-in
functions.Nah, that's historical revisionism --- the exclusion for system functions
is very ancient. It certainly predates transforms altogether, and
probably predates the cast-dumping code in anything like its current form.
Apparently, that exclusion goes back to 7.3.
I think the expectation was that casts using built-in functions were
also built-in and so needn't be dumped. There may be a comment about it
somewhere, which would need to be revised now.
I don't see anything in current code or back to 9.2. Going back
farther, I do find comments about skipping casts which only refer to
things in pg_* namespaces, which, of course, isn't correct either.
As such, creating a cast like so:
create cast (timestamptz as interval) with function age(timestamptz) as
assignment;
results in a cast which no version of pg_dump will actually dump out of
any PG server version since CREATE CAST was added. I spent the effort
to get all the way back to 7.1 up and running, tho CREATE CAST wasn't
actually added until 7.3.
(Actually, the most likely way in which this would break things is if
it started causing built-in casts to get dumped ... have you checked?)
So, this is fun. Apparently casts had OIDs > FirstNormalObjectId back
in 8.0 and earlier, so pg_dump >= 9.0 dumps out all casts which don't
have functions for server versions 7.3-8.0. Casts which do have a
function aren't included though, be they user-defined or not, because
they're excluded by getFuncs() and dumpCast() just punts.
With my change, pg_dump'ing against 8.0 and earlier will dump out all
casts, including those with functions, since the function definitions
will now be pulled in for them by getFuncs().
What isn't clear to me is what to do about this. Given the lack of
anyone complaining, and that this would at least ensure that the
user-defined casts are dumped, we could just go with this change and
tell people who are dumping against 8.0 and earlier databases to ignore
the errors from the extra CREATE CAST commands (they shouldn't hurt
anything, after all) during the restore.
Older versions of pg_dump don't have much to offer us regarding this
case- they didn't dump out user-defined casts which only used components
from pg_catalog either.
Ok, thinking a bit more on this and testing, it looks like we record the
initdb-defined casts as 'pinned' in pg_depend all the way back to 7.3.
Therefore, we could use that as the gating factor for getFuncs() instead
of FirstNormalObjectId and then I can also modify getCasts() to exclude
pinned casts, which should fix pg_dump against 8.0 and earlier.
I'll start working on a patch for that, please let me know if you see
any huge glaring holes in my thinking.
Thanks!
Stephen
Stephen Frost <sfrost@snowman.net> writes:
Ok, thinking a bit more on this and testing, it looks like we record the
initdb-defined casts as 'pinned' in pg_depend all the way back to 7.3.
Therefore, we could use that as the gating factor for getFuncs() instead
of FirstNormalObjectId and then I can also modify getCasts() to exclude
pinned casts, which should fix pg_dump against 8.0 and earlier.
Don't really like that; won't it make the getFuncs query substantially
more expensive?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
(Actually, the most likely way in which this would break things is if
it started causing built-in casts to get dumped ... have you checked?)
So, this is fun. Apparently casts had OIDs > FirstNormalObjectId back
in 8.0 and earlier, so pg_dump >= 9.0 dumps out all casts which don't
have functions for server versions 7.3-8.0. Casts which do have a
function aren't included though, be they user-defined or not, because
they're excluded by getFuncs() and dumpCast() just punts.
With my change, pg_dump'ing against 8.0 and earlier will dump out all
casts, including those with functions, since the function definitions
will now be pulled in for them by getFuncs().
I poked into that, and you're right --- it wasn't until 8.1 (commit
2193a856a) that we had a hard-and-fast rule that initdb-assigned OIDs
would be less than FirstNormalObjectId. Before that, the cutoff was
variable and was recorded in pg_database.datlastsysoid.
What isn't clear to me is what to do about this. Given the lack of
anyone complaining, and that this would at least ensure that the
user-defined casts are dumped, we could just go with this change and
tell people who are dumping against 8.0 and earlier databases to ignore
the errors from the extra CREATE CAST commands (they shouldn't hurt
anything, after all) during the restore.
There's a lot to be said for that. It dumped too much before, it'll
dump a bit more now, but neither case is fatal. And it's unlikely
that anybody really cares anymore.
If you do want to do something about this, the way would be to retrieve
datlastsysoid and use that as the cutoff with a pre-8.1 server. I think
there used to be code to do things that way in pg_dump; we must have
removed it (rather prematurely).
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Tom,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
Ok, thinking a bit more on this and testing, it looks like we record the
initdb-defined casts as 'pinned' in pg_depend all the way back to 7.3.
Therefore, we could use that as the gating factor for getFuncs() instead
of FirstNormalObjectId and then I can also modify getCasts() to exclude
pinned casts, which should fix pg_dump against 8.0 and earlier.Don't really like that; won't it make the getFuncs query substantially
more expensive?
Adding the check against pg_depend doesn't appear to make it much slower
than having the check against FirstNormalObjectId. Adding either seems
to cause the getFuncs() query to go from ~1.7ms to ~3.4ms for HEAD, at
least on my system, which was also built with debugging and asserts and
all that, so the difference in the field is probably less.
Going back to older versions, the difference drops because we drop the
pg_init_privs logic for pre-9.6, and then drop the check for transforms
in pre-9.5.
All versions are just a few ms from HEAD down to 8.4.
In 8.3 and older, we do start to get slower because we don't use a
Merge Anti Join and instead use a Nested Loop Left Join. I was getting
about:
8.3 - ~35ms
8.2 - ~44ms
8.1 - ~66ms
8.0 - ~82ms
7.4 - ~64ms
7.3 - ~61ms
Again, these were Assert and debug-enabled builds, though, of course,
that's not going to make up for the lack of anti-join.
I'm really not sure that's worth troubling over though as I am not aware
of anyone with hundreds of 8.3 or earlier databases that they're going
to need a really fast pg_dump to work on.
Thanks!
Stephen
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
(Actually, the most likely way in which this would break things is if
it started causing built-in casts to get dumped ... have you checked?)So, this is fun. Apparently casts had OIDs > FirstNormalObjectId back
in 8.0 and earlier, so pg_dump >= 9.0 dumps out all casts which don't
have functions for server versions 7.3-8.0. Casts which do have a
function aren't included though, be they user-defined or not, because
they're excluded by getFuncs() and dumpCast() just punts.With my change, pg_dump'ing against 8.0 and earlier will dump out all
casts, including those with functions, since the function definitions
will now be pulled in for them by getFuncs().I poked into that, and you're right --- it wasn't until 8.1 (commit
2193a856a) that we had a hard-and-fast rule that initdb-assigned OIDs
would be less than FirstNormalObjectId. Before that, the cutoff was
variable and was recorded in pg_database.datlastsysoid.
Oh, I see.
What isn't clear to me is what to do about this. Given the lack of
anyone complaining, and that this would at least ensure that the
user-defined casts are dumped, we could just go with this change and
tell people who are dumping against 8.0 and earlier databases to ignore
the errors from the extra CREATE CAST commands (they shouldn't hurt
anything, after all) during the restore.There's a lot to be said for that. It dumped too much before, it'll
dump a bit more now, but neither case is fatal. And it's unlikely
that anybody really cares anymore.
Well, yes, but still, if it's not too hard to do...
If you do want to do something about this, the way would be to retrieve
datlastsysoid and use that as the cutoff with a pre-8.1 server. I think
there used to be code to do things that way in pg_dump; we must have
removed it (rather prematurely).
That's a good point, we might be doing things wrong in other places in
the code by using FirstNormalObjectId on pre-8.1 servers.
What I suggest then is an independent patch which uses a different
variable than FirstNormalObjectId and sets it correctly based on the
version of database that we're connecting to, and after that's working
correctly for HEAD vs. HEAD-8.0, and 9.6-9.2 vs. 9.6-7.1 (all I was able
to get running within a few hours.. if someone wants to test against
7.0 or earlier that's fine, or if someone can provide a clean patch to
make 7.0 work for me, that's fine too) and after that looks good and is
committed, I'll rebase this work on that.
That said, at least initial testing makes it look like it's still going
to be in the 10s-of-ms on 8.3 and earlier. Looking at it a bit more, it
looks like part of the problem there is that, for some reason, we're
using a sequential scan inside a nested loop instead of using the
pg_cast_oid_index.. Setting enable_seqscan = false turns that into a
Bitmap Heap Scan which gets it down to only a few ms again. ANALYZE
doesn't seem to help either, though I'm still not terribly concerned
about this.
Thanks!
Stephen
Stephen Frost <sfrost@snowman.net> writes:
That's a good point, we might be doing things wrong in other places in
the code by using FirstNormalObjectId on pre-8.1 servers.
What I suggest then is an independent patch which uses a different
variable than FirstNormalObjectId and sets it correctly based on the
version of database that we're connecting to,
+1
and after that's working
correctly for HEAD vs. HEAD-8.0, and 9.6-9.2 vs. 9.6-7.1 (all I was able
to get running within a few hours.. if someone wants to test against
7.0 or earlier that's fine, or if someone can provide a clean patch to
make 7.0 work for me, that's fine too) and after that looks good and is
committed, I'll rebase this work on that.
pg_dump never intended to support pre-7.0 servers. I do have 7.0-7.3
servers in captivity and can do testing if you like.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Tom,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
That's a good point, we might be doing things wrong in other places in
the code by using FirstNormalObjectId on pre-8.1 servers.What I suggest then is an independent patch which uses a different
variable than FirstNormalObjectId and sets it correctly based on the
version of database that we're connecting to,+1
Alright, here we go- patches for every currently supported branch, each
tested from that version back to 7.1, back to 7.3 with a user-defined
CAST, and back to 9.5 with a user-defined TRANSFORM. Also includes
regression tests for the TAP test structure in master and 9.6.
pg_dump never intended to support pre-7.0 servers. I do have 7.0-7.3
servers in captivity and can do testing if you like.
You are certainly welcome to test and make sure I didn't break anything
for 7.0 servers, but I don't *think* I changed any code paths which have
differences between 7.0 and 7.1 (which I did test against). That said,
I'm honestly not entirely sure what getCasts() is doing querying out the
"casts" from a 7.1 or 7.0 database. The query does work, but none of
the "casts" returned have an OID beyond datlastsysoid and I'm not really
sure how to create one or if creating one is really a supported
operation. If someone was able to create something that getCasts()
would try to dump out, and it used only built-in functions, they will
at least get an error now letting them know that it failed, instead of
having that "cast" silently ignored.
Trying to adjust the query in getFuncs() to account for that case makes
me concerned that we'd actually break more than fix things, and I really
don't like the idea of trying to blindly fix it for 7.0.
Thanks!
Stephen
Attachments:
fix_pg_dump_cast_transform_v2_96.patchtext/x-diff; charset=us-asciiDownload
From 3ee8993c8073d2af288f5f0385c27a3de79c28b5 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Fri, 9 Dec 2016 15:58:44 -0500
Subject: [PATCH 1/2] For 8.0 servers, get last built-in oid from pg_database
We didn't start ensuring that all built-in objects had OIDs less than
16384 until 8.1, so for 8.0 servers we still need to query the value out
of pg_database. We need this, in particular, to distinguish which casts
were built-in and which were user-defined.
For HEAD, we only worry about going back to 8.0, for the back-branches,
we also ensure that 7.0-7.4 work.
Discussion: https://www.postgresql.org/message-id/flat/20160504183952.GE10850%40tamriel.snowman.net
---
src/bin/pg_dump/pg_dump.c | 38 ++++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index fde7f59..bd2d977 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -96,7 +96,10 @@ bool g_verbose; /* User wants verbose narration of our
/* subquery used to convert user ID (eg, datdba) to user name */
static const char *username_subquery;
-/* obsolete as of 7.3: */
+/*
+ * For 8.0 and earlier servers, pulled from pg_database, for 8.1+ we use
+ * FirstNormalObjectId - 1.
+ */
static Oid g_last_builtin_oid; /* value of the last builtin oid */
/* The specified names/patterns should to match at least one entity */
@@ -683,17 +686,24 @@ main(int argc, char **argv)
exit_horribly(NULL,
"Exported snapshots are not supported by this server version.\n");
- /* Find the last built-in OID, if needed */
- if (fout->remoteVersion < 70300)
+ /*
+ * Find the last built-in OID, if needed (prior to 8.1)
+ *
+ * With 8.1 and above, we can just use FirstNormalObjectId - 1.
+ */
+ if (fout->remoteVersion < 80100)
{
if (fout->remoteVersion >= 70100)
g_last_builtin_oid = findLastBuiltinOid_V71(fout,
PQdb(GetConnection(fout)));
else
g_last_builtin_oid = findLastBuiltinOid_V70(fout);
- if (g_verbose)
- write_msg(NULL, "last built-in OID is %u\n", g_last_builtin_oid);
}
+ else
+ g_last_builtin_oid = FirstNormalObjectId - 1;
+
+ if (g_verbose)
+ write_msg(NULL, "last built-in OID is %u\n", g_last_builtin_oid);
/* Expand schema selection patterns into OID lists */
if (schema_include_patterns.head != NULL)
@@ -1507,7 +1517,7 @@ selectDumpableCast(CastInfo *cast, Archive *fout)
* This would be DUMP_COMPONENT_ACL for from-initdb casts, but they do not
* support ACLs currently.
*/
- if (cast->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (cast->dobj.catId.oid <= (Oid) g_last_builtin_oid)
cast->dobj.dump = DUMP_COMPONENT_NONE;
else
cast->dobj.dump = fout->dopt->include_everything ?
@@ -1539,7 +1549,7 @@ selectDumpableProcLang(ProcLangInfo *plang, Archive *fout)
plang->dobj.dump = DUMP_COMPONENT_NONE;
else
{
- if (plang->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (plang->dobj.catId.oid <= (Oid) g_last_builtin_oid)
plang->dobj.dump = fout->remoteVersion < 90600 ?
DUMP_COMPONENT_NONE : DUMP_COMPONENT_ACL;
else
@@ -1565,7 +1575,7 @@ selectDumpableAccessMethod(AccessMethodInfo *method, Archive *fout)
* This would be DUMP_COMPONENT_ACL for from-initdb access methods, but
* they do not support ACLs currently.
*/
- if (method->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (method->dobj.catId.oid <= (Oid) g_last_builtin_oid)
method->dobj.dump = DUMP_COMPONENT_NONE;
else
method->dobj.dump = fout->dopt->include_everything ?
@@ -1590,7 +1600,7 @@ selectDumpableExtension(ExtensionInfo *extinfo, DumpOptions *dopt)
* change permissions on those objects, if they wish to, and have those
* changes preserved.
*/
- if (dopt->binary_upgrade && extinfo->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (dopt->binary_upgrade && extinfo->dobj.catId.oid <= (Oid) g_last_builtin_oid)
extinfo->dobj.dump = extinfo->dobj.dump_contains = DUMP_COMPONENT_ACL;
else
extinfo->dobj.dump = extinfo->dobj.dump_contains =
@@ -9571,8 +9581,8 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
/*
* We unconditionally create the extension, so we must drop it if it
* exists. This could happen if the user deleted 'plpgsql' and then
- * readded it, causing its oid to be greater than FirstNormalObjectId.
- * The FirstNormalObjectId test was kept to avoid repeatedly dropping
+ * readded it, causing its oid to be greater than g_last_builtin_oid.
+ * The g_last_builtin_oid test was kept to avoid repeatedly dropping
* and recreating extensions like 'plpgsql'.
*/
appendPQExpBuffer(q, "DROP EXTENSION IF EXISTS %s;\n", qextname);
@@ -16284,10 +16294,10 @@ dumpTableConstraintComment(Archive *fout, ConstraintInfo *coninfo)
}
/*
- * findLastBuiltInOid -
+ * findLastBuiltinOid -
* find the last built in oid
*
- * For 7.1 and 7.2, we do this by retrieving datlastsysoid from the
+ * For 7.1 through 8.0, we do this by retrieving datlastsysoid from the
* pg_database entry for the current database
*/
static Oid
@@ -16309,7 +16319,7 @@ findLastBuiltinOid_V71(Archive *fout, const char *dbname)
}
/*
- * findLastBuiltInOid -
+ * findLastBuiltinOid -
* find the last built in oid
*
* For 7.0, we do this by assuming that the last thing that initdb does is to
--
2.7.4
From 1f642813d3b26f8c8be677288ce063c470194f3f Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Fri, 9 Dec 2016 16:05:39 -0500
Subject: [PATCH 2/2] 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 | 46 +++++++++++++++++-----
src/bin/pg_dump/t/002_pg_dump.pl | 82 +++++++++++++++++++++++++---------------
2 files changed, 88 insertions(+), 40 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index bd2d977..6b2a6c9 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4978,8 +4978,11 @@ getFuncs(Archive *fout, int *numFuncs)
* 3. Otherwise, we normally exclude functions in pg_catalog. However, if
* they're members of extensions and we are in binary-upgrade mode then
* include them, since we want to dump extension members individually in
- * that mode. Also, in 9.6 and up, include functions in pg_catalog if
- * they have an ACL different from what's shown in pg_init_privs.
+ * that mode. Also, if they are used by casts or transforms then we need
+ * to gather the information about them, though they won't be dumped if
+ * they are built-in. Also, in 9.6 and up, include functions in
+ * pg_catalog if they have an ACL different from what's shown in
+ * pg_init_privs.
*/
if (fout->remoteVersion >= 90600)
{
@@ -5013,12 +5016,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,
+ g_last_builtin_oid,
+ g_last_builtin_oid);
if (dopt->binary_upgrade)
appendPQExpBufferStr(query,
"\n OR EXISTS(SELECT 1 FROM pg_depend WHERE "
@@ -5052,11 +5064,24 @@ getFuncs(Archive *fout, int *numFuncs)
"\n AND NOT EXISTS (SELECT 1 FROM pg_depend "
"WHERE classid = 'pg_proc'::regclass AND "
"objid = p.oid AND deptype = 'i')");
- appendPQExpBufferStr(query,
+ appendPQExpBuffer(query,
"\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'::oid"
+ "\n AND p.oid = pg_cast.castfunc)",
+ g_last_builtin_oid);
+
+ if (fout->remoteVersion >= 90500)
+ appendPQExpBuffer(query,
+ "\n OR EXISTS (SELECT 1 FROM pg_transform"
+ "\n WHERE pg_transform.oid > '%u'::oid"
+ "\n AND (p.oid = pg_transform.trffromsql"
+ "\n OR p.oid = pg_transform.trftosql))",
+ g_last_builtin_oid);
+
if (dopt->binary_upgrade && fout->remoteVersion >= 90100)
appendPQExpBufferStr(query,
"\n OR EXISTS(SELECT 1 FROM pg_depend WHERE "
@@ -11871,7 +11896,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);
}
/*
@@ -11980,13 +12006,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 37cbdcd..bb2ea34 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -913,6 +913,32 @@ my %tests = (
section_pre_data => 1,
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' => {
regexp => qr/^
\QCREATE DATABASE postgres WITH TEMPLATE = template0 \E
@@ -1515,37 +1541,31 @@ my %tests = (
pg_dumpall_globals_clean => 1,
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_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_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' => {
create_order => 18,
create_sql => 'CREATE LANGUAGE pltestlang
--
2.7.4
fix_pg_dump_cast_transform_v2_95.patchtext/x-diff; charset=us-asciiDownload
From 06a0989d754b8c6629ff54b7667f2c917fc84665 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Fri, 9 Dec 2016 16:48:53 -0500
Subject: [PATCH 1/2] For 8.0 servers, get last built-in oid from pg_database
We didn't start ensuring that all built-in objects had OIDs less than
16384 until 8.1, so for 8.0 servers we still need to query the value out
of pg_database. We need this, in particular, to distinguish which casts
were built-in and which were user-defined.
For HEAD, we only worry about going back to 8.0, for the back-branches,
we also ensure that 7.0-7.4 work.
Discussion: https://www.postgresql.org/message-id/flat/20160504183952.GE10850%40tamriel.snowman.net
---
src/bin/pg_dump/pg_dump.c | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 0353140..5b9f575 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -94,7 +94,10 @@ bool g_verbose; /* User wants verbose narration of our
/* subquery used to convert user ID (eg, datdba) to user name */
static const char *username_subquery;
-/* obsolete as of 7.3: */
+/*
+ * For 8.0 and earlier servers, pulled from pg_database, for 8.1+ we use
+ * FirstNormalObjectId - 1.
+ */
static Oid g_last_builtin_oid; /* value of the last builtin oid */
/*
@@ -673,17 +676,24 @@ main(int argc, char **argv)
exit_horribly(NULL,
"Exported snapshots are not supported by this server version.\n");
- /* Find the last built-in OID, if needed */
- if (fout->remoteVersion < 70300)
+ /*
+ * Find the last built-in OID, if needed (prior to 8.1)
+ *
+ * With 8.1 and above, we can just use FirstNormalObjectId - 1.
+ */
+ if (fout->remoteVersion < 80100)
{
if (fout->remoteVersion >= 70100)
g_last_builtin_oid = findLastBuiltinOid_V71(fout,
PQdb(GetConnection(fout)));
else
g_last_builtin_oid = findLastBuiltinOid_V70(fout);
- if (g_verbose)
- write_msg(NULL, "last built-in OID is %u\n", g_last_builtin_oid);
}
+ else
+ g_last_builtin_oid = FirstNormalObjectId - 1;
+
+ if (g_verbose)
+ write_msg(NULL, "last built-in OID is %u\n", g_last_builtin_oid);
/* Expand schema selection patterns into OID lists */
if (schema_include_patterns.head != NULL)
@@ -1440,7 +1450,7 @@ selectDumpableCast(CastInfo *cast, DumpOptions *dopt)
if (checkExtensionMembership(&cast->dobj, dopt))
return; /* extension membership overrides all else */
- if (cast->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (cast->dobj.catId.oid <= (Oid) g_last_builtin_oid)
cast->dobj.dump = false;
else
cast->dobj.dump = dopt->include_everything;
@@ -1460,7 +1470,7 @@ selectDumpableProcLang(ProcLangInfo *plang, DumpOptions *dopt)
if (checkExtensionMembership(&plang->dobj, dopt))
return; /* extension membership overrides all else */
- if (plang->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (plang->dobj.catId.oid <= (Oid) g_last_builtin_oid)
plang->dobj.dump = false;
else
plang->dobj.dump = dopt->include_everything;
@@ -1479,7 +1489,7 @@ selectDumpableProcLang(ProcLangInfo *plang, DumpOptions *dopt)
static void
selectDumpableExtension(ExtensionInfo *extinfo, DumpOptions *dopt)
{
- if (dopt->binary_upgrade && extinfo->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (dopt->binary_upgrade && extinfo->dobj.catId.oid <= (Oid) g_last_builtin_oid)
extinfo->dobj.dump = false;
else
extinfo->dobj.dump = dopt->include_everything;
@@ -8630,8 +8640,8 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
/*
* We unconditionally create the extension, so we must drop it if it
* exists. This could happen if the user deleted 'plpgsql' and then
- * readded it, causing its oid to be greater than FirstNormalObjectId.
- * The FirstNormalObjectId test was kept to avoid repeatedly dropping
+ * readded it, causing its oid to be greater than g_last_builtin_oid.
+ * The g_last_builtin_oid test was kept to avoid repeatedly dropping
* and recreating extensions like 'plpgsql'.
*/
appendPQExpBuffer(q, "DROP EXTENSION IF EXISTS %s;\n", qextname);
@@ -14946,10 +14956,10 @@ dumpTableConstraintComment(Archive *fout, ConstraintInfo *coninfo)
}
/*
- * findLastBuiltInOid -
+ * findLastBuiltinOid -
* find the last built in oid
*
- * For 7.1 and 7.2, we do this by retrieving datlastsysoid from the
+ * For 7.1 through 8.0, we do this by retrieving datlastsysoid from the
* pg_database entry for the current database
*/
static Oid
@@ -14971,7 +14981,7 @@ findLastBuiltinOid_V71(Archive *fout, const char *dbname)
}
/*
- * findLastBuiltInOid -
+ * findLastBuiltinOid -
* find the last built in oid
*
* For 7.0, we do this by assuming that the last thing that initdb does is to
--
2.7.4
From ca857ea49aa16767c71fb61d8c44210cfde2d3d6 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Mon, 12 Dec 2016 10:39:19 -0500
Subject: [PATCH 2/2] 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 | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 5b9f575..cd67082 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4506,7 +4506,9 @@ getFuncs(Archive *fout, int *numFuncs)
* 3. Otherwise, we normally exclude functions in pg_catalog. However, if
* they're members of extensions and we are in binary-upgrade mode then
* include them, since we want to dump extension members individually in
- * that mode.
+ * that mode. Also, if they are used by casts or transforms then we need
+ * to gather the information about them, though they won't be dumped if
+ * they are built-in.
*/
if (fout->remoteVersion >= 70300)
@@ -4524,11 +4526,24 @@ getFuncs(Archive *fout, int *numFuncs)
"\n AND NOT EXISTS (SELECT 1 FROM pg_depend "
"WHERE classid = 'pg_proc'::regclass AND "
"objid = p.oid AND deptype = 'i')");
- appendPQExpBufferStr(query,
+ appendPQExpBuffer(query,
"\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'::oid"
+ "\n AND p.oid = pg_cast.castfunc)",
+ g_last_builtin_oid);
+
+ if (fout->remoteVersion >= 90500)
+ appendPQExpBuffer(query,
+ "\n OR EXISTS (SELECT 1 FROM pg_transform"
+ "\n WHERE pg_transform.oid > %u::oid"
+ "\n AND (p.oid = pg_transform.trffromsql"
+ "\n OR p.oid = pg_transform.trftosql))",
+ g_last_builtin_oid);
+
if (dopt->binary_upgrade && fout->remoteVersion >= 90100)
appendPQExpBufferStr(query,
"\n OR EXISTS(SELECT 1 FROM pg_depend WHERE "
@@ -10835,7 +10850,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);
}
/*
@@ -10939,13 +10955,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) */
--
2.7.4
fix_pg_dump_cast_transform_v2_94.patchtext/x-diff; charset=us-asciiDownload
From c16a3e32b8abed30c9b75562c0cb4a27fb0d85a8 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Mon, 12 Dec 2016 11:15:05 -0500
Subject: [PATCH 1/2] For 8.0 servers, get last built-in oid from pg_database
We didn't start ensuring that all built-in objects had OIDs less than
16384 until 8.1, so for 8.0 servers we still need to query the value out
of pg_database. We need this, in particular, to distinguish which casts
were built-in and which were user-defined.
For HEAD, we only worry about going back to 8.0, for the back-branches,
we also ensure that 7.0-7.4 work.
Discussion: https://www.postgresql.org/message-id/flat/20160504183952.GE10850%40tamriel.snowman.net
---
src/bin/pg_dump/pg_dump.c | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 3592796..3e7b689 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -95,7 +95,10 @@ static const char *lockWaitTimeout;
/* subquery used to convert user ID (eg, datdba) to user name */
static const char *username_subquery;
-/* obsolete as of 7.3: */
+/*
+ * For 8.0 and earlier servers, pulled from pg_database, for 8.1+ we use
+ * FirstNormalObjectId - 1.
+ */
static Oid g_last_builtin_oid; /* value of the last builtin oid */
/*
@@ -705,17 +708,24 @@ main(int argc, char **argv)
"Run with --no-synchronized-snapshots instead if you do not need\n"
"synchronized snapshots.\n");
- /* Find the last built-in OID, if needed */
- if (fout->remoteVersion < 70300)
+ /*
+ * Find the last built-in OID, if needed (prior to 8.1)
+ *
+ * With 8.1 and above, we can just use FirstNormalObjectId - 1.
+ */
+ if (fout->remoteVersion < 80100)
{
if (fout->remoteVersion >= 70100)
g_last_builtin_oid = findLastBuiltinOid_V71(fout,
PQdb(GetConnection(fout)));
else
g_last_builtin_oid = findLastBuiltinOid_V70(fout);
- if (g_verbose)
- write_msg(NULL, "last built-in OID is %u\n", g_last_builtin_oid);
}
+ else
+ g_last_builtin_oid = FirstNormalObjectId - 1;
+
+ if (g_verbose)
+ write_msg(NULL, "last built-in OID is %u\n", g_last_builtin_oid);
/* Expand schema selection patterns into OID lists */
if (schema_include_patterns.head != NULL)
@@ -1429,7 +1439,7 @@ selectDumpableCast(CastInfo *cast)
if (checkExtensionMembership(&cast->dobj))
return; /* extension membership overrides all else */
- if (cast->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (cast->dobj.catId.oid <= (Oid) g_last_builtin_oid)
cast->dobj.dump = false;
else
cast->dobj.dump = include_everything;
@@ -1449,7 +1459,7 @@ selectDumpableProcLang(ProcLangInfo *plang)
if (checkExtensionMembership(&plang->dobj))
return; /* extension membership overrides all else */
- if (plang->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (plang->dobj.catId.oid <= (Oid) g_last_builtin_oid)
plang->dobj.dump = false;
else
plang->dobj.dump = include_everything;
@@ -1468,7 +1478,7 @@ selectDumpableProcLang(ProcLangInfo *plang)
static void
selectDumpableExtension(ExtensionInfo *extinfo)
{
- if (binary_upgrade && extinfo->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (binary_upgrade && extinfo->dobj.catId.oid <= (Oid) g_last_builtin_oid)
extinfo->dobj.dump = false;
else
extinfo->dobj.dump = include_everything;
@@ -8159,8 +8169,8 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
/*
* We unconditionally create the extension, so we must drop it if it
* exists. This could happen if the user deleted 'plpgsql' and then
- * readded it, causing its oid to be greater than FirstNormalObjectId.
- * The FirstNormalObjectId test was kept to avoid repeatedly dropping
+ * readded it, causing its oid to be greater than g_last_builtin_oid.
+ * The g_last_builtin_oid test was kept to avoid repeatedly dropping
* and recreating extensions like 'plpgsql'.
*/
appendPQExpBuffer(q, "DROP EXTENSION IF EXISTS %s;\n", qextname);
@@ -14257,10 +14267,10 @@ dumpTableConstraintComment(Archive *fout, ConstraintInfo *coninfo)
}
/*
- * findLastBuiltInOid -
+ * findLastBuiltinOid -
* find the last built in oid
*
- * For 7.1 and 7.2, we do this by retrieving datlastsysoid from the
+ * For 7.1 through 8.0, we do this by retrieving datlastsysoid from the
* pg_database entry for the current database
*/
static Oid
@@ -14282,7 +14292,7 @@ findLastBuiltinOid_V71(Archive *fout, const char *dbname)
}
/*
- * findLastBuiltInOid -
+ * findLastBuiltinOid -
* find the last built in oid
*
* For 7.0, we do this by assuming that the last thing that initdb does is to
--
2.7.4
From 4c05cedac564463d3f870337802bfccd6b5ca125 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Mon, 12 Dec 2016 11:28:50 -0500
Subject: [PATCH 2/2] 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 | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 3e7b689..7fa2d5e 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4243,7 +4243,9 @@ getFuncs(Archive *fout, int *numFuncs)
* 3. Otherwise, we normally exclude functions in pg_catalog. However, if
* they're members of extensions and we are in binary-upgrade mode then
* include them, since we want to dump extension members individually in
- * that mode.
+ * that mode. Also, if they are used by casts then we need to gather the
+ * information about them, though they won't be dumped if they are
+ * built-in.
*/
if (fout->remoteVersion >= 70300)
@@ -4261,11 +4263,15 @@ getFuncs(Archive *fout, int *numFuncs)
"\n AND NOT EXISTS (SELECT 1 FROM pg_depend "
"WHERE classid = 'pg_proc'::regclass AND "
"objid = p.oid AND deptype = 'i')");
- appendPQExpBufferStr(query,
+ appendPQExpBuffer(query,
"\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'::oid"
+ "\n AND p.oid = pg_cast.castfunc)",
+ g_last_builtin_oid);
if (binary_upgrade && fout->remoteVersion >= 90100)
appendPQExpBufferStr(query,
"\n OR EXISTS(SELECT 1 FROM pg_depend WHERE "
@@ -10296,7 +10302,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);
}
/*
--
2.7.4
fix_pg_dump_cast_transform_v2_master.patchtext/x-diff; charset=us-asciiDownload
From 74bdd8fb687b6c182d6ce3fcc8b6bba01e7bbbcc Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Fri, 9 Dec 2016 15:28:03 -0500
Subject: [PATCH 1/2] For 8.0 servers, get last built-in oid from pg_database
We didn't start ensuring that all built-in objects had OIDs less than
16384 until 8.1, so for 8.0 servers we still need to query the value out
of pg_database. We need this, in particular, to distinguish which casts
were built-in and which were user-defined.
For HEAD, we only worry about going back to 8.0, for the back-branches,
we also ensure that 7.0-7.4 work.
Discussion: https://www.postgresql.org/message-id/flat/20160504183952.GE10850%40tamriel.snowman.net
---
src/bin/pg_dump/pg_dump.c | 60 ++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 54 insertions(+), 6 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 7949aad..12eb018 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -96,6 +96,12 @@ bool g_verbose; /* User wants verbose narration of our
/* subquery used to convert user ID (eg, datdba) to user name */
static const char *username_subquery;
+/*
+ * For 8.0 and earlier servers, pulled from pg_database, for 8.1+ we use
+ * FirstNormalObjectId - 1.
+ */
+static Oid g_last_builtin_oid; /* value of the last builtin oid */
+
/* The specified names/patterns should to match at least one entity */
static int strict_names = 0;
@@ -233,6 +239,7 @@ static char *convertRegProcReference(Archive *fout,
const char *proc);
static char *convertOperatorReference(Archive *fout, const char *opr);
static char *convertTSFunction(Archive *fout, Oid funcOid);
+static Oid findLastBuiltinOid_V71(Archive *fout, const char *);
static void selectSourceSchema(Archive *fout, const char *schemaName);
static char *getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts);
static void getBlobs(Archive *fout);
@@ -684,6 +691,20 @@ main(int argc, char **argv)
exit_horribly(NULL,
"Exported snapshots are not supported by this server version.\n");
+ /*
+ * Find the last built-in OID, if needed (prior to 8.1)
+ *
+ * With 8.1 and above, we can just use FirstNormalObjectId - 1.
+ */
+ if (fout->remoteVersion < 80100)
+ g_last_builtin_oid = findLastBuiltinOid_V71(fout,
+ PQdb(GetConnection(fout)));
+ else
+ g_last_builtin_oid = FirstNormalObjectId - 1;
+
+ if (g_verbose)
+ write_msg(NULL, "last built-in OID is %u\n", g_last_builtin_oid);
+
/* Expand schema selection patterns into OID lists */
if (schema_include_patterns.head != NULL)
{
@@ -1494,7 +1515,7 @@ selectDumpableCast(CastInfo *cast, Archive *fout)
* This would be DUMP_COMPONENT_ACL for from-initdb casts, but they do not
* support ACLs currently.
*/
- if (cast->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (cast->dobj.catId.oid <= (Oid) g_last_builtin_oid)
cast->dobj.dump = DUMP_COMPONENT_NONE;
else
cast->dobj.dump = fout->dopt->include_everything ?
@@ -1526,7 +1547,7 @@ selectDumpableProcLang(ProcLangInfo *plang, Archive *fout)
plang->dobj.dump = DUMP_COMPONENT_NONE;
else
{
- if (plang->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (plang->dobj.catId.oid <= (Oid) g_last_builtin_oid)
plang->dobj.dump = fout->remoteVersion < 90600 ?
DUMP_COMPONENT_NONE : DUMP_COMPONENT_ACL;
else
@@ -1552,7 +1573,7 @@ selectDumpableAccessMethod(AccessMethodInfo *method, Archive *fout)
* This would be DUMP_COMPONENT_ACL for from-initdb access methods, but
* they do not support ACLs currently.
*/
- if (method->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (method->dobj.catId.oid <= (Oid) g_last_builtin_oid)
method->dobj.dump = DUMP_COMPONENT_NONE;
else
method->dobj.dump = fout->dopt->include_everything ?
@@ -1577,7 +1598,7 @@ selectDumpableExtension(ExtensionInfo *extinfo, DumpOptions *dopt)
* change permissions on those objects, if they wish to, and have those
* changes preserved.
*/
- if (dopt->binary_upgrade && extinfo->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (dopt->binary_upgrade && extinfo->dobj.catId.oid <= (Oid) g_last_builtin_oid)
extinfo->dobj.dump = extinfo->dobj.dump_contains = DUMP_COMPONENT_ACL;
else
extinfo->dobj.dump = extinfo->dobj.dump_contains =
@@ -8820,8 +8841,8 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
/*
* We unconditionally create the extension, so we must drop it if it
* exists. This could happen if the user deleted 'plpgsql' and then
- * readded it, causing its oid to be greater than FirstNormalObjectId.
- * The FirstNormalObjectId test was kept to avoid repeatedly dropping
+ * readded it, causing its oid to be greater than g_last_builtin_oid.
+ * The g_last_builtin_oid test was kept to avoid repeatedly dropping
* and recreating extensions like 'plpgsql'.
*/
appendPQExpBuffer(q, "DROP EXTENSION IF EXISTS %s;\n", qextname);
@@ -15325,6 +15346,33 @@ dumpTableConstraintComment(Archive *fout, ConstraintInfo *coninfo)
}
/*
+ * findLastBuiltinOid_V71 -
+ *
+ * find the last built in oid
+ *
+ * For 7.1 through 8.0, we do this by retrieving datlastsysoid from the
+ * pg_database entry for the current database.
+ */
+static Oid
+findLastBuiltinOid_V71(Archive *fout, const char *dbname)
+{
+ PGresult *res;
+ Oid last_oid;
+ PQExpBuffer query = createPQExpBuffer();
+
+ resetPQExpBuffer(query);
+ appendPQExpBufferStr(query, "SELECT datlastsysoid from pg_database where datname = ");
+ appendStringLiteralAH(query, dbname, fout);
+
+ res = ExecuteSqlQueryForSingleRow(fout, query->data);
+ last_oid = atooid(PQgetvalue(res, 0, PQfnumber(res, "datlastsysoid")));
+ PQclear(res);
+ destroyPQExpBuffer(query);
+
+ return last_oid;
+}
+
+/*
* dumpSequence
* write the declaration (not data) of one user-defined sequence
*/
--
2.7.4
From 0486844d8879e13a703bb59c1c6219dbfdd51c14 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Wed, 7 Dec 2016 14:59:44 -0500
Subject: [PATCH 2/2] 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 | 46 +++++++++++++++++-----
src/bin/pg_dump/t/002_pg_dump.pl | 85 +++++++++++++++++++++++++---------------
2 files changed, 90 insertions(+), 41 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 12eb018..28e3375 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4711,8 +4711,11 @@ getFuncs(Archive *fout, int *numFuncs)
* 3. Otherwise, we normally exclude functions in pg_catalog. However, if
* they're members of extensions and we are in binary-upgrade mode then
* include them, since we want to dump extension members individually in
- * that mode. Also, in 9.6 and up, include functions in pg_catalog if
- * they have an ACL different from what's shown in pg_init_privs.
+ * that mode. Also, if they are used by casts or transforms then we need
+ * to gather the information about them, though they won't be dumped if
+ * they are built-in. Also, in 9.6 and up, include functions in
+ * pg_catalog if they have an ACL different from what's shown in
+ * pg_init_privs.
*/
if (fout->remoteVersion >= 90600)
{
@@ -4746,12 +4749,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,
+ g_last_builtin_oid,
+ g_last_builtin_oid);
if (dopt->binary_upgrade)
appendPQExpBufferStr(query,
"\n OR EXISTS(SELECT 1 FROM pg_depend WHERE "
@@ -4785,11 +4797,24 @@ getFuncs(Archive *fout, int *numFuncs)
"\n AND NOT EXISTS (SELECT 1 FROM pg_depend "
"WHERE classid = 'pg_proc'::regclass AND "
"objid = p.oid AND deptype = 'i')");
- appendPQExpBufferStr(query,
+ appendPQExpBuffer(query,
"\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'::oid"
+ "\n AND p.oid = pg_cast.castfunc)",
+ g_last_builtin_oid);
+
+ if (fout->remoteVersion >= 90500)
+ appendPQExpBuffer(query,
+ "\n OR EXISTS (SELECT 1 FROM pg_transform"
+ "\n WHERE pg_transform.oid > '%u'::oid"
+ "\n AND (p.oid = pg_transform.trffromsql"
+ "\n OR p.oid = pg_transform.trftosql))",
+ g_last_builtin_oid);
+
if (dopt->binary_upgrade && fout->remoteVersion >= 90100)
appendPQExpBufferStr(query,
"\n OR EXISTS(SELECT 1 FROM pg_depend WHERE "
@@ -10966,7 +10991,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);
}
/*
@@ -11075,13 +11101,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
fix_pg_dump_cast_transform_v2_93.patchtext/x-diff; charset=us-asciiDownload
From 5ca459947757e922ee6097a4b79ed7b474bb8961 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Mon, 12 Dec 2016 12:04:16 -0500
Subject: [PATCH 1/2] For 8.0 servers, get last built-in oid from pg_database
We didn't start ensuring that all built-in objects had OIDs less than
16384 until 8.1, so for 8.0 servers we still need to query the value out
of pg_database. We need this, in particular, to distinguish which casts
were built-in and which were user-defined.
For HEAD, we only worry about going back to 8.0, for the back-branches,
we also ensure that 7.0-7.4 work.
Discussion: https://www.postgresql.org/message-id/flat/20160504183952.GE10850%40tamriel.snowman.net
---
src/bin/pg_dump/pg_dump.c | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 68cef24..2f1ef6c 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -99,7 +99,10 @@ const char *lockWaitTimeout;
/* subquery used to convert user ID (eg, datdba) to user name */
static const char *username_subquery;
-/* obsolete as of 7.3: */
+/*
+ * For 8.0 and earlier servers, pulled from pg_database, for 8.1+ we use
+ * FirstNormalObjectId - 1.
+ */
static Oid g_last_builtin_oid; /* value of the last builtin oid */
/*
@@ -697,17 +700,24 @@ main(int argc, char **argv)
"Run with --no-synchronized-snapshots instead if you do not need\n"
"synchronized snapshots.\n");
- /* Find the last built-in OID, if needed */
- if (fout->remoteVersion < 70300)
+ /*
+ * Find the last built-in OID, if needed (prior to 8.1)
+ *
+ * With 8.1 and above, we can just use FirstNormalObjectId - 1.
+ */
+ if (fout->remoteVersion < 80100)
{
if (fout->remoteVersion >= 70100)
g_last_builtin_oid = findLastBuiltinOid_V71(fout,
PQdb(GetConnection(fout)));
else
g_last_builtin_oid = findLastBuiltinOid_V70(fout);
- if (g_verbose)
- write_msg(NULL, "last built-in OID is %u\n", g_last_builtin_oid);
}
+ else
+ g_last_builtin_oid = FirstNormalObjectId - 1;
+
+ if (g_verbose)
+ write_msg(NULL, "last built-in OID is %u\n", g_last_builtin_oid);
/* Expand schema selection patterns into OID lists */
if (schema_include_patterns.head != NULL)
@@ -1419,7 +1429,7 @@ selectDumpableCast(CastInfo *cast)
if (checkExtensionMembership(&cast->dobj))
return; /* extension membership overrides all else */
- if (cast->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (cast->dobj.catId.oid <= (Oid) g_last_builtin_oid)
cast->dobj.dump = false;
else
cast->dobj.dump = include_everything;
@@ -1439,7 +1449,7 @@ selectDumpableProcLang(ProcLangInfo *plang)
if (checkExtensionMembership(&plang->dobj))
return; /* extension membership overrides all else */
- if (plang->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (plang->dobj.catId.oid <= (Oid) g_last_builtin_oid)
plang->dobj.dump = false;
else
plang->dobj.dump = include_everything;
@@ -1458,7 +1468,7 @@ selectDumpableProcLang(ProcLangInfo *plang)
static void
selectDumpableExtension(ExtensionInfo *extinfo)
{
- if (binary_upgrade && extinfo->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (binary_upgrade && extinfo->dobj.catId.oid <= (Oid) g_last_builtin_oid)
extinfo->dobj.dump = false;
else
extinfo->dobj.dump = include_everything;
@@ -8032,8 +8042,8 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
/*
* We unconditionally create the extension, so we must drop it if it
* exists. This could happen if the user deleted 'plpgsql' and then
- * readded it, causing its oid to be greater than FirstNormalObjectId.
- * The FirstNormalObjectId test was kept to avoid repeatedly dropping
+ * readded it, causing its oid to be greater than g_last_builtin_oid.
+ * The g_last_builtin_oid test was kept to avoid repeatedly dropping
* and recreating extensions like 'plpgsql'.
*/
appendPQExpBuffer(q, "DROP EXTENSION IF EXISTS %s;\n", qextname);
@@ -13920,10 +13930,10 @@ dumpTableConstraintComment(Archive *fout, ConstraintInfo *coninfo)
}
/*
- * findLastBuiltInOid -
+ * findLastBuiltinOid -
* find the last built in oid
*
- * For 7.1 and 7.2, we do this by retrieving datlastsysoid from the
+ * For 7.1 through 8.0, we do this by retrieving datlastsysoid from the
* pg_database entry for the current database
*/
static Oid
@@ -13945,7 +13955,7 @@ findLastBuiltinOid_V71(Archive *fout, const char *dbname)
}
/*
- * findLastBuiltInOid -
+ * findLastBuiltinOid -
* find the last built in oid
*
* For 7.0, we do this by assuming that the last thing that initdb does is to
--
2.7.4
From 5004fee29c47341d693789aad57d428316c08cc9 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Mon, 12 Dec 2016 11:28:50 -0500
Subject: [PATCH 2/2] 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 | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 2f1ef6c..466a373 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4204,7 +4204,9 @@ getFuncs(Archive *fout, int *numFuncs)
* 3. Otherwise, we normally exclude functions in pg_catalog. However, if
* they're members of extensions and we are in binary-upgrade mode then
* include them, since we want to dump extension members individually in
- * that mode.
+ * that mode. Also, if they are used by casts then we need to gather the
+ * information about them, though they won't be dumped if they are
+ * built-in.
*/
if (fout->remoteVersion >= 70300)
@@ -4226,7 +4228,11 @@ 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'::oid"
+ "\n AND p.oid = pg_cast.castfunc)",
+ g_last_builtin_oid);
if (binary_upgrade && fout->remoteVersion >= 90100)
appendPQExpBuffer(query,
"\n OR EXISTS(SELECT 1 FROM pg_depend WHERE "
@@ -10170,7 +10176,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);
}
/*
--
2.7.4
fix_pg_dump_cast_transform_v2_92.patchtext/x-diff; charset=us-asciiDownload
From b272e0dc45582d203fa00f36ddf63765858823d7 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Mon, 12 Dec 2016 12:32:25 -0500
Subject: [PATCH 1/2] For 8.0 servers, get last built-in oid from pg_database
We didn't start ensuring that all built-in objects had OIDs less than
16384 until 8.1, so for 8.0 servers we still need to query the value out
of pg_database. We need this, in particular, to distinguish which casts
were built-in and which were user-defined.
For HEAD, we only worry about going back to 8.0, for the back-branches,
we also ensure that 7.0-7.4 work.
Discussion: https://www.postgresql.org/message-id/flat/20160504183952.GE10850%40tamriel.snowman.net
---
src/bin/pg_dump/pg_dump.c | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index ec690b0..36c6236 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -97,7 +97,10 @@ const char *lockWaitTimeout;
/* subquery used to convert user ID (eg, datdba) to user name */
static const char *username_subquery;
-/* obsolete as of 7.3: */
+/*
+ * For 8.0 and earlier servers, pulled from pg_database, for 8.1+ we use
+ * FirstNormalObjectId - 1.
+ */
static Oid g_last_builtin_oid; /* value of the last builtin oid */
/*
@@ -657,17 +660,24 @@ main(int argc, char **argv)
else
username_subquery = "SELECT usename FROM pg_user WHERE usesysid =";
- /* Find the last built-in OID, if needed */
- if (fout->remoteVersion < 70300)
+ /*
+ * Find the last built-in OID, if needed (prior to 8.1)
+ *
+ * With 8.1 and above, we can just use FirstNormalObjectId - 1.
+ */
+ if (fout->remoteVersion < 80100)
{
if (fout->remoteVersion >= 70100)
g_last_builtin_oid = findLastBuiltinOid_V71(fout,
PQdb(GetConnection(fout)));
else
g_last_builtin_oid = findLastBuiltinOid_V70(fout);
- if (g_verbose)
- write_msg(NULL, "last built-in OID is %u\n", g_last_builtin_oid);
}
+ else
+ g_last_builtin_oid = FirstNormalObjectId - 1;
+
+ if (g_verbose)
+ write_msg(NULL, "last built-in OID is %u\n", g_last_builtin_oid);
/* Expand schema selection patterns into OID lists */
if (schema_include_patterns.head != NULL)
@@ -1275,7 +1285,7 @@ selectDumpableCast(CastInfo *cast)
if (checkExtensionMembership(&cast->dobj))
return; /* extension membership overrides all else */
- if (cast->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (cast->dobj.catId.oid <= (Oid) g_last_builtin_oid)
cast->dobj.dump = false;
else
cast->dobj.dump = include_everything;
@@ -1295,7 +1305,7 @@ selectDumpableProcLang(ProcLangInfo *plang)
if (checkExtensionMembership(&plang->dobj))
return; /* extension membership overrides all else */
- if (plang->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (plang->dobj.catId.oid <= (Oid) g_last_builtin_oid)
plang->dobj.dump = false;
else
plang->dobj.dump = include_everything;
@@ -1314,7 +1324,7 @@ selectDumpableProcLang(ProcLangInfo *plang)
static void
selectDumpableExtension(ExtensionInfo *extinfo)
{
- if (binary_upgrade && extinfo->dobj.catId.oid < (Oid) FirstNormalObjectId)
+ if (binary_upgrade && extinfo->dobj.catId.oid <= (Oid) g_last_builtin_oid)
extinfo->dobj.dump = false;
else
extinfo->dobj.dump = include_everything;
@@ -7513,8 +7523,8 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
/*
* We unconditionally create the extension, so we must drop it if it
* exists. This could happen if the user deleted 'plpgsql' and then
- * readded it, causing its oid to be greater than FirstNormalObjectId.
- * The FirstNormalObjectId test was kept to avoid repeatedly dropping
+ * readded it, causing its oid to be greater than g_last_builtin_oid.
+ * The g_last_builtin_oid test was kept to avoid repeatedly dropping
* and recreating extensions like 'plpgsql'.
*/
appendPQExpBuffer(q, "DROP EXTENSION IF EXISTS %s;\n", qextname);
@@ -13330,10 +13340,10 @@ dumpTableConstraintComment(Archive *fout, ConstraintInfo *coninfo)
}
/*
- * findLastBuiltInOid -
+ * findLastBuiltinOid -
* find the last built in oid
*
- * For 7.1 and 7.2, we do this by retrieving datlastsysoid from the
+ * For 7.1 through 8.0, we do this by retrieving datlastsysoid from the
* pg_database entry for the current database
*/
static Oid
@@ -13355,7 +13365,7 @@ findLastBuiltinOid_V71(Archive *fout, const char *dbname)
}
/*
- * findLastBuiltInOid -
+ * findLastBuiltinOid -
* find the last built in oid
*
* For 7.0, we do this by assuming that the last thing that initdb does is to
--
2.7.4
From a5a57c57a23806dd5c8c30a4ecc7265207000e57 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Mon, 12 Dec 2016 12:34:02 -0500
Subject: [PATCH 2/2] 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 | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 36c6236..68468e3 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3856,7 +3856,9 @@ getFuncs(Archive *fout, int *numFuncs)
* 3. Otherwise, we normally exclude functions in pg_catalog. However, if
* they're members of extensions and we are in binary-upgrade mode then
* include them, since we want to dump extension members individually in
- * that mode.
+ * that mode. Also, if they are used by casts then we need to gather the
+ * information about them, though they won't be dumped if they are
+ * built-in.
*/
if (fout->remoteVersion >= 70300)
@@ -3878,7 +3880,11 @@ 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'::oid "
+ "\n AND p.oid = pg_cast.castfunc)",
+ g_last_builtin_oid);
if (binary_upgrade && fout->remoteVersion >= 90100)
appendPQExpBuffer(query,
"\n OR EXISTS(SELECT 1 FROM pg_depend WHERE "
@@ -9651,7 +9657,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);
}
/*
--
2.7.4
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
That's a good point, we might be doing things wrong in other places in
the code by using FirstNormalObjectId on pre-8.1 servers.What I suggest then is an independent patch which uses a different
variable than FirstNormalObjectId and sets it correctly based on the
version of database that we're connecting to,+1
Done.
Thanks!
Stephen