Re: Allowing ALTER TYPE to change storage strategy

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Allowing ALTER TYPE to change storage strategy
Date: 2020-03-02 19:11:10
Message-ID: 10968.1583176270@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I started to look at this patch with fresh eyes after reading the patch
for adding binary I/O for ltree,

https://www.postgresql.org/message-id/flat/CANmj9Vxx50jOo1L7iSRxd142NyTz6Bdcgg7u9P3Z8o0=HGkYyQ(at)mail(dot)gmail(dot)com

and realizing that the only reasonable way to tackle that problem is to
provide an ALTER TYPE command that can set the binary I/O functions for
an existing type. (One might think that it'd be acceptable to UPDATE
the pg_type row directly; but that wouldn't take care of dependencies
properly, and it also wouldn't handle the domain issues I discuss below.)
There are other properties too that can be set in CREATE TYPE but we
have no convenient way to adjust later, though it'd be reasonable to
want to do so.

I do not think that we want to invent bespoke syntax for each property.
The more such stuff we cram into ALTER TYPE, the bigger the risk of
conflicting with some future SQL extension. Moreover, since CREATE TYPE
for base types already uses the "( keyword = value, ... )" syntax for
these properties, and we have a similar precedent in CREATE/ALTER
OPERATOR, it seems to me that the syntax we want here is

ALTER TYPE typename SET ( keyword = value [, ... ] )

Attached is a stab at doing it that way, and implementing setting of
the binary I/O functions for good measure. (It'd be reasonable to
add more stuff, like setting the other support functions, but this
is enough for the immediate discussion.)

The main thing I'm not too happy about is what to do about domains.
Your v2 patch supposed that it's okay to allow ALTER TYPE on domains,
but I'm not sure we want to go that way, and if we do there's certainly
a bunch more work that has to be done. Up to now the system has
supposed that domains inherit all these properties from their base
types. I'm not certain exactly how far that assumption has propagated,
but there's at least one place that implicitly assumes it: pg_dump has
no logic for adjusting a domain to have different storage or support
functions than the base type had. So as v2 stands, a custom storage
option on a domain would be lost in dump/reload.

Another issue that would become a big problem if we allow domains to
have custom I/O functions is that the wire protocol transmits the
base type's OID, not the domain's OID, for an output column that
is of a domain type. A client that expected a particular output
format on the strength of what it was told the column type was
would be in for a big surprise.

Certainly we could fix pg_dump if we had a mind to, but changing
the wire protocol for this would have unpleasant ramifications.
And I'm worried about whether there are other places in the system
that are also making this sort of assumption.

I'm also not very convinced that we *want* to allow domains to vary from
their base types in this way. The primary use-case I can think of for
ALTER TYPE SET is in extension update scripts, and an extension would
almost surely wish for any domains over its type to automatically absorb
whatever changes of this sort it wants to make.

So I think there are two distinct paths we could take here:

* Decide that it's okay to allow domains to vary from their base type
in these properties. Teach pg_dump to cope with that, and stand ready
to fix any other bugs we find, and have some story to tell the people
whose clients we break. Possibly add a CASCADE option to
ALTER TYPE SET, with the semantics of adjusting dependent domains
to match. (This is slightly less scary than the CASCADE semantics
you had in mind, because it would only affect pg_type entries not
tables.)

* Decide that we'll continue to require domains to match their base
type in all these properties. That means refusing to allow ALTER
on a domain per se, and automatically cascading these changes to
dependent domains.

In the v3 patch below, I've ripped out the ALTER DOMAIN syntax on
the assumption that we'd do the latter; but I've not written the
cascade recursion logic, because that seemed like a lot of work
to do in advance of having consensus on it being a good idea.

I've also restricted the code to work just on base types, because
it's far from apparent to me that it makes any sense to allow any
of these operations on derived types such as composites or ranges.
Again, there's a fair amount of code that is not going to be
prepared for such a type to have properties that it could not
have at creation, and I don't see a use-case that really justifies
breaking those expectations.

Thoughts?

regards, tom lane

Attachment Content-Type Size
alter-type-v3.patch text/x-diff 34.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andy Fan 2020-03-02 20:25:51 Re: [PATCH] Erase the distinctClause if the result is unique by definition
Previous Message Cary Huang 2020-03-02 19:06:57 [PATCH] Documentation bug related to client authentication using TLS certificate