From: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
---|---|
To: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: partitioning and identity column |
Date: | 2024-01-22 12:23:19 |
Message-ID: | CAExHW5uX+cnpPuiqbnafuiAMTSEvZHh4s=h-G9_32DYDbbOReA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Jan 22, 2024 at 5:32 PM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> I found another piece of code that might need updating, or at least the
> comment.
>
> In MergeAttributes(), in the part that merges the specified column
> definitions into the inherited ones, it says
>
> /*
> * Identity is never inherited. The new column can have an
> * identity definition, so we always just take that one.
> */
> def->identity = newdef->identity;
>
> This is still correct for regular inheritance, but not for partitioning.
> I think for partitioning, this is not reachable because you can't
> specify identity information when you create a partition(?). So maybe
> something like
You may specify the information when creating a partition, but it will
cause an error. We have tests in identity.sql for the same (look for
pitest1_pfail).
>
> if (newdef->identity)
> {
> Assert(!is_partioning);
> /*
> * Identity is never inherited. The new column can have an
> * identity definition, so we always just take that one.
> */
> def->identity = newdef->identity;
> }
>
> Thoughts?
That code block already has Assert(!is_partition) at line 3085. I
thought that Assert is enough.
There's another thing I found. The file isn't using
check_stack_depth() in the function which traverse inheritance
hierarchies. This isn't just a problem of the identity related
function but most of the functions in that file. Do you think it's
worth fixing it?
--
Best Wishes,
Ashutosh Bapat
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2024-01-22 12:26:16 | Re: Synchronizing slots from primary to standby |
Previous Message | Bharath Rupireddy | 2024-01-22 12:23:15 | Re: Remove unused fields in ReorderBufferTupleBuf |