Suggestion: Unified options API. Need help from core team - Mailing list pgsql-hackers

Hi!

I am still hoping to finish my work on reloptions I've started some years ago.

I've renewed my patch and I think I need help from core team to finish it.

General idea of the patch: Now we have three ways to define options for 
different objects, with more or less different code used for it. It wold be 
better to have unified context independent API for processing options, instead.

Long story short:

There is Option Specification object, that has all information about single 
option, how it should be parsed and validated.

There is Option Specification Set object, an array of Option Specs, that defines 
all options available for certain object (am of some index for example).

When some object (relation, opclass,  etc) wants to have an options, it 
creates an Option Spec Set for there options, and uses it for converting 
options between different representations (to get is from SQL, to store it in 
pg_class, to pass it to the core code as bytea etc)

For indexes Option Spec Set is available via Access Method API. 

For non-index relations all Option Spec Sets are left in reloption.c file, and 
should be moved to heap AM later. (They are not in AM now so will not change 
it now)

Main problem:

There are LockModes. LockModes for options is also stored in Option Spec Set. 
For indexes Option Spec Sec is accessable via AM. So to get LockMode for 
option of an index you need to have access for it's relation object (so you 
can call proper AM method to fetch spec set). So you need "Relation rel" in 
AlterTableGetRelOptionsLockLevel where Lock Level is determinated (src/
backend/access/common/reloptions.c)
AlterTableGetRelOptionsLockLevel is called from AlterTableGetLockLevel (src/
backend/commands/tablecmds.c) so we need "Relation rel" there too.
AlterTableGetLockLevel is called from AlterTableInternal (/src/backend/
commands/tablecmds.c) There we have "Oid relid" so we can try to open relation 
like this

                   Relation rel = relation_open(relid, NoLock);
                   cmd_lockmode = AlterTableGetRelOptionsLockLevel(rel,
                                                   castNode(List, cmd->def));
                   relation_close(rel,NoLock);
                   break;

but this will trigger the assertion 

   Assert(lockmode != NoLock ||
          IsBootstrapProcessingMode() ||
          CheckRelationLockedByMe(r, c, true));

in relation_open (b/src/backend/access/common/relation.c)

For now I've commented this assertion out. I've tried to open relation with 
AccessShareLock but this caused one test to fail, and I am not sure this 
solution is better.

What I have done here I consider a hack, so I need a help of core-team here to 
do it in right way.

General problems:

I guess I need a coauthor, or supervisor from core team, to finish this patch. 
The amount of code is big, and I guess there are parts that can be made more 
in postgres way, then I did them. And I would need an advice there, and I 
guess it would be better to do if before sending it to commitfest.


Current patch status:

1. It is Beta. Some minor issues and FIXMEs are not solved. Some code comments 
needs revising, but in general it do what it is intended to do.

2. This patch does not intend to change postgres behavior at all, all should 
work as before, all changes are internal only.

The only exception is error message for unexciting option name in toast 
namespace 

 CREATE TABLE reloptions_test2 (i int) WITH (toast.not_existing_option = 42);
-ERROR:  unrecognized parameter "not_existing_option"
+ERROR:  unrecognized parameter "toast.not_existing_option"

New message is better I guess, though I can change it back if needed.

3. I am doing my development in this blanch https://gitlab.com/dhyannataraj/
postgres/-/tree/new_options_take_two I am making changes every day, so last 
version will be available there

Would be glad to hear from coreteam before I finish with this patch and made it 
ready for commit-fest.