Thread: plpgsql doesn't coerce boolean expressions to boolean
Following up this gripe http://archives.postgresql.org/pgsql-sql/2003-09/msg00044.php I've realized that plpgsql just assumes that the test expression of an IF, WHILE, or EXIT statement is a boolean expression. It doesn't take any measures to ensure this is the case or convert the value if it's not the case. This seems pretty bogus to me. However ... with the code as it stands, for pass-by-reference datatypes any nonnull value will appear TRUE, while for pass-by-value datatypes any nonzero value will appear TRUE. I fear that people may actually be depending on these behaviors, particularly the latter one which is pretty reasonable if you're accustomed to C. So while I'd like to throw an error if the argument isn't boolean, I'm afraid of breaking people's function definitions. Here are some possible responses, roughly in order of difficulty to implement: 1. Leave well enough alone (and perhaps document the behavior). 2. Throw an error if the expression doesn't return boolean. 3. Try to convert nonbooleans to boolean using plpgsql's usual method for cross-type coercion, ie run the type's outputproc to get a string and feed it to bool's input proc. (This seems unlikely to avoid throwing an error in very manycases, but it'd be the most consistent with other parts of plpgsql.) 4. Use the parser's coerce_to_boolean procedure, so that nonbooleans will be accepted in exactly the same cases where they'dbe accepted in a boolean-requiring SQL construct (such as CASE). (By default, none are, so this isn't really differentfrom #2. But people could create casts to boolean to override this behavior in a controlled fashion.) Any opinions about what to do? regards, tom lane
Tom Lane wrote: >Following up this gripe >http://archives.postgresql.org/pgsql-sql/2003-09/msg00044.php >I've realized that plpgsql just assumes that the test expression >of an IF, WHILE, or EXIT statement is a boolean expression. It >doesn't take any measures to ensure this is the case or convert >the value if it's not the case. This seems pretty bogus to me. > >However ... with the code as it stands, for pass-by-reference datatypes >any nonnull value will appear TRUE, while for pass-by-value datatypes >any nonzero value will appear TRUE. I fear that people may actually be >depending on these behaviors, particularly the latter one which is >pretty reasonable if you're accustomed to C. So while I'd like to throw >an error if the argument isn't boolean, I'm afraid of breaking people's >function definitions. > >Here are some possible responses, roughly in order of difficulty >to implement: > >1. Leave well enough alone (and perhaps document the behavior). > >2. Throw an error if the expression doesn't return boolean. > >3. Try to convert nonbooleans to boolean using plpgsql's usual method > for cross-type coercion, ie run the type's output proc to get a > string and feed it to bool's input proc. (This seems unlikely to > avoid throwing an error in very many cases, but it'd be the most > consistent with other parts of plpgsql.) > >4. Use the parser's coerce_to_boolean procedure, so that nonbooleans > will be accepted in exactly the same cases where they'd be accepted > in a boolean-requiring SQL construct (such as CASE). (By default, > none are, so this isn't really different from #2. But people could > create casts to boolean to override this behavior in a controlled > fashion.) > >Any opinions about what to do? > > > It won't bite me so maybe I don't have a right to express an opinion :-) plpgsql is not C - it appears to be in the Algol/Pascal/Ada family, which do tend to avoid implicit type conversion. On that basis, option 2 seems like it might be the right answer and also the one most likely to break lots of existing functions. Maybe the right thing would be to deprecate relying on implicit conversion to boolean for one release cycle and then make it an error. cheers andrew
Tom Lane wrote: >Here are some possible responses, roughly in order of difficulty >to implement: > >1. Leave well enough alone (and perhaps document the behavior). > >2. Throw an error if the expression doesn't return boolean. > I'd opt for 2. It's quite common that newer compilers will detect more bogus coding than older ones. There might be existing functions that break from this because they rely on the current "feature", but there are probably others that will throw an exception, revealing bad coding (and delivering correct results just by chance, I've seen this more than once...) Regards, Andreas
Andreas Pflug <pgadmin@pse-consulting.de> writes: > Tom Lane wrote: > > > > >2. Throw an error if the expression doesn't return boolean. > > > I'd opt for 2. > It's quite common that newer compilers will detect more bogus coding > than older ones. There might be existing functions that break from > this because they rely on the current "feature", but there are > probably others that will throw an exception, revealing bad coding > (and delivering correct results just by chance, I've seen this more > than once...) I agree, and option 2 also makes sure that "bad" code will fail cleanly, rather than possibly changing behavior and causing data loss/corruption. I agree with another poster that deprecation in 7.4 and removal in 7.5 might make sense. -Doug
Doug McNaught <doug@mcnaught.org> writes:
> I agree with another poster that deprecation in 7.4 and removal in
> 7.5 might make sense.
How would we "deprecate" it exactly? Throw a NOTICE?
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Doug McNaught <doug@mcnaught.org> writes: > > I agree with another poster that deprecation in 7.4 and removal in > > 7.5 might make sense. > > How would we "deprecate" it exactly? Throw a NOTICE? I was thinking of just a mention in the release notes that we've found this problem and intend to fix it after giving people a release cycle to adjust. Not that people read release notes... :( -Doug
Tom Lane wrote: >Doug McNaught <doug@mcnaught.org> writes: > > >>I agree with another poster that deprecation in 7.4 and removal in >>7.5 might make sense. >> >> > >How would we "deprecate" it exactly? Throw a NOTICE? > Good question. A NOTICE just might be ignored, breaking everything "surprisingly" in 7.5. To speak a bit more general, how about some sort of "deprecation checker" setting, if set to true anything marked as deprecated will throw an error, if false only a notice will be generated. This would enable users to check their existing stuff for future compatibility before it's broken in the next release. Regards, Andreas
Tom Lane wrote: >Doug McNaught <doug@mcnaught.org> writes: > > >>I agree with another poster that deprecation in 7.4 and removal in >>7.5 might make sense. >> >> > >How would we "deprecate" it exactly? Throw a NOTICE? > > > Release notes, I guess. A NOTICE would be fine as long as it didn't result in a flood of them. If that happened once at parse time that should be fine, I think. What's the practice in deprecating other "features"? cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>> How would we "deprecate" it exactly? Throw a NOTICE?
>>
> Release notes, I guess. A NOTICE would be fine as long as it didn't
> result in a flood of them. If that happened once at parse time that
> should be fine, I think.
It would be relatively difficult to do that; given the way plpgsql is
structured, a runtime message would be a lot easier.
> What's the practice in deprecating other "features"?
We generally don't ;-). Certainly 7.4 contains bigger incompatible
changes than this one, and so have most of our prior releases.
regards, tom lane
Tom Lane wrote: >Andrew Dunstan <andrew@dunslane.net> writes: > > >>What's the practice in deprecating other "features"? >> >> > >We generally don't ;-). Certainly 7.4 contains bigger incompatible >changes than this one, and so have most of our prior releases. > > > I thought I had seen discussions along the lines of "we'll give them one cycle to fix it and then they are shot". It does seem very late in this cycle to make such changes. cheers andrew
On Mon, 08 Sep 2003 11:40:32 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote: >4. Use the parser's coerce_to_boolean procedure, so that nonbooleans > will be accepted in exactly the same cases where they'd be accepted > in a boolean-requiring SQL construct (such as CASE). (By default, > none are, so this isn't really different from #2. But people could > create casts to boolean to override this behavior in a controlled > fashion.) I vote for 4. And - being fully aware of similar proposals having failed miserably - I propose to proceed as follows: If the current behaviour is considered a bug, let i=4, else let i=5. In 7.i: Create a new GUC variable "plpgsql_strict_boolean" (silly name, I know) in the "VERSION/PLATFORM COMPATIBILITY" section of postgresql.conf. Make the new behaviour dependent on this variable. Default plpgsql_strict_boolean to false. Place a warning into the release notes and maybe into the plpgsql documentation. In 7.j, j>i: Change the default value of plpgsql_strict_boolean to true. Issue WARNINGs or NOTICEs as appropriate. Update documentation. In 7.k, k>j: Remove old behaviour and GUC variable. Update documentation. ServusManfred
Manfred Koizar <mkoi-pg@aon.at> writes:
> On Mon, 08 Sep 2003 11:40:32 -0400, Tom Lane <tgl@sss.pgh.pa.us>
> wrote:
>> 4. Use the parser's coerce_to_boolean procedure, so that nonbooleans
>> will be accepted in exactly the same cases where they'd be accepted
>> in a boolean-requiring SQL construct (such as CASE). (By default,
>> none are, so this isn't really different from #2. But people could
>> create casts to boolean to override this behavior in a controlled
>> fashion.)
> I vote for 4.
I'm willing to do that.
> And - being fully aware of similar proposals having
> failed miserably - I propose to proceed as follows:
> If the current behaviour is considered a bug, let i=4, else let i=5.
> In 7.i: Create a new GUC variable "plpgsql_strict_boolean" (silly
> name, I know) in the "VERSION/PLATFORM COMPATIBILITY" section of
> postgresql.conf. Make the new behaviour dependent on this variable.
> Default plpgsql_strict_boolean to false. Place a warning into the
> release notes and maybe into the plpgsql documentation.
> In 7.j, j>i: Change the default value of plpgsql_strict_boolean to
> true. Issue WARNINGs or NOTICEs as appropriate. Update
> documentation.
> In 7.k, k>j: Remove old behaviour and GUC variable. Update
> documentation.
I'm not willing to do that much work for what is, in the greater scheme
of things, a tiny change. If we did that for every user-visible change,
our rate of forward progress would be a mere fraction of what it is.
regards, tom lane
Define the language! If it breaks code, so be it. <p><b>2. Throw an error if the expression doesn't return boolean.</b><br/>Yes, yes, absolutely. <p>By definition "an IF, WHILE, or EXIT statement is a boolean expression" <br />SO<br /> if "some stupid piece of text" THEN <br />should not compile, there is no BOOLEAN expression. <p>C's implementationof hat is true and false has always, IMHO, been hideous. But then again, I am a Pascal kind of thinker. <br/>An integer with a value of 1 is still only an integer, <br /> IF I <> 0 THEN ... <br />is clear and un-ambiguous.<br /> <br /> <p>Tom Lane wrote: <blockquote type="CITE">Following up this gripe <br /><a href="http://archives.postgresql.org/pgsql-sql/2003-09/msg00044.php">http://archives.postgresql.org/pgsql-sql/2003-09/msg00044.php</a><br />I'verealized that plpgsql just assumes that the test expression <br />of an IF, WHILE, or EXIT statement is a boolean expression. It <br />doesn't take any measures to ensure this is the case or convert <br />the value if it's not the case. This seems pretty bogus to me. <p>However ... with the code as it stands, for pass-by-reference datatypes <br />anynonnull value will appear TRUE, while for pass-by-value datatypes <br />any nonzero value will appear TRUE. I fearthat people may actually be <br />depending on these behaviors, particularly the latter one which is <br />pretty reasonableif you're accustomed to C. So while I'd like to throw <br />an error if the argument isn't boolean, I'm afraidof breaking people's <br />function definitions. <p>Here are some possible responses, roughly in order of difficulty<br />to implement: <p>1. Leave well enough alone (and perhaps document the behavior). <p>2. Throw an error ifthe expression doesn't return boolean. <p>3. Try to convert nonbooleans to boolean using plpgsql's usual method <br /> for cross-type coercion, ie run the type's output proc to get a <br /> string and feed it to bool's input proc. (Thisseems unlikely to <br /> avoid throwing an error in very many cases, but it'd be the most <br /> consistent withother parts of plpgsql.) <p>4. Use the parser's coerce_to_boolean procedure, so that nonbooleans <br /> will be acceptedin exactly the same cases where they'd be accepted <br /> in a boolean-requiring SQL construct (such as CASE). (By default, <br /> none are, so this isn't really different from #2. But people could <br /> create casts toboolean to override this behavior in a controlled <br /> fashion.) <p>Any opinions about what to do? <p> regards, tom lane <p>---------------------------(end of broadcast)--------------------------- <br/>TIP 3: if posting/reading through Usenet, please send an appropriate <br /> subscribe-nomail command to majordomo@postgresql.orgso that your <br /> message can get through to the mailing list cleanly</blockquote>
Tom Lane wrote: > Following up this gripe > http://archives.postgresql.org/pgsql-sql/2003-09/msg00044.php > I've realized that plpgsql just assumes that the test expression > of an IF, WHILE, or EXIT statement is a boolean expression. It > doesn't take any measures to ensure this is the case or convert > the value if it's not the case. This seems pretty bogus to me. > > However ... with the code as it stands, for pass-by-reference datatypes > any nonnull value will appear TRUE, while for pass-by-value datatypes > any nonzero value will appear TRUE. I fear that people may actually be > depending on these behaviors, particularly the latter one which is > pretty reasonable if you're accustomed to C. So while I'd like to throw > an error if the argument isn't boolean, I'm afraid of breaking people's > function definitions. > > Here are some possible responses, roughly in order of difficulty > to implement: > > 1. Leave well enough alone (and perhaps document the behavior). > > 2. Throw an error if the expression doesn't return boolean. ERROR is the cleanest way, but I'd vote for conversion to boolean to keep the damage within reason. Jan > > 3. Try to convert nonbooleans to boolean using plpgsql's usual method > for cross-type coercion, ie run the type's output proc to get a > string and feed it to bool's input proc. (This seems unlikely to > avoid throwing an error in very many cases, but it'd be the most > consistent with other parts of plpgsql.) > > 4. Use the parser's coerce_to_boolean procedure, so that nonbooleans > will be accepted in exactly the same cases where they'd be accepted > in a boolean-requiring SQL construct (such as CASE). (By default, > none are, so this isn't really different from #2. But people could > create casts to boolean to override this behavior in a controlled > fashion.) > > Any opinions about what to do? > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
Jan Wieck <JanWieck@Yahoo.com> writes:
> ERROR is the cleanest way, but I'd vote for conversion to boolean to
> keep the damage within reason.
Which style of conversion did you like? These were the choices:
>> 3. Try to convert nonbooleans to boolean using plpgsql's usual method
>> for cross-type coercion, ie run the type's output proc to get a
>> string and feed it to bool's input proc. (This seems unlikely to
>> avoid throwing an error in very many cases, but it'd be the most
>> consistent with other parts of plpgsql.)
>>
>> 4. Use the parser's coerce_to_boolean procedure, so that nonbooleans
>> will be accepted in exactly the same cases where they'd be accepted
>> in a boolean-requiring SQL construct (such as CASE). (By default,
>> none are, so this isn't really different from #2. But people could
>> create casts to boolean to override this behavior in a controlled
>> fashion.)
At this point I'm kinda leaning to #4, because (for example) people
could create a cast from integer to boolean to avoid having to fix their
plpgsql functions right away. #3 would not offer any configurability of
behavior.
regards, tom lane
Tom Lane wrote: >>>4. Use the parser's coerce_to_boolean procedure, so that nonbooleans >>>will be accepted in exactly the same cases where they'd be accepted >>>in a boolean-requiring SQL construct (such as CASE). (By default, >>>none are, so this isn't really different from #2. But people could >>>create casts to boolean to override this behavior in a controlled >>>fashion.) >>> >>> > >At this point I'm kinda leaning to #4, because (for example) people >could create a cast from integer to boolean to avoid having to fix their >plpgsql functions right away. #3 would not offer any configurability of >behavior. > > > Won't people have to analyse their functions to find out what sort of casts they need to create? If so, why don't they just fix the functions while they are about it? Surely the fixes in most cases will be quite trivial, and in all cases backwards compatible. Does anyone have a take on how many people would be affected? Or how much they would be affected? cheers andrew
Tom Lane wrote: > Manfred Koizar <mkoi-pg@aon.at> writes: > > On Mon, 08 Sep 2003 11:40:32 -0400, Tom Lane <tgl@sss.pgh.pa.us> > > wrote: > >> 4. Use the parser's coerce_to_boolean procedure, so that nonbooleans > >> will be accepted in exactly the same cases where they'd be accepted > >> in a boolean-requiring SQL construct (such as CASE). (By default, > >> none are, so this isn't really different from #2. But people could > >> create casts to boolean to override this behavior in a controlled > >> fashion.) > > > I vote for 4. > > I'm willing to do that. OK, what release should we do this? -- 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, Pennsylvania19073
I would suggest to throw a error, or at least a warning. This will FORCE people to program in the correct way. I also thought that 'IF $1 THEN ...' should work ok but giving it a other thought it's indeed stuped to write that way (I'm from the C world...) Ries -----Oorspronkelijk bericht----- Van: pgsql-sql-owner@postgresql.org [mailto:pgsql-sql-owner@postgresql.org]Namens Tom Lane Verzonden: maandag 8 september 2003 17:41 Aan: pgsql-hackers@postgresql.org; pgsql-sql@postgresql.org Onderwerp: [SQL] plpgsql doesn't coerce boolean expressions to boolean Following up this gripe http://archives.postgresql.org/pgsql-sql/2003-09/msg00044.php I've realized that plpgsql just assumes that the test expression of an IF, WHILE, or EXIT statement is a boolean expression. It doesn't take any measures to ensure this is the case or convert the value if it's not the case. This seems pretty bogus to me. However ... with the code as it stands, for pass-by-reference datatypes any nonnull value will appear TRUE, while for pass-by-value datatypes any nonzero value will appear TRUE. I fear that people may actually be depending on these behaviors, particularly the latter one which is pretty reasonable if you're accustomed to C. So while I'd like to throw an error if the argument isn't boolean, I'm afraid of breaking people's function definitions. Here are some possible responses, roughly in order of difficulty to implement: 1. Leave well enough alone (and perhaps document the behavior). 2. Throw an error if the expression doesn't return boolean. 3. Try to convert nonbooleans to boolean using plpgsql's usual method for cross-type coercion, ie run the type's outputproc to get a string and feed it to bool's input proc. (This seems unlikely to avoid throwing an error in very manycases, but it'd be the most consistent with other parts of plpgsql.) 4. Use the parser's coerce_to_boolean procedure, so that nonbooleans will be accepted in exactly the same cases where they'dbe accepted in a boolean-requiring SQL construct (such as CASE). (By default, none are, so this isn't really differentfrom #2. But people could create casts to boolean to override this behavior in a controlled fashion.) Any opinions about what to do? regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to majordomo@postgresql.orgso that your message can get through to the mailing list cleanly
Tom Lane wrote: > Jan Wieck <JanWieck@Yahoo.com> writes: >> ERROR is the cleanest way, but I'd vote for conversion to boolean to >> keep the damage within reason. > > Which style of conversion did you like? These were the choices: > >>> 3. Try to convert nonbooleans to boolean using plpgsql's usual method >>> for cross-type coercion, ie run the type's output proc to get a >>> string and feed it to bool's input proc. (This seems unlikely to >>> avoid throwing an error in very many cases, but it'd be the most >>> consistent with other parts of plpgsql.) >>> >>> 4. Use the parser's coerce_to_boolean procedure, so that nonbooleans >>> will be accepted in exactly the same cases where they'd be accepted >>> in a boolean-requiring SQL construct (such as CASE). (By default, >>> none are, so this isn't really different from #2. But people could >>> create casts to boolean to override this behavior in a controlled >>> fashion.) > > At this point I'm kinda leaning to #4, because (for example) people > could create a cast from integer to boolean to avoid having to fix their > plpgsql functions right away. #3 would not offer any configurability of > behavior. Agreed - #4. Thinking of the problem about deprication of features and transition time, it would be nice for this kind of compatibility breaking changes to have a _per database_ config option that controls old vs. new behaviour, wouldn't it? Don't know exactly how you'd like that to be. Maybe with a pg_config catalog that inherits default settings from template1 but can then be changed in every database. This would even include the possibility to *switch* one single prod database back to the old behaviour in case the supposedly cleaned up application isn't as clean as supposed to. Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
Where are we on this --- we all decided on #4. Does this just require an announcment in the release notes. (I need to complete the release notes soon.) --------------------------------------------------------------------------- Tom Lane wrote: > Following up this gripe > http://archives.postgresql.org/pgsql-sql/2003-09/msg00044.php > I've realized that plpgsql just assumes that the test expression > of an IF, WHILE, or EXIT statement is a boolean expression. It > doesn't take any measures to ensure this is the case or convert > the value if it's not the case. This seems pretty bogus to me. > > However ... with the code as it stands, for pass-by-reference datatypes > any nonnull value will appear TRUE, while for pass-by-value datatypes > any nonzero value will appear TRUE. I fear that people may actually be > depending on these behaviors, particularly the latter one which is > pretty reasonable if you're accustomed to C. So while I'd like to throw > an error if the argument isn't boolean, I'm afraid of breaking people's > function definitions. > > Here are some possible responses, roughly in order of difficulty > to implement: > > 1. Leave well enough alone (and perhaps document the behavior). > > 2. Throw an error if the expression doesn't return boolean. > > 3. Try to convert nonbooleans to boolean using plpgsql's usual method > for cross-type coercion, ie run the type's output proc to get a > string and feed it to bool's input proc. (This seems unlikely to > avoid throwing an error in very many cases, but it'd be the most > consistent with other parts of plpgsql.) > > 4. Use the parser's coerce_to_boolean procedure, so that nonbooleans > will be accepted in exactly the same cases where they'd be accepted > in a boolean-requiring SQL construct (such as CASE). (By default, > none are, so this isn't really different from #2. But people could > create casts to boolean to override this behavior in a controlled > fashion.) > > Any opinions about what to do? > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > -- 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, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Where are we on this --- we all decided on #4. Does this just require
> an announcment in the release notes.
I haven't done anything about it --- been busy with other stuff, and I
wasn't sure we'd agreed to change it for 7.4 anyway. I'm willing to
make the code change though.
regards, tom lane
Jan Wieck <JanWieck@Yahoo.com> writes:
> Tom Lane wrote:
>>> 4. Use the parser's coerce_to_boolean procedure, so that nonbooleans
>>> will be accepted in exactly the same cases where they'd be accepted
>>> in a boolean-requiring SQL construct (such as CASE). (By default,
>>> none are, so this isn't really different from #2. But people could
>>> create casts to boolean to override this behavior in a controlled
>>> fashion.)
> Agreed - #4.
My first attempt at doing this failed to pass the regression tests,
because it wasn't prepared for this:
if count(*) = 0 from Room where roomno = new.roomno then raise exception ''Room % does not exist'',
new.roomno; end if;
Is this really intended to be a feature? It manages to work because
plpgsql simply sticks "SELECT " in front of whatever appears between
IF and THEN, and passes the result to the main SQL engine. But it sure
surprised the heck out of me. The documentation gives no hint that
you're allowed to write anything but a straight boolean expression in IF.
Does Oracle allow that sort of thing?
I would be inclined to think that a more reasonable expression of the
intent would be
if (select count(*) from Room where roomno = new.roomno) = 0 then
Certainly we'd have a big problem supporting the existing coding if we
ever reimplement plpgsql with more awareness of what expressions are.
regards, tom lane
Tom Lane wrote:
> Jan Wieck <JanWieck@Yahoo.com> writes:
>> Tom Lane wrote:
>>>> 4. Use the parser's coerce_to_boolean procedure, so that nonbooleans
>>>> will be accepted in exactly the same cases where they'd be accepted
>>>> in a boolean-requiring SQL construct (such as CASE). (By default,
>>>> none are, so this isn't really different from #2. But people could
>>>> create casts to boolean to override this behavior in a controlled
>>>> fashion.)
>
>> Agreed - #4.
>
> My first attempt at doing this failed to pass the regression tests,
> because it wasn't prepared for this:
>
> if count(*) = 0 from Room where roomno = new.roomno then
> raise exception ''Room % does not exist'', new.roomno;
> end if;
>
> Is this really intended to be a feature? It manages to work because
> plpgsql simply sticks "SELECT " in front of whatever appears between
> IF and THEN, and passes the result to the main SQL engine. But it sure
> surprised the heck out of me. The documentation gives no hint that
> you're allowed to write anything but a straight boolean expression in IF.
> Does Oracle allow that sort of thing?
I have to admit it was less an intention than more a side effect of the
actual implementation. It was so easy to simply stick "SELECT " in front
of "everything between IF and THEN" and expect the result to be a boolean.
In the same way you can do
varname := count(*) from Room where roomno = new.roomno;
which is straight forward because it's simply sticking "SELECT " in
front of "everything between := and ;". Well, this does a bit more in
that it tries the typinput(typoutput(result)) casting hack ... I know
that you don't like that one.
>
> I would be inclined to think that a more reasonable expression of the
> intent would be
>
> if (select count(*) from Room where roomno = new.roomno) = 0 then
>
> Certainly we'd have a big problem supporting the existing coding if we
> ever reimplement plpgsql with more awareness of what expressions are.
Without parsing much, much more, and finally parsing basically the whole
SQL grammar in the PL/pgSQL parser, I don't see how you can do that.
Jan
--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #
Jan Wieck <JanWieck@Yahoo.com> writes:
> Tom Lane wrote:
>> if count(*) = 0 from Room where roomno = new.roomno then
>> raise exception ''Room % does not exist'', new.roomno;
>> end if;
>>
>> Is this really intended to be a feature?
> I have to admit it was less an intention than more a side effect of the
> actual implementation. It was so easy to simply stick "SELECT " in front
> of "everything between IF and THEN" and expect the result to be a boolean.
Sure, it was easy given a certain implementation technique. Question
is, do we want to consider it a supported feature even if it makes the
implementation hard?
> In the same way you can do
> varname := count(*) from Room where roomno = new.roomno;
This actually doesn't bother me; I see it as simply a variant syntax
for SELECT INTO. (Perhaps it should be documented that way.)
If we want to preserve this behavior for IF et al, I don't think there
is any practical way to apply SQL-level type coercion as I had wanted.
We could instead make the code act like it's assigning to a plpgsql
boolean variable --- but it will apply plpgsql's textual conversion
methods, not SQL type coercion.
regards, tom lane
I said:
> If we want to preserve this behavior for IF et al, I don't think there
> is any practical way to apply SQL-level type coercion as I had wanted.
> We could instead make the code act like it's assigning to a plpgsql
> boolean variable --- but it will apply plpgsql's textual conversion
> methods, not SQL type coercion.
I have now written and tested the patch for this --- it's doing what I
originally called option 3:
>>> 3. Try to convert nonbooleans to boolean using plpgsql's usual method
>>> for cross-type coercion, ie run the type's output proc to get a
>>> string and feed it to bool's input proc. (This seems unlikely to
>>> avoid throwing an error in very many cases, but it'd be the most
>>> consistent with other parts of plpgsql.)
Unless someone objects, I'll commit this.
regards, tom lane