Horton

Re: tablecmds.c/MergeAttributes() cleanup

Peter Eisentraut <peter@eisentraut.org>

View thread View on postgr.es
From: Peter Eisentraut <peter@eisentraut.org>
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>, Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: 2024-01-26T13:42:17Z
Lists: pgsql-hackers 
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.)

Commits

  1. Fix segmentation fault in MergeInheritedAttribute()

  2. Improve compression and storage support with inheritance

  3. Split some code out from MergeAttributes()

  4. MergeAttributes code deduplication

  5. Add a const decoration

  6. MergeAttributes: convert pg_attribute back to ColumnDef before comparing

  7. Refactor ATExecAddColumn() to use BuildDescForRelation()

  8. Move BuildDescForRelation() from tupdesc.c to tablecmds.c

  9. Push attcompression and attstorage handling into BuildDescForRelation()

  10. Push attidentity and attgenerated handling into BuildDescForRelation()

  11. Add TupleDescGetDefault()

  12. MergeAttributes() and related variable renaming

  13. Clean up MergeCheckConstraint()

  14. Clean up MergeAttributesIntoExisting()

  15. Remove useless if condition

  16. Make more use of makeColumnDef()

  17. Add some const decorations

  18. Remove ancient special case code for dropping oid columns

  19. Remove ancient special case code for adding oid columns

  20. Remove obsolete comment about OID support