On Tue, Jan 23, 2024 at 12:29 AM Peter Eisentraut <pe...@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