Re: ALTER tbl rewrite loses CLUSTER ON index

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: ALTER tbl rewrite loses CLUSTER ON index
Date: 2020-03-12 17:19:56
Message-ID: 25886.1584033596@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Justin Pryzby <pryzby(at)telsasoft(dot)com> writes:
> @cfbot: resending with only Amit's 0001, since Michael pushed a variation on
> 0002.

Boy, I really dislike this patch. ATPostAlterTypeParse is documented as
using the supplied definition string, and nothing else, to reconstruct
the index. This breaks that without even the courtesy of documenting
the breakage. Moreover, the reason why it's designed like that is to
avoid requiring the old index objects to still be accessible. So I'm
surprised that this hack works at all. I don't think it would have
worked at the time the code was first written, and I think it's imposing
a constraint we'll have problems with (again?) in future.

The right way to fix this is to note the presence of the indisclustered
flag when we're examining the index earlier, and include a suitable
command in the definition string. So probably pg_get_indexdef_string()
is what needs to change. It doesn't look like that's used anywhere
else, so we can just redefine its behavior as needed.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2020-03-12 17:30:47 Re: PATCH: add support for IN and @> in functional-dependency statistics use
Previous Message Alexey Kondratov 2020-03-12 17:08:46 Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly