Thread: Casting INT4 to BOOL...

Is there any reason why the backend doesn't cast an unquoted integer to
a boolean value?

test=> SELECT 1::BOOL;
ERROR:  cannot cast type integer to boolean
test=> SELECT '1'::BOOL;
  bool
------
  t
(1 row)

Time: 2.478 ms

It doesn't add anything to the grammar and would make life peachy for
Qt users.  In Qt, to get around this, I end up doing one of two things:

QSqlQuery q(db);
q.prepare("SELECT '?'::BOOL");
q.bindValue(0, true);
q.exec();

Or I'll do something even more grotty like:

q.prepare("SELECT CASE ?::INT WHEN 0 THEN FALSE ELSE TRUE END");

One could argue that it's a bug in Qt in that it doesn't quote the
value sent to the backend like it does strings, but, since the backend
already handles unquoted integers, it seems like it'd be trivial to
setup a cast.  On installs that use my software, I have to work around
it with:

CREATE FUNCTION int4_to_bool(INT) RETURNS BOOL AS 'BEGIN IF $1 THEN
RETURN TRUE; ELSE RETURN FALSE; END IF; END;' LANGUAGE 'plpgsql'
IMMUTABLE;
CREATE CAST (INT AS BOOL) WITH FUNCTION int4_to_bool(INT) AS ASSIGNMENT;

But that's not something that is supported by default.  Is there any
reason this hasn't been added yet?  "if (1)" and "if (0)" are
conditional constructs that are accepted in every language that I can
think of (even MUMPs *hopes no one remembers/has to use that
language*).  Could someone apply the attached patch (the C version of
the above) if there are no glaring reasons not to?  It would make the
lives of Qt programmers *much* more pleasant.  Thanks in advance.  -sc

test=> SELECT 1::BOOL, 0::BOOL, 42::BOOL;
  bool | bool | bool
------+------+------
  t    | f    | t
(1 row)



--
Sean Chittenden

Sean Chittenden <chitt@speakeasy.net> writes:
> Is there any reason why the backend doesn't cast an unquoted integer to
> a boolean value?

Hidden cross-category typecasts are evil.  I'd accept this as an
explicit cast ('e' in pg_cast) but not automatic.

Also, what about the other direction?  Providing a cast in only one
direction is pretty inconsistent.

            regards, tom lane

Sean Chittenden wrote:
> Is there any reason why the backend doesn't cast an unquoted integer
> to a boolean value?

What is the reason that it should?  (Qt bugs aside...)

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

>> Is there any reason why the backend doesn't cast an unquoted integer
>> to
>> a boolean value?
>
> Hidden cross-category typecasts are evil.  I'd accept this as an
> explicit cast ('e' in pg_cast) but not automatic.

Alrighty.  Do you want an updated patch for the single character tweak
or can you futz with it before committing?  :)

> Also, what about the other direction?  Providing a cast in only one
> direction is pretty inconsistent.

Ah... well, if you want that, I can provide that too.  Personally, I
could care less about the BOOL->INT direction than INT->BOOL.  In C++,
any bool value is either 0 or 1 and you can do such weirdness
(brokeness?) as:

bool val = false;
if (val == 0) { /* blah */ }

Anyway, with Qt, it converts bool values to integers.  I think Qt's
probably right on this front in that the value 1 should be considered
true and 0 should be considered false.  My $0.02.  -sc

--
Sean Chittenden

Sean Chittenden wrote:
> Alrighty.  Do you want an updated patch for the single character
> tweak or can you futz with it before committing?  :)

I oppose casts from boolean to integer or vice versa.

> Anyway, with Qt, it converts bool values to integers.  I think Qt's
> probably right on this front in that the value 1 should be considered
> true and 0 should be considered false.  My $0.02.  -sc

That's really Qt's problem.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut wrote:

> Sean Chittenden wrote:
>> Alrighty.  Do you want an updated patch for the single character
>> tweak or can you futz with it before committing?  :)
>
> I oppose casts from boolean to integer or vice versa.

Even _explicit_ casts only? It would not have any bad side effects on people
not using it, would it?

Best Regards,
Michael Paesold

"Michael Paesold" <mpaesold@gmx.at> writes:
> Peter Eisentraut wrote:
>> I oppose casts from boolean to integer or vice versa.

> Even _explicit_ casts only? It would not have any bad side effects on people
> not using it, would it?

I agree with Michael's position.  I don't like implicit/automatic casts
any more than Peter does, but I don't see a strong argument against
providing explicit casts.

            regards, tom lane

>> Is there any reason why the backend doesn't cast an unquoted integer
>> to
>> a boolean value?
>
> Hidden cross-category typecasts are evil.  I'd accept this as an
> explicit cast ('e' in pg_cast) but not automatic.
>
> Also, what about the other direction?  Providing a cast in only one
> direction is pretty inconsistent.

test=> SELECT 1::BOOL, 0::BOOL, TRUE::INT4, FALSE::INT4;
  bool | bool | int4 | int4
------+------+------+------
  t    | f    |    1 |    0
(1 row)

Okey doke, both directions are now provided and the cast has to be
explicit.  -sc



--
Sean Chittenden

On Sun, 2004-10-10 at 16:27, Sean Chittenden wrote:
> Is there any reason why the backend doesn't cast an unquoted integer to
> a boolean value?

Can you add some regression tests, please?

The patch treats any non-zero value as "true". Is that the behavior we
want, or should we only allow "1" as an integer representation of
"true"? (I'm not sure myself, I just don't think copying C here is
necessarily the best guide.)

The patch changes the system catalog; it probably ought to also bump the
catalog version number. That also means this probably isn't 8.0
material, unless we make an unrelated system catalog change in a future
beta (... and even then, I'm not sure if I'd cal it a bug fix).

-Neil

Neil Conway <neilc@samurai.com> writes:
> The patch changes the system catalog; it probably ought to also bump the
> catalog version number. That also means this probably isn't 8.0
> material, unless we make an unrelated system catalog change in a future
> beta (... and even then, I'm not sure if I'd cal it a bug fix).

We've already forced an initdb for beta4, so I think any additional
reasons for forcing one are "free" until beta4 goes out.  But having
said that, this is clearly a feature not a bug.

            regards, tom lane

>> Is there any reason why the backend doesn't cast an unquoted integer
>> to
>> a boolean value?
>
> Can you add some regression tests, please?

:-/  I have zero understanding or knowledge of the regression test
suite.  Given the simplicity of the casts, does this really need a
require a regression test?  I could've written it as:
"return(PG_GETARG_INT32(0) ? true : false);" and saved a few lines of
code... there's no chance or room for a bug or change in behavior
unless C changes its trinary operator.... and that's not gunna happen
until hell freezes over and we've all taken up skiing.

> The patch treats any non-zero value as "true". Is that the behavior we
> want, or should we only allow "1" as an integer representation of
> "true"? (I'm not sure myself, I just don't think copying C here is
> necessarily the best guide.)

I would posit that this is the desired behavior as it's consistent with
every language I can think of.

> The patch changes the system catalog; it probably ought to also bump
> the
> catalog version number. That also means this probably isn't 8.0
> material, unless we make an unrelated system catalog change in a future
> beta (... and even then, I'm not sure if I'd cal it a bug fix).

System catalog bumps have been coming through with some degree of
regularity so I wasn't worried about providing the patch to bump the
catalog date.  -sc

--
Sean Chittenden

Sean Chittenden <chitt@speakeasy.net> writes:
>> Can you add some regression tests, please?

> Given the simplicity of the casts, does this really need a
> require a regression test?

That request seems quite over-the-top to me too.  The real problem here
is just whether we want to be accepting a feature addition, small though
it be, at this stage of the beta cycle.  I've got mixed emotions about
that myself.  The odds of breaking anything with this patch seem
essentially zero, and we have certainly seen this requested multiple
times before, so one part of me says "sure, push it in".  But from a
project-management standpoint it's hard to justify not saying "it's got
to wait for 8.1".

>> The patch treats any non-zero value as "true". Is that the behavior we
>> want, or should we only allow "1" as an integer representation of
>> "true"? (I'm not sure myself, I just don't think copying C here is
>> necessarily the best guide.)

> I would posit that this is the desired behavior as it's consistent with
> every language I can think of.

I can't see anything wrong with it either.  Throwing an error for
anything except 0 or 1 would be the other possibility, but that doesn't
seem like it really buys anything.

>> The patch changes the system catalog; it probably ought to also bump
>> the catalog version number.

> System catalog bumps have been coming through with some degree of
> regularity so I wasn't worried about providing the patch to bump the
> catalog date.  -sc

I think the agreed protocol is that the descriptive text should
remind the committer to bump the catversion upon application.  Just
to make sure he doesn't forget ;-)

            regards, tom lane

>>> Can you add some regression tests, please?
>
>> Given the simplicity of the casts, does this really need a
>> require a regression test?
>
> That request seems quite over-the-top to me too.  The real problem here
> is just whether we want to be accepting a feature addition, small
> though
> it be, at this stage of the beta cycle.  I've got mixed emotions about
> that myself.  The odds of breaking anything with this patch seem
> essentially zero, and we have certainly seen this requested multiple
> times before, so one part of me says "sure, push it in".  But from a
> project-management standpoint it's hard to justify not saying "it's got
> to wait for 8.1".

*puts on patch advocate's hat*

 From a feature/usability standpoint, it'd be "more convenient" to have
booleans behave consistently starting with the 8.X branch as opposed to
saying, "starting with 8.1, booleans can be explicitly cast to integers
and visa versa."  From a patch maintainer stand point, given the work
of the original patch, it's hardly worth the effort to maintain and
given the possibility of OIDs being used, waiting is more work than
committing it now.  :)

>>> The patch changes the system catalog; it probably ought to also bump
>>> the catalog version number.
>
>> System catalog bumps have been coming through with some degree of
>> regularity so I wasn't worried about providing the patch to bump the
>> catalog date.  -sc
>
> I think the agreed protocol is that the descriptive text should
> remind the committer to bump the catversion upon application.  Just
> to make sure he doesn't forget ;-)

Fair enough... though with this discussion it would seem like a rather
unnecessary cudgel to the head.  Next time I'll bump it when adding a
built-in function, however.  Thanks for the protocol FYI.  -sc

--
Sean Chittenden

On Mon, 11 Oct 2004, Sean Chittenden wrote:

> > The patch treats any non-zero value as "true". Is that the behavior we
> > want, or should we only allow "1" as an integer representation of
> > "true"? (I'm not sure myself, I just don't think copying C here is
> > necessarily the best guide.)
>
> I would posit that this is the desired behavior as it's consistent with
> every language I can think of.

However, AFAIK it's inconsitent with the type input function which
supports '1' and '0' but not other integers.

Stephan Szabo <sszabo@megazone.bigpanda.com> writes:
> On Mon, 11 Oct 2004, Sean Chittenden wrote:
>> I would posit that this is the desired behavior as it's consistent with
>> every language I can think of.

> However, AFAIK it's inconsitent with the type input function which
> supports '1' and '0' but not other integers.

So?  The type input function also accepts 't', 'f', and other spellings
that are not in the input domain for an integer-to-bool coercion.  Will
you argue we should remove those allowed inputs so that it can be 100%
compatible with the coercion?

The question for an integer-to-bool conversion is what is useful and
expected behavior for that conversion; I don't think that's necessarily
the same as what the textual conversion should do.

A possibly useful analogy is that real-to-integer coercion rounds off
fractions; it doesn't error out, even though the integer input function
won't take a string that includes a decimal point.

To pollute this abstract discussion with an actual fact ;-) ...
I note from recent discussion on the ODBC list that M$ Access likes
to use "-1" to represent TRUE.  So it would certainly make life easier
for Access migrants if the int-to-bool coercion would accept -1.

            regards, tom lane

On Tue, 2004-10-12 at 11:54, Sean Chittenden wrote:
> :-/  I have zero understanding or knowledge of the regression test
> suite.

It is trivial: add some SQL to src/test/regress/sql/foo.sql, run the
tests, hand-verify the output (e.g. by looking at regression.diffs), and
then copy the updated output from src/test/regress/results to
src/test/regress/expected

> Given the simplicity of the casts, does this really need a
> require a regression test?

Yeah, I suppose not. Personally I'd always err on the side of doing more
testing rather than less (I don't see much point in striving for
minimalism in a test suite), but you're right that this functionality is
probably sufficiently trivial it doesn't need tests.

-Neil

On Mon, 11 Oct 2004, Tom Lane wrote:

> Stephan Szabo <sszabo@megazone.bigpanda.com> writes:
> > On Mon, 11 Oct 2004, Sean Chittenden wrote:
> >> I would posit that this is the desired behavior as it's consistent with
> >> every language I can think of.
>
> > However, AFAIK it's inconsitent with the type input function which
> > supports '1' and '0' but not other integers.
>

[strawman snipped]

> The question for an integer-to-bool conversion is what is useful and
> expected behavior for that conversion; I don't think that's necessarily
> the same as what the textual conversion should do.
>
> A possibly useful analogy is that real-to-integer coercion rounds off
> fractions; it doesn't error out, even though the integer input function
> won't take a string that includes a decimal point.

