plpgsql EXECUTE will not set FOUND

Started by Dimitri Fontaineabout 16 years ago15 messages
#1Dimitri Fontaine
dfontaine@hi-media.com

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

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dimitri Fontaine (#1)
Re: plpgsql EXECUTE will not set FOUND

Dimitri Fontaine <dfontaine@hi-media.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

#3Dimitri Fontaine
dfontaine@hi-media.com
In reply to: Tom Lane (#2)
Re: plpgsql EXECUTE will not set FOUND

Tom Lane <tgl@sss.pgh.pa.us> writes:

Dimitri Fontaine <dfontaine@hi-media.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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dimitri Fontaine (#3)
Re: plpgsql EXECUTE will not set FOUND

Dimitri Fontaine <dfontaine@hi-media.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

#5Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#4)
Re: plpgsql EXECUTE will not set FOUND

On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Dimitri Fontaine <dfontaine@hi-media.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

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Robert Haas (#5)
Re: plpgsql EXECUTE will not set FOUND

Robert Haas wrote:

On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Dimitri Fontaine <dfontaine@hi-media.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

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#5)
Re: plpgsql EXECUTE will not set FOUND

Robert Haas <robertmhaas@gmail.com> writes:

On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl@sss.pgh.pa.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

#8Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Tom Lane (#7)
Re: plpgsql EXECUTE will not set FOUND

Tom Lane <tgl@sss.pgh.pa.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

#9Merlin Moncure
mmoncure@gmail.com
In reply to: Kevin Grittner (#8)
Re: plpgsql EXECUTE will not set FOUND

On Fri, Oct 23, 2009 at 11:07 AM, Kevin Grittner
<Kevin.Grittner@wicourts.gov> wrote:

Tom Lane <tgl@sss.pgh.pa.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

#10Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#7)
Re: plpgsql EXECUTE will not set FOUND

On Fri, Oct 23, 2009 at 10:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl@sss.pgh.pa.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

#11Pavel Stehule
pavel.stehule@gmail.com
In reply to: Robert Haas (#10)
Re: plpgsql EXECUTE will not set FOUND

2009/10/23 Robert Haas <robertmhaas@gmail.com>:

On Fri, Oct 23, 2009 at 10:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl@sss.pgh.pa.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

Show quoted text

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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12Robert Haas
robertmhaas@gmail.com
In reply to: Pavel Stehule (#11)
Re: plpgsql EXECUTE will not set FOUND

On Fri, Oct 23, 2009 at 12:05 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

2009/10/23 Robert Haas <robertmhaas@gmail.com>:

On Fri, Oct 23, 2009 at 10:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl@sss.pgh.pa.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

#13Bruce Momjian
bruce@momjian.us
In reply to: Robert Haas (#12)
1 attachment(s)
Re: plpgsql EXECUTE will not set FOUND

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.stehule@gmail.com> wrote:

2009/10/23 Robert Haas <robertmhaas@gmail.com>:

On Fri, Oct 23, 2009 at 10:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Fri, Oct 23, 2009 at 9:52 AM, Tom Lane <tgl@sss.pgh.pa.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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Attachments:

/rtmp/difftext/x-diffDownload
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.
      </para>
  
     </sect2>
#14David Fetter
david@fetter.org
In reply to: Bruce Momjian (#13)
Re: plpgsql EXECUTE will not set FOUND

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@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.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

#15Bruce Momjian
bruce@momjian.us
In reply to: David Fetter (#14)
Re: plpgsql EXECUTE will not set FOUND

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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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