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:
> >> (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