Re: Allowing ALTER TYPE to change storage strategy - Mailing list pgsql-hackers
From | Tomas Vondra |
---|---|
Subject | Re: Allowing ALTER TYPE to change storage strategy |
Date | |
Msg-id | 20200302220531.cdz4rlbw5gmcicsa@development Whole thread Raw |
In response to | Re: Allowing ALTER TYPE to change storage strategy (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: Allowing ALTER TYPE to change storage strategy
|
List | pgsql-hackers |
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
pgsql-hackers by date: