Re: plpgsql EXECUTE will not set FOUND

But it will set GET DIAGNOSTIC ... = ROW_COUNT, am I being told on IRC.

I was really suprised FOUND is not set by EXECUTE in 8.3 when doing a
partitioning UPDATE trigger (we partition a summary table and have to
see about doing UPSERT).

As I wouldn't have figured GET DIAGNOSTIC was the way to go, I had to
resort to unclean method, namely EXECUTE ... RETURNING ... INTO then
checking the variable against NULL.

Why isn't that a bug again, that GET DIAGNOSTIC and FOUND are not
exposing the same reality?
--
dim

Dimitri Fontaine <dfontaine(at)hi-media(dot)com> writes:
> But it will set GET DIAGNOSTIC ... = ROW_COUNT, am I being told on IRC.

This has been discussed before, please read archives.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> Dimitri Fontaine <dfontaine(at)hi-media(dot)com> writes:
>> But it will set GET DIAGNOSTIC ... = ROW_COUNT, am I being told on IRC.
>
> This has been discussed before, please read archives.

The thread I found is pretty content free as far as answering my
question goes.
http://archives.postgresql.org/pgsql-general/2009-06/msg00208.php

I'll go search for more, meantime I'll just add the main goal of this
new thread is to have -hackers know there is a real user demand for
having EXECUTE set FOUND the same way it sets GET DIAGNOSTIC.

Regards,
--
dim

Dimitri Fontaine <dfontaine(at)hi-media(dot)com> writes:
> I'll go search for more, meantime I'll just add the main goal of this
> new thread is to have -hackers know there is a real user demand for
> having EXECUTE set FOUND the same way it sets GET DIAGNOSTIC.

[shrug...] There is also real user demand for not silently breaking
code that works now, which is what we risk anytime we change the set
of statements that can set FOUND.

regards, tom lane

On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Dimitri Fontaine <dfontaine(at)hi-media(dot)com> writes:
>> I'll go search for more, meantime I'll just add the main goal of this
>> new thread is to have -hackers know there is a real user demand for
>> having EXECUTE set FOUND the same way it sets GET DIAGNOSTIC.
>
> [shrug...]  There is also real user demand for not silently breaking
> code that works now, which is what we risk anytime we change the set
> of statements that can set FOUND.

We've had this discussion before and I'm still unpersuaded by your
position. I *never* write "IF FOUND THEN" except immediately after
the statement where I expect that variable to be set, and I submit
that anyone who who does write code that relies on certain statements
not setting FOUND is, IMO, depending on a bug. We don't and shouldn't
have a policy of making future PostgreSQL releases bug-compatible with
previous releases.

...Robert

Robert Haas wrote:
> On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>> Dimitri Fontaine <dfontaine(at)hi-media(dot)com> writes:
>>
>>> I'll go search for more, meantime I'll just add the main goal of this
>>> new thread is to have -hackers know there is a real user demand for
>>> having EXECUTE set FOUND the same way it sets GET DIAGNOSTIC.
>>>
>> [shrug...] There is also real user demand for not silently breaking
>> code that works now, which is what we risk anytime we change the set
>> of statements that can set FOUND.
>>
>
> We've had this discussion before and I'm still unpersuaded by your
> position. I *never* write "IF FOUND THEN" except immediately after
> the statement where I expect that variable to be set, and I submit
> that anyone who who does write code that relies on certain statements
> not setting FOUND is, IMO, depending on a bug. We don't and shouldn't
> have a policy of making future PostgreSQL releases bug-compatible with
> previous releases.
>
>
>

I agree that doing that is bad practice. I disagree that it's a bug.
And if it is, and we change it, then locating all the places where the
bug might occur will be a nightmare. In effect it means you'll need to
review every single use of FOUND in your code (possibly hundreds of
thousands or millions of lines) to make sure you're not accidentally
relying on the behaviour. No thanks.

cheers

andrew

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> [shrug...] There is also real user demand for not silently breaking
>> code that works now, which is what we risk anytime we change the set
>> of statements that can set FOUND.

> We've had this discussion before and I'm still unpersuaded by your
> position. I *never* write "IF FOUND THEN" except immediately after
> the statement where I expect that variable to be set, and I submit
> that anyone who who does write code that relies on certain statements
> not setting FOUND is, IMO, depending on a bug. We don't and shouldn't
> have a policy of making future PostgreSQL releases bug-compatible with
> previous releases.

