On Sat, Jul 18, 2020 at 4:57 AM Noah Misch <n...@leadboat.com> wrote:
> On Fri, Jul 17, 2020 at 04:08:36PM +0200, Magnus Hagander wrote: > > On Fri, Jul 17, 2020 at 5:40 AM Noah Misch <n...@leadboat.com> wrote: > > > On Wed, Jul 15, 2020 at 02:54:37PM +0200, Magnus Hagander wrote: > > > > Our Fine Manual (TM) specifies: > > > > "As an exception, when changing the type of an existing column, if > the > > > > USING clause does not change the column contents and the old type is > > > either > > > > binary coercible to the new type or an unconstrained domain over the > new > > > > type, a table rewrite is not needed; but any indexes on the affected > > > > columns must still be rebuilt." > > > > > > > > First of all, how is a non-internals-expert even supposed to know > what a > > > > binary coercible type is? > > > > > > The manual defines it at <firstterm>binary coercible</firstterm>. > > > > The only way to actually realize that this is a <firstterm> is to look at > > the source code though, right? > > I see italic typeface for <firstterm>. This one deserves an <indexterm>, > too. > (I bet many other <firstterm> uses deserve an <indexterm>.) > > > It's definitely not clear that one should go > > look at the CREATE CAST documentation to find the definition -- certainly > > not from the ALTER TABLE documentation, which I would argue is the place > > where most people would go. > > Agreed. > > > > We can also for example increase the precision of numeric without a > > > rewrite > > > > (but not scale). Or we can change between text and varchar. And we > can > > > > increase the length of a varchar but not decrease it. > > > > > > > > Surely we can do better than this when it comes to documenting it? > Even > > > if > > > > it's a pluggable thing so it may or may not be true of external > > > > datatypes installed later, we should be able to at least be more > clear > > > > about the builtin types, I think? > > > > > > I recall reasoning that ATColumnChangeRequiresRewrite() is a DDL > analog of > > > query optimizer logic. The manual brings up only a minority of planner > > > optimizations, and comprehensive lists of optimization preconditions > are > > > even > > > rarer. But I don't mind if $SUBJECT documentation departs from that > norm. > > > > I can see the argument being made for that, and certainly having been > made > > for it in the future. But I'd say given the very bad consequences of > > getting it wrong, it's far from minor. And given the number of times I've > > had to answer the question "can I make this change safely" (which usually > > amounts to me trying it out to see what happens, if I hadn't done that > > exact one many times before) indicates the need for a more detailed > > documentation on it. > > Such a doc addition is fine with me. I agree with Tom that it will be > prone > to staleness, but I don't conclude that the potential for staleness reduces > its net value below zero. Having said that, if the consequences of doc > staleness are "very bad", you may consider documenting the debug1 user > interface (https://postgr.es/m/20121202020736.gd13...@tornado.leadboat.com > ) > instead of documenting the exact rules. Either way is fine with me. > The DEBUG1 method is only after the fact though, isn't it? That makes it pretty hard for someone to say review a migration script and see "this is going to cause problems". And if it's going to be run in an env, I personally find it more useful to just stick an event trigger in there per our documentation and block it (though it might be a good idea to link to that from the alter table reference page, and not just have it under event trigger examples). I agree that documenting the rules would definitely be prone to staleness, and that having EXPLAIN for DDL would be the *better* solution. But also that having the docs, even if they go a bit stale, would be better than the scenario today. Unfortunately, I'm not sure I know enough of the details of what the rules actually *are* to explain them in a way that's easy enough to go in the docs... -- Magnus Hagander Me: https://www.hagander.net/ <http://www.hagander.net/> Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
