Re: factorial function/phase out postfix operators? - Mailing list pgsql-hackers

From Mark Dilger
Subject Re: factorial function/phase out postfix operators?
Date
Msg-id 6F87472B-085B-4409-A3C9-FE99E2E57F7A@enterprisedb.com
Whole thread Raw
In response to Re: factorial function/phase out postfix operators?  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: factorial function/phase out postfix operators?  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers

> On Sep 11, 2020, at 11:25 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Mark Dilger <mark.dilger@enterprisedb.com> writes:
>> On Sep 11, 2020, at 8:36 AM, Tom Lane <tgl@sss.pgh.pa.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
todo with postfix operators specifically, but be more general like --dump-incompatible-objects vs.
--omit-incompatible-objectsvs. --error-on-incompatible-objects vs. --do-your-best-to-fixup-incompatible-objects, with
oneof 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
forcomment. 

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






pgsql-hackers by date:

Previous
From: John Naylor
Date:
Subject: Re: WIP: BRIN multi-range indexes
Next
From: Julien Rouhaud
Date:
Subject: Re: track_planning causing performance regression