This position is nonsense for two reasons:

1. It can hardly be considered a bug that FOUND is set only by the
statements that the documentation specifically states are the only ones
it is set by.

2. In order to use FOUND *at all* you must assume that it has got some
amount of stability. "IF FOUND" is already assuming that the "IF"
statement didn't reset the flag before evaluating the expression.
Lots of other perfectly reasonable constructions assume that FOUND
will stay stable across "no op" statements.

Any change here is *not* a bug fix, it is a change of clearly documented
and not-obviously-unreasonable behavior. We have to take seriously the
likelihood that it will break existing code. If there were a way to
flag such breakage I would be happier about changing it; but as Andrew
already noted, there doesn't seem to be any way to find affected code
except painful manual review.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Any change here is *not* a bug fix, it is a change of clearly
> documented and not-obviously-unreasonable behavior. We have to take
> seriously the likelihood that it will break existing code.

Perhaps plpgsql could support tests of SQLSTATE, and recognize '02000'
(the standard value for "zero rows affected") to support the desired
new semantics?

-Kevin

On Fri, Oct 23, 2009 at 11:07 AM, Kevin Grittner
<Kevin(dot)Grittner(at)wicourts(dot)gov> wrote:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>> Any change here is *not* a bug fix, it is a change of clearly
>> documented and not-obviously-unreasonable behavior.  We have to take
>> seriously the likelihood that it will break existing code.
>
> Perhaps plpgsql could support tests of SQLSTATE, and recognize '02000'
> (the standard value for "zero rows affected") to support the desired
> new semantics?

+1

I rarely use found because it's dangerous ...would be nice to have a
more rigorous test...

merlin

On Fri, Oct 23, 2009 at 10:50 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> [shrug...]  There is also real user demand for not silently breaking
>>> code that works now, which is what we risk anytime we change the set
>>> of statements that can set FOUND.
>
>> We've had this discussion before and I'm still unpersuaded by your
>> position.  I *never* write "IF FOUND THEN" except immediately after
>> the statement where I expect that variable to be set, and I submit
>> that anyone who who does write code that relies on certain statements
>> not setting FOUND is, IMO, depending on a bug.  We don't and shouldn't
>> have a policy of making future PostgreSQL releases bug-compatible with
>> previous releases.
>
> This position is nonsense for two reasons:
>
> 1. It can hardly be considered a bug that FOUND is set only by the
> statements that the documentation specifically states are the only ones
> it is set by.

OK, it's not a bug: it's a misfeature. :-)

> 2. In order to use FOUND *at all* you must assume that it has got some
> amount of stability.  "IF FOUND" is already assuming that the "IF"
> statement didn't reset the flag before evaluating the expression.
> Lots of other perfectly reasonable constructions assume that FOUND
> will stay stable across "no op" statements.

Sure. I think there's a big difference between assuming that the word
IF (or the intervening semicolon and/or whitespace) did not reset
FOUND and assuming that it will not be reset by the execution of a
dynamic SQL query. The former is necessary for there to be any
conceivable way of using FOUND; the latter is assuming that for some
reason we want to treat dynamic SQL queries differently than static
ones.

...Robert

2009/10/23 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Fri, Oct 23, 2009 at 10:50 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>> On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> [shrug...]  There is also real user demand for not silently breaking
>>>> code that works now, which is what we risk anytime we change the set
>>>> of statements that can set FOUND.
>>
>>> We've had this discussion before and I'm still unpersuaded by your
>>> position.  I *never* write "IF FOUND THEN" except immediately after
>>> the statement where I expect that variable to be set, and I submit
>>> that anyone who who does write code that relies on certain statements
>>> not setting FOUND is, IMO, depending on a bug.  We don't and shouldn't
>>> have a policy of making future PostgreSQL releases bug-compatible with
>>> previous releases.
>>
>> This position is nonsense for two reasons:
>>
>> 1. It can hardly be considered a bug that FOUND is set only by the
>> statements that the documentation specifically states are the only ones
>> it is set by.
>
> OK, it's not a bug: it's a misfeature.  :-)

