Re: partitioning and identity column

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