[PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead - Mailing list pgsql-hackers

This is part or my bigger patch
https://www.postgresql.org/message-id/flat/2146419.veIEZdk4E4@x200m#2146419.veIEZdk4E4@x200mwe've decided to 
 
commit by smaller parts.

Now in postgres an StdRdOptions structure is used as binary represenations of 
reloptions for heap, toast, and some indexes. It has a number of fields, and 
only heap relation uses them all. Toast relations uses only autovacuum 
options, and indexes uses only fillfactor option.

So for example if you set custom fillfactor value for some index, then it will 
lead to allocating 84 bytes of memory (sizeof StdRdOptions on i386) and only 8 
bytes will be actually used (varlena header and fillfactor value). 74 bytes is 
not much, but allocating them for each index for no particular reason is bad 
idea, as I think.

Moreover when I wrote my big reloption refactoring patch, I came to "one 
reloption kind - one binary representation" philosophy. It allows to write 
code with more logic in it.

This patch replaces StdRdOptions with HeapRelOptions, ToastRelOptions, 
BTRelOptions, HashRelOptions, SpGistRelOptions and PartitionedRelOptions

one for each relation kind that were using StdRdOptions before.

The second thing I've done, I've renamed Relation* macroses from 
src/include/utils/rel.h, that were working with reloptions. I've renamed them 
into Heap*, Toast* and View* (depend on what relation options they were 
actually using)

I did it because there names were misleading. For example 
RelationHasCheckOption can be called only for View relation, and will give 
wrong result for other relation types. It just takes binary representation of 
reloptions, cast is to (ViewOptions *) and then returns some value from it. 
Naming it as ViewHasCheckOption would better reflect what it actually do, and 
strictly specify that it is applicable only to View relations.


Possible flaws:

I replaced 

saveFreeSpace = RelationGetTargetPageFreeSpace(state->rs_new_rel,
                                    HEAP_DEFAULT_FILLFACTOR);

with

if (IsToastRelation(state->rs_new_rel))
      saveFreeSpace = ToastGetTargetPageFreeSpace();
else
      saveFreeSpace = HeapGetTargetPageFreeSpace(state->rs_new_rel);

wherever I met it (and other cases like that), but I am not sure if in some 
cases that part of code is used for heap only or not. So may be this "ifs" is 
not needed and should be removed, and only Heap-case should be left. But I am 
not that much familiar with postgres internals to see it for sure... I need 
advice of more experienced developers here.

-- 
Do code for fun.