Isn't this behave shared with PL/SQL? In some environments the dynamic
queries are external - so there wasn't possibility to get return
state. I afraid so somewhere this feature was extensively used - I
dislike this feature too, but I agree with Tom - this is small
problem, and it is better do nothing.

What about to add new flag to EXECUTE?

or create execute function, that returns found

like

execute('SELECT ....' INTO ... USING ...)?

it's obscure too.

Regards
Pavel Stehule

>
>> 2. In order to use FOUND *at all* you must assume that it has got some
>> amount of stability.  "IF FOUND" is already assuming that the "IF"
>> statement didn't reset the flag before evaluating the expression.
>> Lots of other perfectly reasonable constructions assume that FOUND
>> will stay stable across "no op" statements.
>
> Sure.  I think there's a big difference between assuming that the word
> IF (or the intervening semicolon and/or whitespace) did not reset
> FOUND and assuming that it will not be reset by the execution of a
> dynamic SQL query.  The former is necessary for there to be any
> conceivable way of using FOUND; the latter is assuming that for some
> reason we want to treat dynamic SQL queries differently than static
> ones.
>
> ...Robert
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

On Fri, Oct 23, 2009 at 12:05 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> 2009/10/23 Robert Haas <robertmhaas(at)gmail(dot)com>:
>> On Fri, Oct 23, 2009 at 10:50 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>>> On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>>> [shrug...]  There is also real user demand for not silently breaking
>>>>> code that works now, which is what we risk anytime we change the set
>>>>> of statements that can set FOUND.
>>>
>>>> We've had this discussion before and I'm still unpersuaded by your
>>>> position.  I *never* write "IF FOUND THEN" except immediately after
>>>> the statement where I expect that variable to be set, and I submit
>>>> that anyone who who does write code that relies on certain statements
>>>> not setting FOUND is, IMO, depending on a bug.  We don't and shouldn't
>>>> have a policy of making future PostgreSQL releases bug-compatible with
>>>> previous releases.
>>>
>>> This position is nonsense for two reasons:
>>>
>>> 1. It can hardly be considered a bug that FOUND is set only by the
>>> statements that the documentation specifically states are the only ones
>>> it is set by.
>>
>> OK, it's not a bug: it's a misfeature.  :-)
>
> Isn't this behave shared with PL/SQL? In some environments the dynamic
> queries are external - so there wasn't possibility to get return
> state. I afraid so somewhere this feature was extensively used - I
> dislike this feature too, but I agree with Tom - this is small
> problem, and it is better do nothing.
>
> What about to add new flag to EXECUTE?
>
> or create execute function, that returns found
>
> like
>
> execute('SELECT ....' INTO ... USING ...)?
>
> it's obscure too.

Yeah, I mean, if the consensus is that we shouldn't change this, then
people will just have to work around it using some other method, like
GET DIAGNOSTICS. It's not really worth adding a whole separate way of
doing this just to set FOUND. However, it would be worth documenting
the workaround, because I can see where the OP was left scratching his
head.

...Robert

I have applied the attached patch to document that FOUND is not affected
by EXECUTE, while GET DIAGNOSTICS is.

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

