[PATCH] New [relation] option engine - Mailing list pgsql-hackers
From | Nikolay Shaplov |
---|---|
Subject | [PATCH] New [relation] option engine |
Date | |
Msg-id | 3766675.7eaCOWfIcx@thinkpad-pgpro Whole thread Raw |
Responses |
Re: [PATCH] New [relation] option engine
Re: [PATCH] New [relation] option engine |
List | pgsql-hackers |
Hi! I'd like to introduce a patch that reworks options processing. This patch detaches code for options processing and validating from the code that uses these options. This will allow to reuse same code in any part of postgres that may need options. Currently there are three ways of defining options, and this code is scattered along the source tree, that leads to a problems: code is hard to understand, hard to maintain, I saw several options added or changed, and and in one of two cases, there was one or another mistake done because of code unclearity. General idea: There is an object OptionSpec that describes how single option should be parsed, validated, stored into bytea, etc. OptionSpecSet is a list of OptionSpecs, that describes all possible options for certain object (e.g. options for nbtree index relation) All options related functions that are not depended on context were moved to src/backend/access/common/options.c. These functions receives OptionSpecSet (or OptionSpec) and option data that should be processed. And OptionSpecSet should contain all information that is needed for proper processing. Options from the perspective of the option engine can exist in four representation: - defList - the way they came from SQL parser - TEXT[] - the way they are stored in pg_class or similar place - Bytea - the way they stored in C-structure, for caching and using in postgres code that uses these options - Value List - is an internal representation that is used while parsing, validating and converting between representations See code comments for more info. There are functions that is used for conversion from one representation to another: - optionsDefListToRawValues : defList -> Values - optionsTextArrayToDefList :TEXT[] -> defList - optionsTextArrayToRawValues : TEXT[] -> Values - optionsValuesToTextArray: Values -> TEXT[] - optionsValuesToBytea: Values -> Bytea This functions are called from meta-functions that is used when postgres receives an SQL command for creating or updating options: - optionsDefListToTextArray - when options are created - optionsUpdateTexArrayWithDefList - when option are updated. They also trigger validation while processing. There are also functions for SpecSet processing: - allocateOptionsSpecSet - optionsSpecSetAddBool - optionsSpecSetAddBool - optionsSpecSetAddReal - optionsSpecSetAddEnum - optionsSpecSetAddString For index access methods "amoptions" member function that preformed option processing, were replaced with "amreloptspecset" member function that provided an SpecSet for reloptions for this AM, so caller can trigger option processing himself. For other relation types options have been processing by "fixed" functions, so these processing were replaced with "fixed" SpecSet providers + processing using that SpecSet. Later on these should be moved to access method the same way it is done in indexes. I plan to do it after this patch is commit. As for local options, that is used of opclass options, I kept all current API, but made this API a wrapper around new option engine. Local options should be moved to unified option engine API later on. I hope to do it too. This patch does not change any of postgres behaviour (even if this behaviour is not quite right). The only change is text of the warning for unexisting option in toast namespace. But I hope this change is for better. The only problem I am not sure how to solve is an obtaining a LockMode. To get a LockMode for option , you need a SpecSet. For indexes we get SpecSets via AccessMethod. To get access for AccessMethod, you need relation C- structure. To get relation structure you need open relation with NoLock mode. But if I do it, I get Assert in relation_open. (There were no such Assert when I started this work, it appeared later). All code related to this issue is marked with FIXME comments. I've commented that Assert out, but not quite sure I was right. Here I need a help of more experienced members of community. This quite a big patch. Unfortunately it can't be split into smaller parts. I also suggest to consider this patch as an implementation of general idea, that works well. I have ideas in mind to make it better, but it will be infinite improvement process that will never lead to final commit. So if the concept is good and implementation is good enough, I suggest to commit it, and make it better later on by smaller patches over it. If it is not good enough, let me know, I will try to make it good. -- Nikolay Shaplov aka Nataraj Fuzzing Engineer at Postgres Professional Matrix IM: @dhyan:nataraj.su
Attachment
pgsql-hackers by date: