Questions about parsing boolean and casting to anyelement
The pg_autovacuum system catalog will be deprecated in 8.4,
but my customers use them to control autovacuum to emulate
maintenance window. So, I'm trying to re-implement the catalog
using a VIEW and RULEs in 8.4.
The attached is a WIP script, but I have some questions around it:
(XXX: I don't mean to propose the script in the core.)
- Postgres interprets 'on' as true and 'off' as false in configuration
parameters, but they are not accepted in sql-boolean.
Is it a design? or should we add a parser for 'on' and 'off' ?
I'd like to allow 'on' and 'off' in sql-boolean, too.
- The input strings are stored as-is in pg_class.reloptions.
So, mixed values could be shown in reloptions. For example
autovacuum_enabled=0/1/on/off/true/false .
Should we canonicalize them? However, I think the current behavior
is not so bad because it can preserve user inputs.
- Are there any limitations in casting to anyelement?
I got an error when I define the 3rd argument of array_find()
as anyelement:
ERROR: UNION types text and integer cannot be matched
Even if I use casts, it seems to be ignored.
CREATE FUNCTION array_find(text[], text, anyelement)
RETURNS anyelement AS
$$
SELECT substring(i from E'\\W*=(.*)')::anyelement
FROM unnest($1) AS t(i) WHERE i LIKE $2 || '=%'
UNION ALL SELECT $3 LIMIT 1
$$
LANGUAGE sql IMMUTABLE STRICT;
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
Attachments:
ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
- Are there any limitations in casting to anyelement?
It's a no-op ... probably we shouldn't even let you do it, if the
lack of an error leaves room for such misinterpretation as this.
anyelement and friends are placeholders for use in function
declarations, not real types that it makes sense to cast to.
CREATE FUNCTION array_find(text[], text, anyelement)
RETURNS anyelement AS
$$
SELECT substring(i from E'\\W*=(.*)')::anyelement
FROM unnest($1) AS t(i) WHERE i LIKE $2 || '=%'
UNION ALL SELECT $3 LIMIT 1
$$
LANGUAGE sql IMMUTABLE STRICT;
The substring() necessarily produces type text, so I dunno why you
think $3 needs to be anything but text.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> wrote:
ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
- Are there any limitations in casting to anyelement?
It's a no-op ... probably we shouldn't even let you do it, if the
lack of an error leaves room for such misinterpretation as this.
anyelement and friends are placeholders for use in function
declarations, not real types that it makes sense to cast to.
I hope anyelement could be used in cast because casts are supported by
almost programming languages where template or generics are available.
Moreover, we can cast to anyelement if we use C functions; using oid of
anyelement in runtime and querying an associated cast function from
system catalog.
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
Tom Lane <tgl@sss.pgh.pa.us> wrote:
ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
- Are there any limitations in casting to anyelement?
It's a no-op ... probably we shouldn't even let you do it, if the
lack of an error leaves room for such misinterpretation as this.
anyelement and friends are placeholders for use in function
declarations, not real types that it makes sense to cast to.
I hope anyelement could be used in cast because casts are supported by
almost programming languages where template or generics are available.
I think what you're suggesting is that inside a polymorphic function,
anyelement would somehow be a macro for the type that the function's
current anyelement parameter(s) have. It's an interesting idea but
it's just fantasy at the moment; I don't even have an idea of how we
might implement that.
In the meantime I'm more convinced than ever that we should throw an
error for attempting such a cast. If people are imagining that it will
do something like that, we need to disillusion them.
regards, tom lane
ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> wrote:
- Postgres interprets 'on' as true and 'off' as false in configuration
parameters, but they are not accepted in sql-boolean.
Is it a design? or should we add a parser for 'on' and 'off' ?
I'd like to allow 'on' and 'off' in sql-boolean, too.
Here is a patch to allow 'on' and 'off' as input texts for boolean.
Duplicated boolean parsers in parse_bool() and boolin() are merged
into a new parse_bool_with_len().
I think the change is useful when we treat reloptions in programs.
Since human-readable texts are not suitable for programs, we would need
conversions from text to boolean. Then the shared parser works well.
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
Attachments:
boolin_accepts_onoff.patchapplication/octet-stream; name=boolin_accepts_onoff.patchDownload
diff -cpr head/src/backend/utils/adt/bool.c boolin_accepts_onoff/src/backend/utils/adt/bool.c
*** head/src/backend/utils/adt/bool.c Mon Jan 5 00:22:25 2009
--- boolin_accepts_onoff/src/backend/utils/adt/bool.c Tue Feb 17 15:06:03 2009
***************
*** 20,25 ****
--- 20,28 ----
#include "libpq/pqformat.h"
#include "utils/builtins.h"
+ /* FIXME: Should include guc.h or move the prototype to another place? */
+ extern bool parse_bool_with_len(const char *value, size_t len, bool *result);
+
/*****************************************************************************
* USER I/O ROUTINES *
*****************************************************************************/
***************
*** 27,34 ****
/*
* boolin - converts "t" or "f" to 1 or 0
*
! * Check explicitly for "true/false" and TRUE/FALSE, 1/0, YES/NO.
! * Reject other values. - thomas 1997-10-05
*
* In the switch statement, check the most-used possibilities first.
*/
--- 30,37 ----
/*
* boolin - converts "t" or "f" to 1 or 0
*
! * Check explicitly for "true/false" and TRUE/FALSE, 1/0, YES/NO, ON/OFF.
! * Reject other values.
*
* In the switch statement, check the most-used possibilities first.
*/
*************** boolin(PG_FUNCTION_ARGS)
*** 38,43 ****
--- 41,47 ----
const char *in_str = PG_GETARG_CSTRING(0);
const char *str;
size_t len;
+ bool result;
/*
* Skip leading and trailing whitespace
*************** boolin(PG_FUNCTION_ARGS)
*** 50,94 ****
while (len > 0 && isspace((unsigned char) str[len - 1]))
len--;
! switch (*str)
! {
! case 't':
! case 'T':
! if (pg_strncasecmp(str, "true", len) == 0)
! PG_RETURN_BOOL(true);
! break;
!
! case 'f':
! case 'F':
! if (pg_strncasecmp(str, "false", len) == 0)
! PG_RETURN_BOOL(false);
! break;
!
! case 'y':
! case 'Y':
! if (pg_strncasecmp(str, "yes", len) == 0)
! PG_RETURN_BOOL(true);
! break;
!
! case '1':
! if (pg_strncasecmp(str, "1", len) == 0)
! PG_RETURN_BOOL(true);
! break;
!
! case 'n':
! case 'N':
! if (pg_strncasecmp(str, "no", len) == 0)
! PG_RETURN_BOOL(false);
! break;
!
! case '0':
! if (pg_strncasecmp(str, "0", len) == 0)
! PG_RETURN_BOOL(false);
! break;
!
! default:
! break;
! }
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
--- 54,61 ----
while (len > 0 && isspace((unsigned char) str[len - 1]))
len--;
! if (parse_bool_with_len(str, len, &result))
! PG_RETURN_BOOL(result);
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
diff -cpr head/src/backend/utils/misc/guc.c boolin_accepts_onoff/src/backend/utils/misc/guc.c
*** head/src/backend/utils/misc/guc.c Thu Jan 22 09:48:53 2009
--- boolin_accepts_onoff/src/backend/utils/misc/guc.c Tue Feb 17 15:06:03 2009
*************** ReportGUCOption(struct config_generic *
*** 4096,4159 ****
bool
parse_bool(const char *value, bool *result)
{
! size_t len = strlen(value);
! if (pg_strncasecmp(value, "true", len) == 0)
! {
! if (result)
! *result = true;
! }
! else if (pg_strncasecmp(value, "false", len) == 0)
! {
! if (result)
! *result = false;
! }
! else if (pg_strncasecmp(value, "yes", len) == 0)
! {
! if (result)
! *result = true;
! }
! else if (pg_strncasecmp(value, "no", len) == 0)
! {
! if (result)
! *result = false;
! }
! /* 'o' is not unique enough */
! else if (pg_strncasecmp(value, "on", (len > 2 ? len : 2)) == 0)
{
! if (result)
! *result = true;
! }
! else if (pg_strncasecmp(value, "off", (len > 2 ? len : 2)) == 0)
! {
! if (result)
! *result = false;
! }
! else if (pg_strcasecmp(value, "1") == 0)
! {
! if (result)
! *result = true;
! }
! else if (pg_strcasecmp(value, "0") == 0)
! {
! if (result)
! *result = false;
}
! else
! {
! if (result)
! *result = false; /* suppress compiler warning */
! return false;
! }
! return true;
}
-
-
/*
* Try to parse value as an integer. The accepted formats are the
* usual decimal, octal, or hexadecimal formats, optionally followed by
--- 4096,4187 ----
bool
parse_bool(const char *value, bool *result)
{
! bool ok;
! bool r;
! ok = parse_bool_with_len(value, strlen(value), &r);
! if (result)
! *result = r;
! return ok;
! }
! bool
! parse_bool_with_len(const char *value, size_t len, bool *result)
! {
! AssertArg(value != NULL);
! AssertArg(result != NULL);
! switch (*value)
{
! case 't':
! case 'T':
! if (pg_strncasecmp(value, "true", len) == 0)
! {
! *result = true;
! return true;
! }
! break;
! case 'f':
! case 'F':
! if (pg_strncasecmp(value, "false", len) == 0)
! {
! *result = false;
! return true;
! }
! break;
! case 'y':
! case 'Y':
! if (pg_strncasecmp(value, "yes", len) == 0)
! {
! *result = true;
! return true;
! }
! break;
! case 'n':
! case 'N':
! if (pg_strncasecmp(value, "no", len) == 0)
! {
! *result = false;
! return true;
! }
! break;
! case 'o':
! case 'O':
! /* 'o' is not unique enough */
! if (pg_strncasecmp(value, "on", (len > 2 ? len : 2)) == 0)
! {
! *result = true;
! return true;
! }
! else if (pg_strncasecmp(value, "off", (len > 2 ? len : 2)) == 0)
! {
! *result = false;
! return true;
! }
! break;
! case '1':
! if (pg_strncasecmp(value, "1", len) == 0)
! {
! *result = true;
! return true;
! }
! break;
! case '0':
! if (pg_strncasecmp(value, "0", len) == 0)
! {
! *result = false;
! return true;
! }
! break;
! default:
! break;
}
! *result = false; /* suppress compiler warning */
! return false;
}
/*
* Try to parse value as an integer. The accepted formats are the
* usual decimal, octal, or hexadecimal formats, optionally followed by
diff -cpr head/src/include/utils/guc.h boolin_accepts_onoff/src/include/utils/guc.h
*** head/src/include/utils/guc.h Mon Jan 5 00:22:25 2009
--- boolin_accepts_onoff/src/include/utils/guc.h Tue Feb 17 15:06:03 2009
*************** extern void AtEOXact_GUC(bool isCommit,
*** 258,263 ****
--- 258,264 ----
extern void BeginReportingGUCOptions(void);
extern void ParseLongOption(const char *string, char **name, char **value);
extern bool parse_bool(const char *value, bool *result);
+ extern bool parse_bool_with_len(const char *value, size_t len, bool *result);
extern bool parse_int(const char *value, int *result, int flags,
const char **hintmsg);
extern bool parse_real(const char *value, double *result);
On Mon, Feb 16, 2009 at 08:03:33PM -0500, Tom Lane wrote:
ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
I hope anyelement could be used in cast because casts are supported by
almost programming languages where template or generics are available.
Programming languages with "generics" (aka, parametric polymorphism in
literature) should mean that you need *less* casts because the type
system is expressive enough that you don't need to "escape" through a
cast.
I think what you're suggesting is that inside a polymorphic function,
anyelement would somehow be a macro for the type that the function's
current anyelement parameter(s) have. It's an interesting idea but
it's just fantasy at the moment; I don't even have an idea of how we
might implement that.
A couple of solutions would immediately present themselves; making
functions first class objects and introducing something called "type
classes" (please note these bear little resemblance to "classes" in
object orientated programming).
If functions were first class objects; you could pass in the "input"
function (i.e. boolin, or numeric_in) to the "array_find" function
directly call it in place of the "magic" cast syntax (magic because it
has to figure out the type of the LHS, whereas if it was a function with
known type then it wouldn't need to infer the source type).
Type classes[1]http://portal.acm.org/citation.cfm?id=75277.75283 is the original paper[2]http://portal.acm.org/citation.cfm?id=141536 extends them to have multiple type parameters, not for PG but nice to know it's been done before and isn't new ground are a general mechanism for making the "magic" above
tractable and sound. The cast above would be exactly analogous to the
"read" function in Haskell, and is used very regularly in most code.
In the meantime I'm more convinced than ever that we should throw an
error for attempting such a cast. If people are imagining that it will
do something like that, we need to disillusion them.
Yes, sounds sensible at the moment.
--
Sam http://samason.me.uk/
[1]: http://portal.acm.org/citation.cfm?id=75277.75283 is the original paper
is the original paper
[2]: http://portal.acm.org/citation.cfm?id=141536 extends them to have multiple type parameters, not for PG but nice to know it's been done before and isn't new ground
extends them to have multiple type parameters, not for PG but nice
to know it's been done before and isn't new ground
I wrote:
ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
I hope anyelement could be used in cast because casts are supported by
almost programming languages where template or generics are available.
I think what you're suggesting is that inside a polymorphic function,
anyelement would somehow be a macro for the type that the function's
current anyelement parameter(s) have. It's an interesting idea but
it's just fantasy at the moment; I don't even have an idea of how we
might implement that.
After thinking about it for awhile, I don't like the notation anyway
--- it's not immediately obvious that a cast to anyelement should mean
something like that. What seems more sensible to me is to introduce
a function to get the type of an expression, so that you could write
something likecast(expression as typeof(expression))
This special function would act like C's sizeof and similar constructs
in that its argument would never be evaluated, only inspected at parse
time to determine its type. (There are already precedents for this in
SQL; see the IS OF construct.) So the original requirement would be
met with something like "expression::typeof($1)".
A small disadvantage of this approach is that it's notationally a bit
uglier for anyelement/anyarray pairs. For example, consider a function
"foo(anyelement) returns anyarray". To get at the element type you just
say typeof($1), but if you have to name the array type you need a hack
like typeof(array[$1]). In the other direction (name the element type
of a parameter array) something like typeof($1[1]) would work.
The countervailing advantage is that this solves a lot of problems that
overloading anyelement wouldn't ever solve, since you can get at the
type of any expression not just a bare parameter.
Also I think it'd be relatively easy to stick into the parser; it
wouldn't require introduction of any new parse-time context information.
Anyway, none of this is material for 8.4, just a possible TODO item.
regards, tom lane
On Wed, Feb 18, 2009 at 2:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
After thinking about it for awhile, I don't like the notation anyway --- it's not immediately obvious that a cast to anyelement should mean something like that. What seems more sensible to me is to introduce a function to get the type of an expression, so that you could write something like
We already have such a function, pg_typeof(). I submitted a patch for
it in the November commitfest, and you committed it. [1]http://git.postgresql.org/?p=postgresql.git;a=commit;h=1a850edf036a1c7dbb9f4fcfeae1e5f2c68cf049
Or is that not the sort of function you were thinking of?
Cheers,
BJ
[1]: http://git.postgresql.org/?p=postgresql.git;a=commit;h=1a850edf036a1c7dbb9f4fcfeae1e5f2c68cf049
Brendan Jurd <direvus@gmail.com> writes:
On Wed, Feb 18, 2009 at 2:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
After thinking about it for awhile, I don't like the notation anyway --- it's not immediately obvious that a cast to anyelement should mean something like that. What seems more sensible to me is to introduce a function to get the type of an expression, so that you could write something like
We already have such a function, pg_typeof().
No, pg_typeof is a more-or-less ordinary function that delivers an OID
at runtime. What we need here is something that will work as a CAST
target, ie, it has to be treated as a type name at parse time.
regards, tom lane
2009/2/17 Tom Lane <tgl@sss.pgh.pa.us>:
I wrote:
ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
I hope anyelement could be used in cast because casts are supported by
almost programming languages where template or generics are available.I think what you're suggesting is that inside a polymorphic function,
anyelement would somehow be a macro for the type that the function's
current anyelement parameter(s) have. It's an interesting idea but
it's just fantasy at the moment; I don't even have an idea of how we
might implement that.After thinking about it for awhile, I don't like the notation anyway --- it's not immediately obvious that a cast to anyelement should mean something like that. What seems more sensible to me is to introduce a function to get the type of an expression, so that you could write something likecast(expression as typeof(expression))
This special function would act like C's sizeof and similar constructs
in that its argument would never be evaluated, only inspected at parse
time to determine its type. (There are already precedents for this in
SQL; see the IS OF construct.) So the original requirement would be
met with something like "expression::typeof($1)".A small disadvantage of this approach is that it's notationally a bit
uglier for anyelement/anyarray pairs. For example, consider a function
"foo(anyelement) returns anyarray". To get at the element type you just
say typeof($1), but if you have to name the array type you need a hack
like typeof(array[$1]). In the other direction (name the element type
of a parameter array) something like typeof($1[1]) would work.The countervailing advantage is that this solves a lot of problems that
overloading anyelement wouldn't ever solve, since you can get at the
type of any expression not just a bare parameter.Also I think it'd be relatively easy to stick into the parser; it
wouldn't require introduction of any new parse-time context information.Anyway, none of this is material for 8.4, just a possible TODO item.
it's look like good idea
regards
Pavel Stehule
Show quoted text
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I wrote:
In the meantime I'm more convinced than ever that we should throw an
error for attempting such a cast. If people are imagining that it will
do something like that, we need to disillusion them.
BTW, I wrote up what I thought was a trivial patch to make this happen,
and promptly got a regression test failure:
CREATE TABLE enumtest_child (parent rainbow REFERENCES enumtest_parent);
INSERT INTO enumtest_parent VALUES ('red');
INSERT INTO enumtest_child VALUES ('red');
+ ERROR: casting to a polymorphic type such as anyenum is meaningless
+ LINE 1: ... FROM ONLY "public"."enumtest_parent" x WHERE "id"::pg_catal...
+ ^
+ QUERY: SELECT 1 FROM ONLY "public"."enumtest_parent" x WHERE "id"::pg_catalog.anyenum OPERATOR(pg_catalog.=) $1::pg_catalog.anyenum FOR SHARE OF x
INSERT INTO enumtest_child VALUES ('blue'); -- failWhat is happening is that the code to generate RI check queries is
blindly casting to the declared input type of the operator it's
selected, which here is "anyenum = anyenum". We could easily prevent
it from doing that for polymorphic input types; but since I tripped over
this case almost immediately, I'm wondering what other cases might be
out there that would get broken by throwing this error.
Seeing that this type of confusion hasn't come up before, I think it
might be better to leave things alone here.
regards, tom lane
ITAGAKI Takahiro wrote:
Here is a patch to allow 'on' and 'off' as input texts for boolean.
Duplicated boolean parsers in parse_bool() and boolin() are merged
into a new parse_bool_with_len().
Regarding your FIXME comment, I think parse_bool* should be in bool.c
and declared in builtins.h, which guc.c already includes.
(Conceptually, the valid format of a bool should be drived by the
boolean type, not the GUC system, I think.)
Peter Eisentraut <peter_e@gmx.net> wrote:
ITAGAKI Takahiro wrote:
Here is a patch to allow 'on' and 'off' as input texts for boolean.
Regarding your FIXME comment, I think parse_bool* should be in bool.c
and declared in builtins.h, which guc.c already includes.
(Conceptually, the valid format of a bool should be drived by the
boolean type, not the GUC system, I think.)
Here is an updated patch to move parse_bool* into bool.c.
I also added tests of on/off values to the regression test.
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
Attachments:
boolin_accepts_onoff.2.patchapplication/octet-stream; name=boolin_accepts_onoff.2.patchDownload
diff -cpr head/src/backend/utils/adt/bool.c boolin_accepts_onoff/src/backend/utils/adt/bool.c
*** head/src/backend/utils/adt/bool.c Mon Jan 5 00:22:25 2009
--- boolin_accepts_onoff/src/backend/utils/adt/bool.c Fri Feb 20 17:46:15 2009
***************
*** 20,25 ****
--- 20,118 ----
#include "libpq/pqformat.h"
#include "utils/builtins.h"
+ /*
+ * Try to interpret value as boolean value. Valid values are: true,
+ * false, yes, no, on, off, 1, 0; as well as unique prefixes thereof.
+ * If the string parses okay, return true, else false.
+ * If okay and result is not NULL, return the value in *result.
+ */
+ bool
+ parse_bool(const char *value, bool *result)
+ {
+ return parse_bool_with_len(value, strlen(value), result);
+ }
+
+ bool
+ parse_bool_with_len(const char *value, size_t len, bool *result)
+ {
+ switch (*value)
+ {
+ case 't':
+ case 'T':
+ if (pg_strncasecmp(value, "true", len) == 0)
+ {
+ if (result)
+ *result = true;
+ return true;
+ }
+ break;
+ case 'f':
+ case 'F':
+ if (pg_strncasecmp(value, "false", len) == 0)
+ {
+ if (result)
+ *result = false;
+ return true;
+ }
+ break;
+ case 'y':
+ case 'Y':
+ if (pg_strncasecmp(value, "yes", len) == 0)
+ {
+ if (result)
+ *result = true;
+ return true;
+ }
+ break;
+ case 'n':
+ case 'N':
+ if (pg_strncasecmp(value, "no", len) == 0)
+ {
+ if (result)
+ *result = false;
+ return true;
+ }
+ break;
+ case 'o':
+ case 'O':
+ /* 'o' is not unique enough */
+ if (pg_strncasecmp(value, "on", (len > 2 ? len : 2)) == 0)
+ {
+ if (result)
+ *result = true;
+ return true;
+ }
+ else if (pg_strncasecmp(value, "off", (len > 2 ? len : 2)) == 0)
+ {
+ if (result)
+ *result = false;
+ return true;
+ }
+ break;
+ case '1':
+ if (len == 1)
+ {
+ if (result)
+ *result = true;
+ return true;
+ }
+ break;
+ case '0':
+ if (len == 1)
+ {
+ if (result)
+ *result = false;
+ return true;
+ }
+ break;
+ default:
+ break;
+ }
+
+ *result = false; /* suppress compiler warning */
+ return false;
+ }
+
/*****************************************************************************
* USER I/O ROUTINES *
*****************************************************************************/
***************
*** 27,34 ****
/*
* boolin - converts "t" or "f" to 1 or 0
*
! * Check explicitly for "true/false" and TRUE/FALSE, 1/0, YES/NO.
! * Reject other values. - thomas 1997-10-05
*
* In the switch statement, check the most-used possibilities first.
*/
--- 120,127 ----
/*
* boolin - converts "t" or "f" to 1 or 0
*
! * Check explicitly for "true/false" and TRUE/FALSE, 1/0, YES/NO, ON/OFF.
! * Reject other values.
*
* In the switch statement, check the most-used possibilities first.
*/
*************** boolin(PG_FUNCTION_ARGS)
*** 38,43 ****
--- 131,137 ----
const char *in_str = PG_GETARG_CSTRING(0);
const char *str;
size_t len;
+ bool result;
/*
* Skip leading and trailing whitespace
*************** boolin(PG_FUNCTION_ARGS)
*** 50,94 ****
while (len > 0 && isspace((unsigned char) str[len - 1]))
len--;
! switch (*str)
! {
! case 't':
! case 'T':
! if (pg_strncasecmp(str, "true", len) == 0)
! PG_RETURN_BOOL(true);
! break;
!
! case 'f':
! case 'F':
! if (pg_strncasecmp(str, "false", len) == 0)
! PG_RETURN_BOOL(false);
! break;
!
! case 'y':
! case 'Y':
! if (pg_strncasecmp(str, "yes", len) == 0)
! PG_RETURN_BOOL(true);
! break;
!
! case '1':
! if (pg_strncasecmp(str, "1", len) == 0)
! PG_RETURN_BOOL(true);
! break;
!
! case 'n':
! case 'N':
! if (pg_strncasecmp(str, "no", len) == 0)
! PG_RETURN_BOOL(false);
! break;
!
! case '0':
! if (pg_strncasecmp(str, "0", len) == 0)
! PG_RETURN_BOOL(false);
! break;
!
! default:
! break;
! }
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
--- 144,151 ----
while (len > 0 && isspace((unsigned char) str[len - 1]))
len--;
! if (parse_bool_with_len(str, len, &result))
! PG_RETURN_BOOL(result);
ereport(ERROR,
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
diff -cpr head/src/backend/utils/misc/guc.c boolin_accepts_onoff/src/backend/utils/misc/guc.c
*** head/src/backend/utils/misc/guc.c Thu Jan 22 09:48:53 2009
--- boolin_accepts_onoff/src/backend/utils/misc/guc.c Fri Feb 20 17:46:15 2009
*************** ReportGUCOption(struct config_generic *
*** 4086,4159 ****
}
}
-
- /*
- * Try to interpret value as boolean value. Valid values are: true,
- * false, yes, no, on, off, 1, 0; as well as unique prefixes thereof.
- * If the string parses okay, return true, else false.
- * If okay and result is not NULL, return the value in *result.
- */
- bool
- parse_bool(const char *value, bool *result)
- {
- size_t len = strlen(value);
-
- if (pg_strncasecmp(value, "true", len) == 0)
- {
- if (result)
- *result = true;
- }
- else if (pg_strncasecmp(value, "false", len) == 0)
- {
- if (result)
- *result = false;
- }
-
- else if (pg_strncasecmp(value, "yes", len) == 0)
- {
- if (result)
- *result = true;
- }
- else if (pg_strncasecmp(value, "no", len) == 0)
- {
- if (result)
- *result = false;
- }
-
- /* 'o' is not unique enough */
- else if (pg_strncasecmp(value, "on", (len > 2 ? len : 2)) == 0)
- {
- if (result)
- *result = true;
- }
- else if (pg_strncasecmp(value, "off", (len > 2 ? len : 2)) == 0)
- {
- if (result)
- *result = false;
- }
-
- else if (pg_strcasecmp(value, "1") == 0)
- {
- if (result)
- *result = true;
- }
- else if (pg_strcasecmp(value, "0") == 0)
- {
- if (result)
- *result = false;
- }
-
- else
- {
- if (result)
- *result = false; /* suppress compiler warning */
- return false;
- }
- return true;
- }
-
-
-
/*
* Try to parse value as an integer. The accepted formats are the
* usual decimal, octal, or hexadecimal formats, optionally followed by
--- 4086,4091 ----
diff -cpr head/src/include/utils/builtins.h boolin_accepts_onoff/src/include/utils/builtins.h
*** head/src/include/utils/builtins.h Mon Feb 9 09:25:51 2009
--- boolin_accepts_onoff/src/include/utils/builtins.h Fri Feb 20 17:46:15 2009
*************** extern Datum boolle(PG_FUNCTION_ARGS);
*** 109,114 ****
--- 109,116 ----
extern Datum boolge(PG_FUNCTION_ARGS);
extern Datum booland_statefunc(PG_FUNCTION_ARGS);
extern Datum boolor_statefunc(PG_FUNCTION_ARGS);
+ extern bool parse_bool(const char *value, bool *result);
+ extern bool parse_bool_with_len(const char *value, size_t len, bool *result);
/* char.c */
extern Datum charin(PG_FUNCTION_ARGS);
diff -cpr head/src/include/utils/guc.h boolin_accepts_onoff/src/include/utils/guc.h
*** head/src/include/utils/guc.h Mon Jan 5 00:22:25 2009
--- boolin_accepts_onoff/src/include/utils/guc.h Fri Feb 20 17:46:15 2009
*************** extern int NewGUCNestLevel(void);
*** 257,263 ****
extern void AtEOXact_GUC(bool isCommit, int nestLevel);
extern void BeginReportingGUCOptions(void);
extern void ParseLongOption(const char *string, char **name, char **value);
- extern bool parse_bool(const char *value, bool *result);
extern bool parse_int(const char *value, int *result, int flags,
const char **hintmsg);
extern bool parse_real(const char *value, double *result);
--- 257,262 ----
diff -cpr head/src/test/regress/expected/boolean.out boolin_accepts_onoff/src/test/regress/expected/boolean.out
*** head/src/test/regress/expected/boolean.out Tue Oct 7 09:40:59 2008
--- boolin_accepts_onoff/src/test/regress/expected/boolean.out Fri Feb 20 17:46:15 2009
*************** SELECT bool 'nay' AS error;
*** 88,93 ****
--- 88,123 ----
ERROR: invalid input syntax for type boolean: "nay"
LINE 1: SELECT bool 'nay' AS error;
^
+ SELECT bool 'on' AS true;
+ true
+ ------
+ t
+ (1 row)
+
+ SELECT bool 'off' AS false;
+ false
+ -------
+ f
+ (1 row)
+
+ SELECT bool 'of' AS false;
+ false
+ -------
+ f
+ (1 row)
+
+ SELECT bool 'o' AS error;
+ ERROR: invalid input syntax for type boolean: "o"
+ LINE 1: SELECT bool 'o' AS error;
+ ^
+ SELECT bool 'on_' AS error;
+ ERROR: invalid input syntax for type boolean: "on_"
+ LINE 1: SELECT bool 'on_' AS error;
+ ^
+ SELECT bool 'off_' AS error;
+ ERROR: invalid input syntax for type boolean: "off_"
+ LINE 1: SELECT bool 'off_' AS error;
+ ^
SELECT bool '1' AS true;
true
------
diff -cpr head/src/test/regress/sql/boolean.sql boolin_accepts_onoff/src/test/regress/sql/boolean.sql
*** head/src/test/regress/sql/boolean.sql Tue Oct 7 09:40:59 2008
--- boolin_accepts_onoff/src/test/regress/sql/boolean.sql Fri Feb 20 17:46:15 2009
*************** SELECT bool 'no' AS false;
*** 40,45 ****
--- 40,57 ----
SELECT bool 'nay' AS error;
+ SELECT bool 'on' AS true;
+
+ SELECT bool 'off' AS false;
+
+ SELECT bool 'of' AS false;
+
+ SELECT bool 'o' AS error;
+
+ SELECT bool 'on_' AS error;
+
+ SELECT bool 'off_' AS error;
+
SELECT bool '1' AS true;
SELECT bool '11' AS error;
ITAGAKI Takahiro wrote:
Peter Eisentraut <peter_e@gmx.net> wrote:
ITAGAKI Takahiro wrote:
Here is a patch to allow 'on' and 'off' as input texts for boolean.
Regarding your FIXME comment, I think parse_bool* should be in bool.c
and declared in builtins.h, which guc.c already includes.
(Conceptually, the valid format of a bool should be drived by the
boolean type, not the GUC system, I think.)Here is an updated patch to move parse_bool* into bool.c.
I also added tests of on/off values to the regression test.
applied