Robert Haas wrote:
> On Fri, Oct 23, 2009 at 12:05 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> > 2009/10/23 Robert Haas <robertmhaas(at)gmail(dot)com>:
> >> On Fri, Oct 23, 2009 at 10:50 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> >>>> On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >>>>> [shrug...] ?There is also real user demand for not silently breaking
> >>>>> code that works now, which is what we risk anytime we change the set
> >>>>> of statements that can set FOUND.
> >>>
> >>>> We've had this discussion before and I'm still unpersuaded by your
> >>>> position. ?I *never* write "IF FOUND THEN" except immediately after
> >>>> the statement where I expect that variable to be set, and I submit
> >>>> that anyone who who does write code that relies on certain statements
> >>>> not setting FOUND is, IMO, depending on a bug. ?We don't and shouldn't
> >>>> have a policy of making future PostgreSQL releases bug-compatible with
> >>>> previous releases.
> >>>
> >>> This position is nonsense for two reasons:
> >>>
> >>> 1. It can hardly be considered a bug that FOUND is set only by the
> >>> statements that the documentation specifically states are the only ones
> >>> it is set by.
> >>
> >> OK, it's not a bug: it's a misfeature. ?:-)
> >
> > Isn't this behave shared with PL/SQL? In some environments the dynamic
> > queries are external - so there wasn't possibility to get return
> > state. I afraid so somewhere this feature was extensively used - I
> > dislike this feature too, but I agree with Tom - this is small
> > problem, and it is better do nothing.
> >
> > What about to add new flag to EXECUTE?
> >
> > or create execute function, that returns found
> >
> > like
> >
> > execute('SELECT ....' INTO ... USING ...)?
> >
> > it's obscure too.
>
> Yeah, I mean, if the consensus is that we shouldn't change this, then
> people will just have to work around it using some other method, like
> GET DIAGNOSTICS. It's not really worth adding a whole separate way of
> doing this just to set FOUND. However, it would be worth documenting
> the workaround, because I can see where the OP was left scratching his
> head.
>
> ...Robert
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Mon, Nov 09, 2009 at 09:01:14PM -0500, Bruce Momjian wrote:
>
> I have applied the attached patch to document that FOUND is not affected
> by EXECUTE, while GET DIAGNOSTICS is.

One minor nit here:

> Index: doc/src/sgml/plpgsql.sgml
> ===================================================================
> RCS file: /cvsroot/pgsql/doc/src/sgml/plpgsql.sgml,v
> retrieving revision 1.143
> diff -c -c -r1.143 plpgsql.sgml
> *** doc/src/sgml/plpgsql.sgml 29 Sep 2009 20:05:29 -0000 1.143
> --- doc/src/sgml/plpgsql.sgml 10 Nov 2009 01:55:33 -0000
> ***************
> *** 1366,1372 ****
>
> <literal>FOUND</literal> is a local variable within each
> <application>PL/pgSQL</application> function; any changes to it
> ! affect only the current function.
> </para>
>
> </sect2>
> --- 1366,1374 ----
>
> <literal>FOUND</literal> is a local variable within each
> <application>PL/pgSQL</application> function; any changes to it
> ! affect only the current function. <literal>FOUND</literal> is not
> ! affected by <literal>EXECUTE</literal>, while <command>GET
> ! DIAGNOSTICS</command> is effected.

The passive form of this phrase would be "is affected," instead of "is
effected," or (my preference) change to an active construction like:

<literal>EXECUTE</literal> changes the output of <command>GET
DIAGNOSTICS</command>, but does not change the state of
<literal>FOUND</literal>.

Cheers,
David (Strunk and White ftw! ;)
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

David Fetter wrote:
> On Mon, Nov 09, 2009 at 09:01:14PM -0500, Bruce Momjian wrote:
> >
> > I have applied the attached patch to document that FOUND is not affected
> > by EXECUTE, while GET DIAGNOSTICS is.
>
> One minor nit here:
>
> > Index: doc/src/sgml/plpgsql.sgml
> > ===================================================================
> > RCS file: /cvsroot/pgsql/doc/src/sgml/plpgsql.sgml,v
> > retrieving revision 1.143
> > diff -c -c -r1.143 plpgsql.sgml
> > *** doc/src/sgml/plpgsql.sgml 29 Sep 2009 20:05:29 -0000 1.143
> > --- doc/src/sgml/plpgsql.sgml 10 Nov 2009 01:55:33 -0000
> > ***************
> > *** 1366,1372 ****
> >
> > <literal>FOUND</literal> is a local variable within each
> > <application>PL/pgSQL</application> function; any changes to it
> > ! affect only the current function.
> > </para>
> >
> > </sect2>
> > --- 1366,1374 ----
> >
> > <literal>FOUND</literal> is a local variable within each
> > <application>PL/pgSQL</application> function; any changes to it
> > ! affect only the current function. <literal>FOUND</literal> is not
> > ! affected by <literal>EXECUTE</literal>, while <command>GET
> > ! DIAGNOSTICS</command> is effected.
>
> The passive form of this phrase would be "is affected," instead of "is
> effected," or (my preference) change to an active construction like:
>
> <literal>EXECUTE</literal> changes the output of <command>GET
> DIAGNOSTICS</command>, but does not change the state of
> <literal>FOUND</literal>.

Thank you so much. I was struggling with the phrasing myself. Updated.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +