On Mon, Jan 22, 2024 at 5:32 PM Peter Eisentraut <peter@eisentraut.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