On Thu, Jan 27, 2011 at 2:48 PM, Noah Misch <noah@leadboat.com> wrote:
> Done as attached. This preserves compatibility with our current handling of
> composite type dependencies. The rest you've seen before.
OK, so I took a look at this in more detail today. The current logic
for table rewrites looks like this:
1. If we're changing the data type of a column, or adding a column
with a default, or adding/dropping OIDs, rewrite the table. Stop.
2. Otherwise, if we're adding a constraint or NOT NULL, scan the table
and check constraints.
3. If we're changing tablespaces, copy the table block-by-block.
It seems to me that the revised logic needs to look like this:
1. If we're changing the data type of a column and the existing
contents are not binary coercible to the new contents, or if we're
adding a column with a default or adding/dropping OIDs, rewrite the
table. Stop.
2. Otherwise, if we're adding a constraint or NOT NULL, scan the table
and check constraints.
3. If we're changing the data type of a column in the table, reindex the table.
4. If we're changing tablespaces, copy the table block-by-block.
I might be missing something, but I don't see that the patch includes
step #3, which I think is necessary. For example, citext is binary
coercible to text, but you can't reuse the index because the
comparison function is different. Of course, if you're changing the
type of a column to its already-current type, you can skip #3, but if
that's the only case we can optimize, it's not much of an
accomplishment. I guess this gets back to the ALTER TYPE 7 patch,
which I haven't looked at in detail, but I have a feeling it may be
controversial.
Another problem here is that if you have to do both #2 and #3, you
might have been better off (or just as well off) doing #1, unless you
can somehow jigger things so that the same scan does both the
constraint checks and the index rebuild. That doesn't look simple.
Thoughts?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company