pg_settings.enumval as array

Magnus Hagander <magnus@hagander.net> writes:

The attached patch changes pg_settings.enumval to be an array of text
instead of just a string, per previous discussion and the open items list.

Comments?

Hmmm ... this coding will fail if any enumval contains a double quote.
Which is probably not a big problem, but we ought to document the
restriction somewhere.

Also, this:

! if (len > strlen(separator)-1)
! /* Replace final separator */
! hintmsg[len-strlen(separator)] = '\0';

would read better IMHO as "if (len >= strlen(separator))".

Also, the output datatype should be text[] not cstring[].

Otherwise seems okay.

regards, tom lane

Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

The attached patch changes pg_settings.enumval to be an array of text
instead of just a string, per previous discussion and the open items list.

Comments?

Hmmm ... this coding will fail if any enumval contains a double quote.
Which is probably not a big problem, but we ought to document the
restriction somewhere.

Hmm. Know what, I had a code comment about that in there. Then I
re-factored my patch and lost it!

I think a code comment is enough. It would actually be quite silly to
*have* a double quote in an enumval - I doubt it will happen by misake.

Will add back.

Also, this:

! if (len > strlen(separator)-1)
! /* Replace final separator */
! hintmsg[len-strlen(separator)] = '\0';

would read better IMHO as "if (len >= strlen(separator))".

Pfft, result of too much copy/paste. Changed.

Also, the output datatype should be text[] not cstring[].

Ok, will change. That requires hardcode of 1009? Or should I a #define
to pg_type.h?

//Magnus

Magnus Hagander <magnus@hagander.net> writes:

Tom Lane wrote:

Also, the output datatype should be text[] not cstring[].

Ok, will change. That requires hardcode of 1009? Or should I a #define
to pg_type.h?

Add a #define (bit surprising we didn't have one already)

regards, tom lane

Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

Tom Lane wrote:

Also, the output datatype should be text[] not cstring[].

Ok, will change. That requires hardcode of 1009? Or should I a #define
to pg_type.h?

Add a #define (bit surprising we didn't have one already)

Yeah, that surprised me too - which is why I had to ask (in case there
was some strange reason we didn't want it).

Changed and applied. Thanks for your comments!

//Magnus