Well, technically AFAICS, a string to integer conversion has to follow the
same general rules as numeric to integer by SQL99.  So a varchar value of
'2.5' cast to int should work and give you the same value as 2.5 cast to
int (or at least, I can't see any other way to read 6.22GR6b). But since
we do odd things with quoted literals, it makes the case anyway and I
withdraw the comment.

Am Montag, 11. Oktober 2004 15:50 schrieb Tom Lane:
> I agree with Michael's position.  I don't like implicit/automatic casts
> any more than Peter does, but I don't see a strong argument against
> providing explicit casts.

I find the chosen mapping to be somewhat arbitrary and artifical.  (2::int =>
'2'::text is not arbitrary in my mind, but 1::int => true is just something a
C programmer could make up.)  But I'm clearly in the minority here.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

>>> The patch treats any non-zero value as "true". Is that the behavior
>>> we
>>> want, or should we only allow "1" as an integer representation of
>>> "true"? (I'm not sure myself, I just don't think copying C here is
>>> necessarily the best guide.)
>>
>> I would posit that this is the desired behavior as it's consistent
>> with
>> every language I can think of.
>
> However, AFAIK it's inconsitent with the type input function which
> supports '1' and '0' but not other integers.

I actually pondered that and came up with a patch that I didn't submit.
  False has a very specific set of possibilities that can be reasonably
easily defined.  True, is anything not false.  I eventually didn't
submit it because I was able to convince myself with the following
statement.

Regardless of whether or not true is any non-zero value, this is a
database where data and its inputs must be validated and constrained to
a given set of probable and process-able possibilities.  Perl's
decision to let any non-empty string be true doesn't mean a database
should take any nonfalse-like value and assume it should be true.
42::BOOL == TRUE, on the other hand, has a long mathematical president
wherein non-zero values are true and zero values are false.

Unlike the previous int4_bool()/bool_int4() patch which addresses a
mathematical technicality, accepting different string values as true or
false seems exceedingly dangerous, though probably an okay
interpretation.  I went one step further, however, and tested for an
empty string as a valid false value (one of Perl's false values).

Since this subject isn't ever going to get resolved, I don't think it's
worth trudging down this path, but, I thought the extreme is helpful in
justifying the current string->bool conversion and the new
int4->bool/bool->int4 conversion, IMHO.  -sc


... I wonder what color this bikeshed is gunna be...



--
Sean Chittenden

Sean Chittenden said:
> Perl's
> decision to let any non-empty string be true doesn't mean a database
> should take any nonfalse-like value and assume it should be true.
> 42::BOOL == TRUE, on the other hand, has a long mathematical president
> wherein non-zero values are true and zero values are false.
>

(ITMY precedent)

FYI, perl does not quite do this. Both "" and "0" are false. Any other
string is true.

Of course, the Unix shell treats an exit value of 0 as success and non-zero
as failure, so the rule is hardly universal.

Personally, I would prefer to make boolean as opaque as possible. But I
don't care that much.

>
>
> ... I wonder what color this bikeshed is gunna be...

purple.

cheers

andrew

This has been saved for the 8.1 release:

    http:/momjian.postgresql.org/cgi-bin/pgpatches2

---------------------------------------------------------------------------

 Sean Chittenden wrote:
> >> Is there any reason why the backend doesn't cast an unquoted integer
> >> to
> >> a boolean value?
> >
> > Hidden cross-category typecasts are evil.  I'd accept this as an
> > explicit cast ('e' in pg_cast) but not automatic.
> >
> > Also, what about the other direction?  Providing a cast in only one
> > direction is pretty inconsistent.
>
> test=> SELECT 1::BOOL, 0::BOOL, TRUE::INT4, FALSE::INT4;
>   bool | bool | int4 | int4
> ------+------+------+------
>   t    | f    |    1 |    0
> (1 row)
>
> Okey doke, both directions are now provided and the cast has to be
> explicit.  -sc
>

[ Attachment, skipping... ]

>
>
> --
> Sean Chittenden

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
>       subscribe-nomail command to majordomo@postgresql.org so that your
>       message can get through to the mailing list cleanly

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Patch applied by Neil.  Thanks.

---------------------------------------------------------------------------


Sean Chittenden wrote:
> >> Is there any reason why the backend doesn't cast an unquoted integer
> >> to
> >> a boolean value?
> >
> > Hidden cross-category typecasts are evil.  I'd accept this as an
> > explicit cast ('e' in pg_cast) but not automatic.
> >
> > Also, what about the other direction?  Providing a cast in only one
> > direction is pretty inconsistent.
>
> test=> SELECT 1::BOOL, 0::BOOL, TRUE::INT4, FALSE::INT4;
>   bool | bool | int4 | int4
> ------+------+------+------
>   t    | f    |    1 |    0
> (1 row)
>
> Okey doke, both directions are now provided and the cast has to be
> explicit.  -sc
>

[ Attachment, skipping... ]

>
>
> --
> Sean Chittenden

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
>       subscribe-nomail command to majordomo@postgresql.org so that your
>       message can get through to the mailing list cleanly

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073