Re: pg_dump vs. TRANSFORMs

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)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