Re: factorial function/phase out postfix operators?

Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> writes:
> On Sep 11, 2020, at 8:36 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> My inclination is to simply not change pg_dump. There is no need to break
>> the use-case of loading the output back into the server version it came
>> from, if we don't have to. If the output is getting loaded into a server
>> that lacks postfix operators, that server can throw the error. There's no
>> real gain in having pg_dump prejudge the issue.

> I think some kind of indication that the dump won't be loadable is
> useful if they're planning to move the dump file across an expensive
> link, or if they intend to blow away the old data directory to make room
> for the new. Whether that indication should be in the form of a warning
> or an error is less clear to me.

I think definitely not an error, because that breaks a plausible (even if
not recommended) use-case.

> Whatever we do here, I think it sets a precedent for how such situations
> are handled in the future, so maybe focusing overmuch on the postfix
> operator issue is less helpful than on the broader concept. What, for
> example, would we do if we someday dropped GiST support?

I'm not sure that there is or should be a one-size-fits-all policy.
We do actually have multiple precedents already:

* DefineIndex substitutes "gist" for "rtree" to allow transparent updating
of dumps from DBs that used the old rtree AM.

* Up till very recently (84eca14bc), ResolveOpClass had similar hacks to
substitute for old opclass names.

* bb03010b9 and e58a59975 got rid of other server-side hacks for
converting old dump files.

So generally the preference is to make the server deal with conversion
issues; and this must be so, since what you have to work with may be a
dump taken with an old pg_dump. In this case, though, it doesn't seem
like there's any plausible way for the server to translate old DDL.

As for the pg_dump side, aside from the WITH OIDS precedent you mentioned,
there was till recently (d9fa17aa7) code to deal with unconvertible
pre-7.1 aggregates. That code issued a pg_log_warning and then ignored
(didn't dump) the aggregate. I think it didn't have much choice about
the latter step because, if memory serves, there simply wasn't any way to
represent those old aggregates in the new CREATE AGGREGATE syntax; so we
couldn't leave it to the server to decide whether to throw error or not.
(It's also possible, given how far back that was, that we simply weren't
being very considerate of upgrade issues. It's old enough that I would
not take it as great precedent. But it is a precedent.)

The behavior of WITH OIDS is to issue a pg_log_warning and then ignore
the property. I do not much care for this, although I see the point that
we don't want to stick WITH OIDS into the CREATE TABLE because then the
CREATE would fail, leaving the dump completely unusable on newer servers.
My choice would have been to write CREATE TABLE without that option and
then add ALTER TABLE ... WITH OIDS. In this way the dump script does
what it should when restoring into an old server, while if you load into
a new server you hear about it --- and you can ignore the error if you
want.

I think the right thing for postfix operators is probably to issue
pg_log_warning and then dump the object anyway.

regards, tom lane