Date: Mon, 10 Sep 2018 18:02:10 +0700
From: Aleksandr Parfenov <a.parfenov@postgrespro.ru>
To: Nikolay Shaplov <dhyan@nataraj.su>
Cc: Nikita Glukhov <n.gluhov@postgrespro.ru>, Alvaro Herrera
 <alvherre@alvh.no-ip.org>, PostgreSQL mailing lists
 <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH][PROPOSAL] Add enum releation option type
Message-ID: <20180910180210.579b8303@asp437-ThinkPad-L380>
In-Reply-To: <3831139.TYuODbpbJx@x200m>
References: <20180215213518.5btgsjhbjxlolife@alvherre.pgsql>
	<68976929-aeeb-336d-869c-94e53c9b1c87@postgrespro.ru>
	<3831139.TYuODbpbJx@x200m>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Nikolay,

I did a quick look at yout patch and have some questions/points to
discuss. I like the idea of the patch and think that enum reloptions
can be useful. Especially for some frequently checked values, as it was
mentioned before.

There are few typos in comments, like 'validateing'.

I have two questions about naming of variables/structures:

1) variable opt_enum in parse_one_reloption named in different way
other similar variables named (without underscore).

2) enum gist_option_buffering_numeric_values/gist_option_buffering_value_numbers.
Firstly, it has two names. Secondly, can we name it
gist_option_buffering, why not?

As you mentioned in previous mail, you prefer to keep enum and
relopt_enum_element_definition array in the same .h file. I'm not sure,
but I think it is done to keep everything related to enum in one place
to avoid inconsistency in case of changes in some part (e.g. change of
enum without change of array). On the other hand, array content created
without array creation itself in .h file. Can we move actual array
creation into same .h file? What is the point to separate array content
definition and array definition?

-- 
Aleksandr Parfenov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company