Re: tablecmds.c/MergeAttributes() cleanup

On 24.01.24 07:27, Ashutosh Bapat wrote:
> While working on identity support and now while looking at the
> compression problem you referred to, I found MergeAttribute() to be
> hard to read. It's hard to follow high level logic in that function
> since the function is not modular. I took a stab at modularising a
> part of it. Attached is the resulting patch series.
>
> 0001 is your patch as is
> 0002 is pgindent fix and also fixing what I think is a typo/thinko
> from 0001. If you are fine with the changes, 0002 should be merged
> into 0003.
> 0003 separates the part of code merging a child attribute to the
> corresponding inherited attribute into its own function.
> 0004 does the same for code merging inherited attributes incrementally.
>
> I have kept 0003 and 0004 separate in case we pick one and not the
> other. But they can be committed as a single commit.

I have committed all this. These are great improvements.

(One little change I made to your 0003 and 0004 patches is that I kept
the check whether the new column matches an existing one by name in
MergeAttributes(). I found that pushing that down made the logic in
MergeAttributes() too hard to follow. But it's pretty much the same.)