pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Nikolay Shaplov <dhyan@nataraj.su>
To: pgsql-hackers@postgresql.org
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Subject: Re: [PATCH] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind for custom AM
Date: Tue, 28 Mar 2017 21:50:31 +0300
Message-ID: <2702961.B9hHnMG2c2@x200m> (raw)
In-Reply-To: <20170326180212.6kdmzc6i2icsax6s@alvherre.pgsql>
References: <20170326180212.6kdmzc6i2icsax6s@alvherre.pgsql>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>

В письме от 26 марта 2017 15:02:12 пользователь Alvaro Herrera написал:
> Nikolay Shaplov wrote:
> > If I would think about it now: we always know how many options we will
> > have. So we can just pass this number to palloc and assert if somebody
> > adds more options then expected... What do yo think about it.
> 
> I think we need to preserve the ability to add custom options, because
> extensions may want to do that.

I've been thinking about it for a while... I think this might lead to reducing 
the quality of the code...

For example: There was 10 options for some relation type. I decided to add one 
more option, but i did not ++ the number of options for 
allocateOptionsCatalog. So space for 10 options were allocated, and then when 
11th option is added, optionCatalogAddItem would allocate space for ten more 
options, and nine of them would not be used. And nobody will notice it.

So, I see here four solutions:

0. Leave it as it was. (We both do not like it)
1. Forbid dynamic number of options (you do not like it)
2. Allow dynamic number of options only for special cases, and in all other 
cases make them strict, and asserting if option number is wrong. This can be 
done in two ways:
2a. Add strict boolean flag, that tells if allow to add more options or not
2b. Do not add any flags, but if number of items is specified, then process 
number of items in strict mode. If it is set to -1, then do as it is done now, 
totally dynamically.

I would prefer 2b, if you sure that somebody will need dynamic number of 
options.

PS. I hardly believe that there can be dynamic number of options, as this 
options later are mapped into C-structure that is quite static. No use case 
comes into my mind, where I would like to have dynamic number of options, not 
knowing at build time, how many of them there would be.



-- 
Nikolay Shaplov, independent Perl & C/C++ developer. Available for hire.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



view thread (46+ messages)  latest in thread

Message-ID: <2702961.B9hHnMG2c2@x200m>
Permalink:  ../2702961.B9hHnMG2c2@x200m/
Also on:    postgresql.org/message-id/2702961.B9hHnMG2c2@x200m

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: dhyan@nataraj.su, alvherre@2ndquadrant.com
  Subject: Re: [PATCH] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind for custom AM
  In-Reply-To: <2702961.B9hHnMG2c2@x200m>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox