pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Nikolay Shaplov <dhyan@nataraj.su>
To: pgsql-hackers@lists.postgresql.org
Cc: Alvaro Herrera from 2ndQuadrant <alvherre@postgresql.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Nikita Glukhov <n.gluhov@postgrespro.ru>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH][PROPOSAL] Add enum releation option type
Date: Fri, 13 Sep 2019 10:16:30 +0300
Message-ID: <2049090.vyOS18DeL7@x200m> (raw)
In-Reply-To: <20190905154227.GA5287@alvherre.pgsql>
References: <20190905154227.GA5287@alvherre.pgsql>

В письме от четверг, 5 сентября 2019 г. 11:42:27 MSK пользователь Alvaro 
Herrera from 2ndQuadrant написал:
> After looking closer once again, I don't like this patch.
> 
> I think the FOOBAR_ENUM_DEF defines serve no purpose, other than
> source-code placement next to the enum value definitions.  I think for
> example check_option, living in reloptions.c, should look like this:

This sounds as a good idea, I tried it, but did not succeed.

When I do as you suggest I get a bunch of warnings, and more over it, tests do 
not pass afterwards.

reloptions.c:447:3: warning: braces around scalar initializer
   {
   ^
reloptions.c:447:3: warning: (near initialization for 
‘enumRelOpts[0].members’)
reloptions.c:448:4: warning: braces around scalar initializer
    { "on",  GIST_OPTION_BUFFERING_ON },
    ^
reloptions.c:448:4: warning: (near initialization for 
‘enumRelOpts[0].members’)
reloptions.c:448:4: warning: initialization from incompatible pointer type
reloptions.c:448:4: warning: (near initialization for 
‘enumRelOpts[0].members’)

and so on, see full warning list attached.

I've played with it around, and did some googling, but without much success.
If we are moving this way (an this way seems to be good one), I need you help, 
because this thing is beyond my C knowledge, I will not manage it myself.

I also attached a diff of what I have done to get these warnings. It should be 
applied to latest version of patch we are discussing.
reloptions.c:447:3: warning: braces around scalar initializer
   {
   ^
reloptions.c:447:3: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:448:4: warning: braces around scalar initializer
    {"on",  GIST_OPTION_BUFFERING_ON },
    ^
reloptions.c:448:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:448:4: warning: initialization from incompatible pointer type
reloptions.c:448:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:448:4: warning: excess elements in scalar initializer
reloptions.c:448:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:449:4: warning: braces around scalar initializer
    {"off",  GIST_OPTION_BUFFERING_OFF },
    ^
reloptions.c:449:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:449:4: warning: initialization from incompatible pointer type
reloptions.c:449:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:449:4: warning: excess elements in scalar initializer
reloptions.c:449:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:449:4: warning: excess elements in scalar initializer
reloptions.c:449:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:450:4: warning: braces around scalar initializer
    {"auto", GIST_OPTION_BUFFERING_AUTO },
    ^
reloptions.c:450:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:450:4: warning: initialization from incompatible pointer type
reloptions.c:450:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:450:4: warning: excess elements in scalar initializer
reloptions.c:450:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:450:4: warning: excess elements in scalar initializer
reloptions.c:450:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:451:4: warning: braces around scalar initializer
    { NULL }
    ^
reloptions.c:451:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:451:4: warning: excess elements in scalar initializer
reloptions.c:451:4: warning: (near initialization for ‘enumRelOpts[0].members’)

Attachments:

  [text/plain] full-warning-list.txt (2.2K, ../2049090.vyOS18DeL7@x200m/2-full-warning-list.txt)
  download | inline:
reloptions.c:447:3: warning: braces around scalar initializer
   {
   ^
reloptions.c:447:3: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:448:4: warning: braces around scalar initializer
    {"on",  GIST_OPTION_BUFFERING_ON },
    ^
reloptions.c:448:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:448:4: warning: initialization from incompatible pointer type
reloptions.c:448:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:448:4: warning: excess elements in scalar initializer
reloptions.c:448:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:449:4: warning: braces around scalar initializer
    {"off",  GIST_OPTION_BUFFERING_OFF },
    ^
reloptions.c:449:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:449:4: warning: initialization from incompatible pointer type
reloptions.c:449:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:449:4: warning: excess elements in scalar initializer
reloptions.c:449:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:449:4: warning: excess elements in scalar initializer
reloptions.c:449:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:450:4: warning: braces around scalar initializer
    {"auto", GIST_OPTION_BUFFERING_AUTO },
    ^
reloptions.c:450:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:450:4: warning: initialization from incompatible pointer type
reloptions.c:450:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:450:4: warning: excess elements in scalar initializer
reloptions.c:450:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:450:4: warning: excess elements in scalar initializer
reloptions.c:450:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:451:4: warning: braces around scalar initializer
    { NULL }
    ^
reloptions.c:451:4: warning: (near initialization for ‘enumRelOpts[0].members’)
reloptions.c:451:4: warning: excess elements in scalar initializer
reloptions.c:451:4: warning: (near initialization for ‘enumRelOpts[0].members’)

  [text/x-patch] enum-reloptions.not-a-commitfest-patch (2.0K, ../2049090.vyOS18DeL7@x200m/3-enum-reloptions.not-a-commitfest-patch)
  download | inline diff:
diff --git a/.gitignore b/.gitignore
index 794e35b..37331c2 100644
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index fffab3a..fcf4766 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -433,8 +433,6 @@ static relopt_real realRelOpts[] =
 	{{NULL}}
 };
 
-static relopt_enum_elt_def gist_buffering_enum_def[] =
-GIST_OPTION_BUFFERING_ENUM_DEF;
 static relopt_enum_elt_def view_check_option_enum_def[] =
 VIEW_OPTION_CHECK_OPTION_ENUM_DEF;
 static relopt_enum enumRelOpts[] =
@@ -446,8 +444,13 @@ static relopt_enum enumRelOpts[] =
 				RELOPT_KIND_GIST,
 				AccessExclusiveLock
 		},
-			gist_buffering_enum_def,
-			GIST_OPTION_BUFFERING_AUTO
+		{
+			{"on",		GIST_OPTION_BUFFERING_ON },
+			{"off",		GIST_OPTION_BUFFERING_OFF },
+			{"auto",	GIST_OPTION_BUFFERING_AUTO },
+			{ NULL }
+		},
+		GIST_OPTION_BUFFERING_AUTO
 	},
 	{
 		{
diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h
index d586b04..047490a 100644
--- a/src/include/access/gist_private.h
+++ b/src/include/access/gist_private.h
@@ -379,24 +379,15 @@ typedef struct GISTBuildBuffers
 } GISTBuildBuffers;
 
 /*
- * Buffering is an enum option
- * gist_option_buffering_numeric_values defines a numeric representation of
- * option values, and GIST_OPTION_BUFFERING_ENUM_DEF defines enum string values
- * and maps them to numeric one.
+ * gist_option_buffering_values defines values for buffering enum reloption
  */
-typedef enum gist_option_buffering_numeric_values
+typedef enum gist_option_buffering_values
 {
 	GIST_OPTION_BUFFERING_ON = 0,
 	GIST_OPTION_BUFFERING_OFF = 1,
 	GIST_OPTION_BUFFERING_AUTO = 2,
-}			gist_option_buffering_numeric_values;
-
-#define GIST_OPTION_BUFFERING_ENUM_DEF { 	\
-	{ "on",		GIST_OPTION_BUFFERING_ON },		\
-	{ "off",	GIST_OPTION_BUFFERING_OFF },	\
-	{ "auto",	GIST_OPTION_BUFFERING_AUTO },	\
-	{ (const char *) NULL, 0 }					\
-}
+} gist_option_buffering_numeric_values;
+
 
 /*
  * Storage type for GiST's reloptions

view thread (40+ messages)  latest in thread

Message-ID: <2049090.vyOS18DeL7@x200m>
Permalink:  ../2049090.vyOS18DeL7@x200m/
Also on:    postgresql.org/message-id/2049090.vyOS18DeL7@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, pgsql-hackers@lists.postgresql.org, alvherre@postgresql.org, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, n.gluhov@postgrespro.ru
  Subject: Re: [PATCH][PROPOSAL] Add enum releation option type
  In-Reply-To: <2049090.vyOS18DeL7@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