Re: BUG #14866: The generated constraint in the typed table causes the server to crash

Sorry Michael. I didn't notice this in my gmail inbox until today.

On 2017/11/01 23:05, Michael Paquier wrote:
> On Wed, Nov 1, 2017 at 1:36 PM, Peter Eisentraut
> <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>> On 10/23/17 18:21, Michael Paquier wrote:
>>> Attached is a patch to address the problem. There are a couple of
>>> things to consider:
>>> - transformColumnDefinition is missing the fact that a type may not be
>>> set for a column defined, and as far as I can see the type name is
>>> needed beforehand to allow the generation of all the serial commands.
>>> This can happen when using CREATE TABLE OF, as you reported, for which
>>> the data type can be found in the type defined. But this can happen as
>>> well when declaring a child partition.
>>
>> I wonder whether we should even allow this. The SQL standard does not
>> allow identity columns in typed tables, so there is support for that.
>
> OK. At the same time being able to support that is not complicated
> either, even if the patch I sent earlier is a bit grotty in the way it
> does handle it. I can see arguments in favor of either solution.

Agree that it's not that hard to support IDENTITY on typed table columns.

I rewrote your patch a bit so that it now makes transformColumnDefinition
use the ColumnDef initialized by transformOfType which contains enough
information to get the needed type OID. I think you might find it a bit
simpler. :)

Also, I carved it out into the attached patch 0001.

>> I'm not sure whether it makes sense in partitions. You are supposed to
>> insert through the partition root, so making identity columns in
>> partitions would just be confusing.
>
> Robert, Amit, do you have opinions on the matter? It is possible to
> have multiple level of partitions as well.

IMHO, we should support IDENTITY on partitions, just like we support
specifying DEFAULT on partitions. We support issuing direct INSERT on
partitions no matter where in the hierarchy it is, so having these
features work normally makes sense to me.

I looked at your patch and saw a minor problem with it. With your patch,
there exists a lock-strength-upgrade deadlock hazard. It is making an
earlier phase of partition creation (transformCreateStmt) take an
AccessShareLock on the parent table, whereas a later phase
(DefineRelation) will take an AccessExclusiveLock. I modified your patch
to take the AccessExclusiveLock to begin with and added a comment nearby
clarifying the same.

You'll find the updated version in attached 0002.

Sorry again for the delay in replying.

Thanks,
Amit