[PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead
От | Nikolay Shaplov |
---|---|
Тема | [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead |
Дата | |
Msg-id | 2083183.Rn7qOxG4Ov@x200m обсуждение исходный текст |
Ответы |
Re: [PATCH] get rid of StdRdOptions, use individual binaryreloptions representation for each relation kind instead
|
Список | 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.
Вложения
В списке pgsql-hackers по дате отправления: