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.)



Reply via email to