Re: partitioning and identity column

On Tue, Jan 23, 2024 at 12:29 AM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 22.01.24 13:23, Ashutosh Bapat wrote:
> >>       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.
>
> Ok.  Maybe just rephrase that comment somehow then?

Please see refactoring patches attached to [1]. Refactoring that way
makes it unnecessary to mention "regular inheritance" in each comment.
Yet I have included a modified version of the comment in that patch
set.

>
> > 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?
>
> I suppose the number of inheritance levels is usually not a problem for
> stack depth?
>

Practically it should not. I would rethink the application design if
it requires so many inheritance or partition levels. But functions in
optimizer like try_partitionwise_join() and set_append_rel_size() call

/* Guard against stack overflow due to overly deep inheritance tree. */
check_stack_depth();

I am fine if we want to skip this.

-- 
Best Wishes,
Ashutosh Bapat