Re: Which SET TYPE don't actually require a rewrite

On Sat, Jul 18, 2020 at 4:57 AM Noah Misch <noah(at)leadboat(dot)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 <noah(at)leadboat(dot)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.GD13163@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/>