Re: Allowing ALTER TYPE to change storage strategy
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: pgsql-hackers@postgresql.org
Date: 2020-03-02T19:11:10Z
Lists: pgsql-hackers
Attachments
- alter-type-v3.patch (text/x-diff) patch v3
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 [, ... ] ) 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
Commits
-
Allow ALTER TYPE to change some properties of a base type.
- fe30e7ebfa38 13.0 landed