Re: ALTER TABLE ADD COLUMN fast default

From: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: ALTER TABLE ADD COLUMN fast default
Date: 2017-12-31 23:48:29
Message-ID: 67fa4ee5-0060-e950-2f64-1d49312d91a2@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 12/12/2017 02:13 PM, Andrew Dunstan wrote:
>
> On 12/06/2017 11:26 AM, Andrew Dunstan wrote:
>>> In general, you need to be thinking about this in terms of making sure
>>> that it's impossible to accidentally not update code that needs to be
>>> updated. Another example is that I noticed that some places the patch
>>> has s/CreateTupleDescCopy/CreateTupleDescCopyConstr/, evidently because
>>> the former will fail to copy the missing-attribute support data.
>>> I think that's an astonishingly bad idea. There is basically no situation
>>> in which CreateTupleDescCopy defined in that way will ever be safe to use.
>>> The missing-attribute info is now a fundamental part of a tupdesc and it
>>> has to be copied always, just as much as e.g. atttypid.
>>>
>>> I would opine that it's a mistake to design TupleDesc as though the
>>> missing-attribute data had anything to do with default values. It may
>>> have originated from the same place but it's a distinct thing. Better
>>> to store it in a separate sub-structure.
>> OK, will work on all that. Thanks again.
>>
>
>
> Looking closer at this. First, there is exactly one place where the
> patch does  s/CreateTupleDescCopy/CreateTupleDescCopyConstr/.
>
> making this like atttypid suggests that it belongs right outside the
> TupleConstr structure. But then it seems to me that the change could
> well end up being a darn site more invasive. For example, should we be
> passing the number of missing values to CreateTemplateTupleDesc and
> CreateTupleDesc? I'm happy to do whatever work is required, but I want
> to have a firm understanding of the design before I spend lots of time
> cutting code. Once I understand how tupdesc.h should look I should be good.
>

Here is a new version of the patch. It separates the missing values
constructs and logic from the default values constructs and logic. The
missing values now sit alongside the default values in tupleConstr. In
some places the separation makes the logic a good bit cleaner.

Some of the logic is also reworked to remove an assumption that the
missing values structure is populated in attnum order, Also code to
support the missing stuctures is added to equalTupleDescs.

We still have that one extra place where we need to call
CreateTupleDescCopyConstr instead of CreateTupleDescCopy. I haven't seen
an easy way around that.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
fast_default-v4.patch text/x-patch 103.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2018-01-01 03:43:55 Re: Contributing with code
Previous Message Alvaro Herrera 2017-12-31 19:49:08 Re: Foreign keys and partitioned tables