Re: factorial function/phase out postfix operators?

> On Sep 11, 2020, at 11:25 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> 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.

That happens to be the patch behavior as it stands now.

Another option would be to have pg_dump take a strictness mode option. I don't think the option should have anything to do with postfix operators specifically, but be more general like --dump-incompatible-objects vs. --omit-incompatible-objects vs. --error-on-incompatible-objects vs. --do-your-best-to-fixup-incompatible-objects, with one of those being the default (and with all of them having better names). If --error-on-incompatible-objects were the default, that would behave as Robert recommended upthread.

I can totally see an objection to the added complexity of such options, so I'm really just putting this out on the list for comment.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company