Re: Allowing ALTER TYPE to change storage strategy

On Mon, Mar 02, 2020 at 02:11:10PM -0500, Tom Lane wrote:
>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@mail.gmail.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 [, ... ] )
>

Agreed, it seems reasonable to use the ALTER OPRATOR precedent.

>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 do agree we should do the latter, i.e. maintain the assumption that
domains have the same properties as their base type. I can't think of a
use case for allowing them to differ, it just didn't occur to me there
is this implicit assumption when writing the patch.

>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.
>

Yeah, that makes sense too, I think.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services