Relax requirement for INTO with SELECT in pl/pgsql
Patch is trivial (see below), discussion is not :-).
I see no useful reason to require INTO when returning data with
SELECT. However, requiring queries to indicate not needing data via
PERFORM causes some annoyances:
*) converting routines back and forth between pl/pgsql and pl/sql
requires needless busywork and tends to cause errors to be thrown at
runtime
*) as much as possible, (keywords begin/end remain a problem),
pl/pgsql should be a superset of sql
*) it's much more likely to be burned by accidentally forgetting to
swap in PERFORM than to accidentally leave in a statement with no
actionable target. Even if you did so in the latter case, it stands
to reason you'd accidentally leave in the target variable, too.
*) the PERFORM requirement hails from the days when only statements
starting with SELECT return data. There is no PERFORM equivalent for
WITH/INSERT/DELETE/UPDATE and there are real world scenarios where you
might have a RETURNING clause that does something but not necessarily
want to place the result in a variable (for example passing to
volatile function). Take a look at the errhint() clause below -- we
don't even have a suggestion in that case.
This has come up before, and there was a fair amount of sympathy for
this argument albeit with some dissent -- notably Pavel. I'd like to
get a hearing on the issue -- thanks. If we decide to move forward,
this would effectively deprecate PERFORM and the documentation will be
suitably modified as well.
merlin
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index b7f44ca..a860066 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -3457,12 +3457,9 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
}
else
{
- /* If the statement returned a tuple table, complain */
+ /* If the statement returned a tuple table, free it. */
if (SPI_tuptable != NULL)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("query has no destination for result data"),
- (rc == SPI_OK_SELECT) ? errhint("If you want to
discard the results of a SELECT, use PERFORM instead.") : 0));
+ SPI_freetuptable(SPI_tuptable);
}if (paramLI)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi
2016-03-21 21:24 GMT+01:00 Merlin Moncure <mmoncure@gmail.com>:
Patch is trivial (see below), discussion is not :-).
I see no useful reason to require INTO when returning data with
SELECT. However, requiring queries to indicate not needing data via
PERFORM causes some annoyances:*) converting routines back and forth between pl/pgsql and pl/sql
requires needless busywork and tends to cause errors to be thrown at
runtime*) as much as possible, (keywords begin/end remain a problem),
pl/pgsql should be a superset of sql*) it's much more likely to be burned by accidentally forgetting to
swap in PERFORM than to accidentally leave in a statement with no
actionable target. Even if you did so in the latter case, it stands
to reason you'd accidentally leave in the target variable, too.*) the PERFORM requirement hails from the days when only statements
starting with SELECT return data. There is no PERFORM equivalent for
WITH/INSERT/DELETE/UPDATE and there are real world scenarios where you
might have a RETURNING clause that does something but not necessarily
want to place the result in a variable (for example passing to
volatile function). Take a look at the errhint() clause below -- we
don't even have a suggestion in that case.This has come up before, and there was a fair amount of sympathy for
this argument albeit with some dissent -- notably Pavel. I'd like to
get a hearing on the issue -- thanks. If we decide to move forward,
this would effectively deprecate PERFORM and the documentation will be
suitably modified as well.
My negative opinion is known. The PERFORM statement is much more workaround
than well designed statement, but I would to see ANSI/SQL based fix. I try
to compare benefits and loss.
Can you start with analyze what is possible, and what semantic is allowed
in standard and other well known SQL databases?
Regards
Pavel
Show quoted text
merlin
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index b7f44ca..a860066 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -3457,12 +3457,9 @@ exec_stmt_execsql(PLpgSQL_execstate *estate, } else { - /* If the statement returned a tuple table, complain */ + /* If the statement returned a tuple table, free it. */ if (SPI_tuptable != NULL) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("query has no destination for result data"), - (rc == SPI_OK_SELECT) ? errhint("If you want to discard the results of a SELECT, use PERFORM instead.") : 0)); + SPI_freetuptable(SPI_tuptable); }if (paramLI)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Mar 21, 2016 at 4:13 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
Hi
2016-03-21 21:24 GMT+01:00 Merlin Moncure <mmoncure@gmail.com>:
Patch is trivial (see below), discussion is not :-).
I see no useful reason to require INTO when returning data with
SELECT. However, requiring queries to indicate not needing data via
PERFORM causes some annoyances:*) converting routines back and forth between pl/pgsql and pl/sql
requires needless busywork and tends to cause errors to be thrown at
runtime*) as much as possible, (keywords begin/end remain a problem),
pl/pgsql should be a superset of sql*) it's much more likely to be burned by accidentally forgetting to
swap in PERFORM than to accidentally leave in a statement with no
actionable target. Even if you did so in the latter case, it stands
to reason you'd accidentally leave in the target variable, too.*) the PERFORM requirement hails from the days when only statements
starting with SELECT return data. There is no PERFORM equivalent for
WITH/INSERT/DELETE/UPDATE and there are real world scenarios where you
might have a RETURNING clause that does something but not necessarily
want to place the result in a variable (for example passing to
volatile function). Take a look at the errhint() clause below -- we
don't even have a suggestion in that case.This has come up before, and there was a fair amount of sympathy for
this argument albeit with some dissent -- notably Pavel. I'd like to
get a hearing on the issue -- thanks. If we decide to move forward,
this would effectively deprecate PERFORM and the documentation will be
suitably modified as well.My negative opinion is known. The PERFORM statement is much more workaround
than well designed statement, but I would to see ANSI/SQL based fix. I try
to compare benefits and loss.
Well, pl/pgsql is based on oracle pl/sql so I don't see how the
standard is involved. FWICT, "PERFORM" is a postgres extension to
pl/pgsql. I don't see how the standard plays at all.
Can you start with analyze what is possible, and what semantic is allowed in
standard and other well known SQL databases?
Typical use of PERFORM is void returning function. Oracle allows use
of those functions without any decoration at all. For example, in
postgres we might do:
PERFORM LogIt('I did something');
in Oracle, you'd simply do:
LogIt('I did something');
I'm not sure what Oracle does for SELECT statements without INTO/BULK
UPDATE. I'm not really inclined to care -- I'm really curious to see
an argument where usage of PERFORM actually helps in some meaningful
way. Notably, SELECT without INTO is accepted syntax, but fails only
after running the query. I think that's pretty much stupid but it's
fair to say I'm not inventing syntax, only disabling the error.
I'm not sure what other databases do is relevant. They use other
procedure languages than pl//sql (the biggest players are pl/psm and
t-sql) which have a different set of rules in terms of passing
variables in and out of queries.
merlin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/21/16 5:03 PM, Merlin Moncure wrote:
in Oracle, you'd simply do:
LogIt('I did something');
It would be *great* if we could support that in plpgsql.
I'm not sure what Oracle does for SELECT statements without INTO/BULK
UPDATE. I'm not really inclined to care -- I'm really curious to see
an argument where usage of PERFORM actually helps in some meaningful
way. Notably, SELECT without INTO is accepted syntax, but fails only
after running the query. I think that's pretty much stupid but it's
fair to say I'm not inventing syntax, only disabling the error.
I don't think it buys much at all.
While we're on the subject, it'd be great if variable := SELECT ...
worked too.
I'm not sure what other databases do is relevant. They use other
procedure languages than pl//sql (the biggest players are pl/psm and
t-sql) which have a different set of rules in terms of passing
variables in and out of queries.
+1
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
On 3/21/16 5:03 PM, Merlin Moncure wrote:
in Oracle, you'd simply do:
LogIt('I did something');
It would be *great* if we could support that in plpgsql.
FWIW, I'm hesitant to just start accepting that syntax as if it were an
equivalent to "SELECT f(x)" or "PERFORM f(x)". I think that someday
we will want to have the ability to have pass-by-reference parameters
to functions, and that's a fundamentally incompatible behavior so it
ought to be invoked by a new syntax. I'd like to save "f(x)" as a
bare statement for that purpose. We could also consider inventing
"CALL f(x)"; but supposing that we already had both "CALL f(x)" (which
would allow x to be pass-by-ref and possibly modified) and "SELECT f(x)"
(which would not), which one would you assign bare "f(x)" to mean? It
should be "CALL f(x)", not least because that would be the semantics most
comparable to Oracle's behavior (since they have pass-by-ref parameters
already).
So, I'm -1 on not having any keyword at all. I have no objection
to Merlin's proposal though. I agree that PERFORM is starting to
look a bit silly, since it doesn't play with WITH for instance.
While we're on the subject, it'd be great if variable := SELECT ...
worked too.
It does, though you might need parens around the sub-select.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Mar 21, 2016 at 5:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
So, I'm -1 on not having any keyword at all. I have no objection
to Merlin's proposal though. I agree that PERFORM is starting to
look a bit silly, since it doesn't play with WITH for instance.
All right -- I'll submit a revised patch with documentation
adjustments. "Basic Statements" needs to be revised, in particular
the section, "Executing a Command With No Result". I'm inclined to
remove that section completely, and rename the next section from
"Executing a Query with a Single-row Result" to simply, "Executing a
Query" (or perhaps "Non-Dynamic Query").
I'd like to remove documentation and usage of PERFORM except for a
note mentioning the old syntax...this would replace the current note
that existentially questions the necessity of PERFORM in the first
place. Thanks.
merlin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Mar 21, 2016 at 6:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
So, I'm -1 on not having any keyword at all. I have no objection
to Merlin's proposal though. I agree that PERFORM is starting to
look a bit silly, since it doesn't play with WITH for instance.
Yeah, I think requiring PERFORM is stupid and annoying. +1 for
letting people write a SELECT with no target.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2016-03-21 23:03 GMT+01:00 Merlin Moncure <mmoncure@gmail.com>:
On Mon, Mar 21, 2016 at 4:13 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:Hi
2016-03-21 21:24 GMT+01:00 Merlin Moncure <mmoncure@gmail.com>:
Patch is trivial (see below), discussion is not :-).
I see no useful reason to require INTO when returning data with
SELECT. However, requiring queries to indicate not needing data via
PERFORM causes some annoyances:*) converting routines back and forth between pl/pgsql and pl/sql
requires needless busywork and tends to cause errors to be thrown at
runtime*) as much as possible, (keywords begin/end remain a problem),
pl/pgsql should be a superset of sql*) it's much more likely to be burned by accidentally forgetting to
swap in PERFORM than to accidentally leave in a statement with no
actionable target. Even if you did so in the latter case, it stands
to reason you'd accidentally leave in the target variable, too.*) the PERFORM requirement hails from the days when only statements
starting with SELECT return data. There is no PERFORM equivalent for
WITH/INSERT/DELETE/UPDATE and there are real world scenarios where you
might have a RETURNING clause that does something but not necessarily
want to place the result in a variable (for example passing to
volatile function). Take a look at the errhint() clause below -- we
don't even have a suggestion in that case.This has come up before, and there was a fair amount of sympathy for
this argument albeit with some dissent -- notably Pavel. I'd like to
get a hearing on the issue -- thanks. If we decide to move forward,
this would effectively deprecate PERFORM and the documentation will be
suitably modified as well.My negative opinion is known. The PERFORM statement is much more
workaround
than well designed statement, but I would to see ANSI/SQL based fix. I
try
to compare benefits and loss.
Well, pl/pgsql is based on oracle pl/sql so I don't see how the
standard is involved. FWICT, "PERFORM" is a postgres extension to
pl/pgsql. I don't see how the standard plays at all.
PERFORM is not interesting - it is proprietary extension.
Can you start with analyze what is possible, and what semantic is
allowed in
standard and other well known SQL databases?
Typical use of PERFORM is void returning function. Oracle allows use
of those functions without any decoration at all. For example, in
postgres we might do:
PERFORM LogIt('I did something');in Oracle, you'd simply do:
LogIt('I did something');
It is procedure call - it is not SELECT fx(), but CALL fx(), when CALL
statement is implicit.
I'm not sure what Oracle does for SELECT statements without INTO/BULK
UPDATE. I'm not really inclined to care -- I'm really curious to see
an argument where usage of PERFORM actually helps in some meaningful
way. Notably, SELECT without INTO is accepted syntax, but fails only
after running the query. I think that's pretty much stupid but it's
fair to say I'm not inventing syntax, only disabling the error.
I'm not sure what other databases do is relevant. They use other
procedure languages than pl//sql (the biggest players are pl/psm and
t-sql) which have a different set of rules in terms of passing
variables in and out of queries.
But this is important, and you are ignoring this case. If we allow "SELECT
expr;" when result will be ignored once, we cannot to revert it back ever.
So this can be really issue, when you will port applications between
Postgres and other, or if you will switch between Postgres and other db.
Regards
Pavel
Show quoted text
merlin
2016-03-21 23:26 GMT+01:00 Jim Nasby <Jim.Nasby@bluetreble.com>:
On 3/21/16 5:03 PM, Merlin Moncure wrote:
in Oracle, you'd simply do:
LogIt('I did something');It would be *great* if we could support that in plpgsql.
I'm not sure what Oracle does for SELECT statements without INTO/BULK
UPDATE. I'm not really inclined to care -- I'm really curious to see
an argument where usage of PERFORM actually helps in some meaningful
way. Notably, SELECT without INTO is accepted syntax, but fails only
after running the query. I think that's pretty much stupid but it's
fair to say I'm not inventing syntax, only disabling the error.I don't think it buys much at all.
While we're on the subject, it'd be great if variable := SELECT ... worked
too.
We are support
var := (query expression)
and this syntax is required and supported by ANSI/SQL - there are no any
reason to support other proprietary extension.
Regards
Pavel
Show quoted text
I'm not sure what other databases do is relevant. They use other
procedure languages than pl//sql (the biggest players are pl/psm and
t-sql) which have a different set of rules in terms of passing
variables in and out of queries.+1
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
2016-03-21 23:49 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
On 3/21/16 5:03 PM, Merlin Moncure wrote:
in Oracle, you'd simply do:
LogIt('I did something');It would be *great* if we could support that in plpgsql.
FWIW, I'm hesitant to just start accepting that syntax as if it were an
equivalent to "SELECT f(x)" or "PERFORM f(x)". I think that someday
we will want to have the ability to have pass-by-reference parameters
to functions, and that's a fundamentally incompatible behavior so it
ought to be invoked by a new syntax. I'd like to save "f(x)" as a
bare statement for that purpose. We could also consider inventing
"CALL f(x)"; but supposing that we already had both "CALL f(x)" (which
would allow x to be pass-by-ref and possibly modified) and "SELECT f(x)"
(which would not), which one would you assign bare "f(x)" to mean? It
should be "CALL f(x)", not least because that would be the semantics most
comparable to Oracle's behavior (since they have pass-by-ref parameters
already).
I can live with SELECT fx(x). It is little bit dangerous, but this risk can
be easy detected by plpgsql_check.
So, I'm -1 on not having any keyword at all. I have no objection
to Merlin's proposal though. I agree that PERFORM is starting to
look a bit silly, since it doesn't play with WITH for instance.
Isn't time to fix PERFORM instead?
Show quoted text
While we're on the subject, it'd be great if variable := SELECT ...
worked too.It does, though you might need parens around the sub-select.
regards, tom lane
Pavel Stehule <pavel.stehule@gmail.com> writes:
I can live with SELECT fx(x). It is little bit dangerous, but this risk can
be easy detected by plpgsql_check.
Dangerous how?
So, I'm -1 on not having any keyword at all. I have no objection
to Merlin's proposal though. I agree that PERFORM is starting to
look a bit silly, since it doesn't play with WITH for instance.
Isn't time to fix PERFORM instead?
I do not think it can be fixed without embedding knowledge of PERFORM into
the core parser, which I doubt anybody would consider a good idea.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2016-03-22 6:06 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
Pavel Stehule <pavel.stehule@gmail.com> writes:
I can live with SELECT fx(x). It is little bit dangerous, but this risk
can
be easy detected by plpgsql_check.
Dangerous how?
I afraid of useless and forgotten call of functions. But the risk is same
like PERFORM - so this is valid from one half. The PERFORM statement holds
special semantic, and it is interesting.
But I don't see any risk if we allow SELECT fx(x) without INTO when fx is
void function. It is absolutely correct.
So, I'm -1 on not having any keyword at all. I have no objection
to Merlin's proposal though. I agree that PERFORM is starting to
look a bit silly, since it doesn't play with WITH for instance.Isn't time to fix PERFORM instead?
I do not think it can be fixed without embedding knowledge of PERFORM into
the core parser, which I doubt anybody would consider a good idea.
I don't see, why PERFORM should be in core parser? What use case should be
fixed?
Regards
Pavel
Show quoted text
regards, tom lane
On Tue, Mar 22, 2016 at 12:20 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
2016-03-22 6:06 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
Pavel Stehule <pavel.stehule@gmail.com> writes:
I can live with SELECT fx(x). It is little bit dangerous, but this risk
can
be easy detected by plpgsql_check.Dangerous how?
I afraid of useless and forgotten call of functions. But the risk is same
like PERFORM - so this is valid from one half. The PERFORM statement holds
special semantic, and it is interesting.
I see your point here, but the cost of doing that far outweighs the
risks. And I don't think the arbitrary standard of defining forcing
the user to identify if the query should return data is a good way of
identifying dead code.
Furthermore, after reviewing the docs, it's clear to me they've been
wrong for about a bazillion years. In a couple of places they
mandated the use of PERFORM when the query returned rows, but it had
to be used even when the query was capable of returning rows
(regardless if it did or not).
Anyways, here's the patch with documentation adjustments as promised.
I ended up keeping the 'without result' section because it contained
useful information about plan caching,
merlin
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index 9786242..512eaa7
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*************** my_record.user_id := 20;
*** 904,910 ****
<title>Executing a Command With No Result</title> <para>
! For any SQL command that does not return rows, for example
<command>INSERT</> without a <literal>RETURNING</> clause, you can
execute the command within a <application>PL/pgSQL</application> function
just by writing the command.
--- 904,910 ----
<title>Executing a Command With No Result</title> <para>
! For any SQL command, for example
<command>INSERT</> without a <literal>RETURNING</> clause, you can
execute the command within a <application>PL/pgSQL</application> function
just by writing the command.
*************** my_record.user_id := 20;
*** 925,972 ****
<xref linkend="plpgsql-plan-caching">.
</para>
- <para>
- Sometimes it is useful to evaluate an expression or <command>SELECT</>
- query but discard the result, for example when calling a function
- that has side-effects but no useful result value. To do
- this in <application>PL/pgSQL</application>, use the
- <command>PERFORM</command> statement:
-
- <synopsis>
- PERFORM <replaceable>query</replaceable>;
- </synopsis>
-
- This executes <replaceable>query</replaceable> and discards the
- result. Write the <replaceable>query</replaceable> the same
- way you would write an SQL <command>SELECT</> command, but replace the
- initial keyword <command>SELECT</> with <command>PERFORM</command>.
- For <command>WITH</> queries, use <command>PERFORM</> and then
- place the query in parentheses. (In this case, the query can only
- return one row.)
- <application>PL/pgSQL</application> variables will be
- substituted into the query just as for commands that return no result,
- and the plan is cached in the same way. Also, the special variable
- <literal>FOUND</literal> is set to true if the query produced at
- least one row, or false if it produced no rows (see
- <xref linkend="plpgsql-statements-diagnostics">).
- </para>
-
<note>
<para>
! One might expect that writing <command>SELECT</command> directly
! would accomplish this result, but at
! present the only accepted way to do it is
! <command>PERFORM</command>. A SQL command that can return rows,
! such as <command>SELECT</command>, will be rejected as an error
! unless it has an <literal>INTO</> clause as discussed in the
! next section.
</para>
</note>
<para>
An example:
<programlisting>
! PERFORM create_mv('cs_session_page_requests_mv', my_query);
</programlisting>
</para>
</sect2>
--- 925,944 ----
<xref linkend="plpgsql-plan-caching">.
</para> <note>
<para>
! In older versions of PostgreSQL, it was mandatory to use
! <command>PERFORM</command> instead of <command>SELECT</command>
! when the query could return data that was not captured into
! variables. This requirement has been relaxed and usage of
! <command>PERFORM</command> has been deprecated.
</para>
</note>
<para>
An example:
<programlisting>
! SELECT create_mv('cs_session_page_requests_mv', my_query);
</programlisting>
</para>
</sect2>
*************** GET DIAGNOSTICS integer_var = ROW_COUNT;
*** 1480,1491 ****
</listitem>
<listitem>
<para>
! A <command>PERFORM</> statement sets <literal>FOUND</literal>
true if it produces (and discards) one or more rows, false if
no row is produced.
</para>
</listitem>
<listitem>
<para>
<command>UPDATE</>, <command>INSERT</>, and <command>DELETE</>
statements set <literal>FOUND</literal> true if at least one
--- 1452,1471 ----
</listitem>
<listitem>
<para>
! A <command>SELECT</> statement sets <literal>FOUND</literal>
true if it produces (and discards) one or more rows, false if
no row is produced.
</para>
</listitem>
<listitem>
+ <para>
+ A <command>PERFORM</> statement sets <literal>FOUND</literal>
+ true if it produces (and discards) one or more rows, false if
+ no row is produced. This statement is equivalent to
+ <command>SELECT</> without INTO.
+ </para>
+ </listitem>
+ <listitem>
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index 9786242..512eaa7
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*************** my_record.user_id := 20;
*** 904,910 ****
<title>Executing a Command With No Result</title> <para>
! For any SQL command that does not return rows, for example
<command>INSERT</> without a <literal>RETURNING</> clause, you can
execute the command within a <application>PL/pgSQL</application> function
just by writing the command.
--- 904,910 ----
<title>Executing a Command With No Result</title> <para>
! For any SQL command, for example
<command>INSERT</> without a <literal>RETURNING</> clause, you can
execute the command within a <application>PL/pgSQL</application> function
just by writing the command.
*************** my_record.user_id := 20;
*** 925,972 ****
<xref linkend="plpgsql-plan-caching">.
</para>
- <para>
- Sometimes it is useful to evaluate an expression or <command>SELECT</>
- query but discard the result, for example when calling a function
- that has side-effects but no useful result value. To do
- this in <application>PL/pgSQL</application>, use the
- <command>PERFORM</command> statement:
-
- <synopsis>
- PERFORM <replaceable>query</replaceable>;
- </synopsis>
-
- This executes <replaceable>query</replaceable> and discards the
- result. Write the <replaceable>query</replaceable> the same
- way you would write an SQL <command>SELECT</> command, but replace the
- initial keyword <command>SELECT</> with <command>PERFORM</command>.
- For <command>WITH</> queries, use <command>PERFORM</> and then
- place the query in parentheses. (In this case, the query can only
- return one row.)
- <application>PL/pgSQL</application> variables will be
- substituted into the query just as for commands that return no result,
- and the plan is cached in the same way. Also, the special variable
- <literal>FOUND</literal> is set to true if the query produced at
- least one row, or false if it produced no rows (see
- <xref linkend="plpgsql-statements-diagnostics">).
- </para>
-
<note>
<para>
! One might expect that writing <command>SELECT</command> directly
! would accomplish this result, but at
! present the only accepted way to do it is
! <command>PERFORM</command>. A SQL command that can return rows,
! such as <command>SELECT</command>, will be rejected as an error
! unless it has an <literal>INTO</> clause as discussed in the
! next section.
</para>
</note>
<para>
An example:
<programlisting>
! PERFORM create_mv('cs_session_page_requests_mv', my_query);
</programlisting>
</para>
</sect2>
--- 925,944 ----
<xref linkend="plpgsql-plan-caching">.
</para> <note>
<para>
! In older versions of PostgreSQL, it was mandatory to use
! <command>PERFORM</command> instead of <command>SELECT</command>
! when the query could return data that was not captured into
! variables. This requirement has been relaxed and usage of
! <command>PERFORM</command> has been deprecated.
</para>
</note>
<para>
An example:
<programlisting>
! SELECT create_mv('cs_session_page_requests_mv', my_query);
</programlisting>
</para>
</sect2>
*************** GET DIAGNOSTICS integer_var = ROW_COUNT;
*** 1480,1491 ****
</listitem>
<listitem>
<para>
! A <command>PERFORM</> statement sets <literal>FOUND</literal>
true if it produces (and discards) one or more rows, false if
no row is produced.
</para>
</listitem>
<listitem>
<para>
<command>UPDATE</>, <command>INSERT</>, and <command>DELETE</>
statements set <literal>FOUND</literal> true if at least one
--- 1452,1471 ----
</listitem>
<listitem>
<para>
! A <command>SELECT</> statement sets <literal>FOUND</literal>
true if it produces (and discards) one or more rows, false if
no row is produced.
</para>
</listitem>
<listitem>
+ <para>
+ A <command>PERFORM</> statement sets <literal>FOUND</literal>
+ true if it produces (and discards) one or more rows, false if
+ no row is produced. This statement is equivalent to
+ <command>SELECT</> without INTO.
+ </para>
+ </listitem>
+ <listitem>
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index 9786242..512eaa7
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*************** my_record.user_id := 20;
*** 904,910 ****
<title>Executing a Command With No Result</title> <para>
! For any SQL command that does not return rows, for example
<command>INSERT</> without a <literal>RETURNING</> clause, you can
execute the command within a <application>PL/pgSQL</application> function
just by writing the command.
--- 904,910 ----
<title>Executing a Command With No Result</title> <para>
! For any SQL command, for example
<command>INSERT</> without a <literal>RETURNING</> clause, you can
execute the command within a <application>PL/pgSQL</application> function
just by writing the command.
*************** my_record.user_id := 20;
*** 925,972 ****
<xref linkend="plpgsql-plan-caching">.
</para>
- <para>
- Sometimes it is useful to evaluate an expression or <command>SELECT</>
- query but discard the result, for example when calling a function
- that has side-effects but no useful result value. To do
- this in <application>PL/pgSQL</application>, use the
- <command>PERFORM</command> statement:
-
- <synopsis>
- PERFORM <replaceable>query</replaceable>;
- </synopsis>
-
- This executes <replaceable>query</replaceable> and discards the
- result. Write the <replaceable>query</replaceable> the same
- way you would write an SQL <command>SELECT</> command, but replace the
- initial keyword <command>SELECT</> with <command>PERFORM</command>.
- For <command>WITH</> queries, use <command>PERFORM</> and then
- place the query in parentheses. (In this case, the query can only
- return one row.)
- <application>PL/pgSQL</application> variables will be
- substituted into the query just as for commands that return no result,
- and the plan is cached in the same way. Also, the special variable
- <literal>FOUND</literal> is set to true if the query produced at
- least one row, or false if it produced no rows (see
- <xref linkend="plpgsql-statements-diagnostics">).
- </para>
-
<note>
<para>
! One might expect that writing <command>SELECT</command> directly
! would accomplish this result, but at
! present the only accepted way to do it is
! <command>PERFORM</command>. A SQL command that can return rows,
! such as <command>SELECT</command>, will be rejected as an error
! unless it has an <literal>INTO</> clause as discussed in the
! next section.
</para>
</note>
<para>
An example:
<programlisting>
! PERFORM create_mv('cs_session_page_requests_mv', my_query);
</programlisting>
</para>
</sect2>
--- 925,944 ----
<xref linkend="plpgsql-plan-caching">.
</para> <note>
<para>
! In older versions of PostgreSQL, it was mandatory to use
! <command>PERFORM</command> instead of <command>SELECT</command>
! when the query could return data that was not captured into
! variables. This requirement has been relaxed and usage of
! <command>PERFORM</command> has been deprecated.
</para>
</note>
<para>
An example:
<programlisting>
! SELECT create_mv('cs_session_page_requests_mv', my_query);
</programlisting>
</para>
</sect2>
*************** GET DIAGNOSTICS integer_var = ROW_COUNT;
*** 1480,1491 ****
</listitem>
<listitem>
<para>
! A <command>PERFORM</> statement sets <literal>FOUND</literal>
true if it produces (and discards) one or more rows, false if
no row is produced.
</para>
</listitem>
<listitem>
<para>
<command>UPDATE</>, <command>INSERT</>, and <command>DELETE</>
statements set <literal>FOUND</literal> true if at least one
--- 1452,1471 ----
</listitem>
<listitem>
<para>
! A <command>SELECT</> statement sets <literal>FOUND</literal>
true if it produces (and discards) one or more rows, false if
no row is produced.
</para>
</listitem>
<listitem>
+ <para>
+ A <command>PERFORM</> statement sets <literal>FOUND</literal>
+ true if it produces (and discards) one or more rows, false if
+ no row is produced. This statement is equivalent to
+ <command>SELECT</> without INTO.
+ </para>
+ </listitem>
+ <listitem>
<para>
<command>UPDATE</>, <command>INSERT</>, and <command>DELETE</>
statements set <literal>FOUND</literal> true if at least one
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
new file mode 100644
index b63ecac..975e8fe
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
*************** exec_stmt_assign(PLpgSQL_execstate *esta
*** 1557,1562 ****
--- 1557,1563 ----
* exec_stmt_perform Evaluate query and discard result (but set
* FOUND depending on whether at least one row
* was returned).
+ * This syntax is deprecated.
* ----------
*/
static int
*************** exec_stmt_execsql(PLpgSQL_execstate *est
*** 3647,3658 ****
}
else
{
! /* If the statement returned a tuple table, complain */
if (SPI_tuptable != NULL)
! ereport(ERROR,
! (errcode(ERRCODE_SYNTAX_ERROR),
! errmsg("query has no destination for result data"),
! (rc == SPI_OK_SELECT) ? errhint("If you want to
discard the results of a SELECT, use PERFORM instead.") : 0));
} return PLPGSQL_RC_OK;
--- 3648,3656 ----
}
else
{
! /* If the statement returned a tuple table without INTO, free it. */
if (SPI_tuptable != NULL)
! SPI_freetuptable(SPI_tuptable);
}return PLPGSQL_RC_OK;
merlin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/22/16 8:37 AM, Merlin Moncure wrote:
I afraid of useless and forgotten call of functions. But the risk is same
like PERFORM - so this is valid from one half. The PERFORM statement holds
special semantic, and it is interesting.I see your point here, but the cost of doing that far outweighs the
risks. And I don't think the arbitrary standard of defining forcing
the user to identify if the query should return data is a good way of
identifying dead code.
Not to mention that there's tons of other ways to introduce unintended
inefficiencies. Off the top of my head, declaring variables that are
never referenced and have no assignment is a big one.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Mar 23, 2016 at 10:57 AM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
On 3/22/16 8:37 AM, Merlin Moncure wrote:
I afraid of useless and forgotten call of functions. But the risk is same
like PERFORM - so this is valid from one half. The PERFORM statement
holds
special semantic, and it is interesting.I see your point here, but the cost of doing that far outweighs the
risks. And I don't think the arbitrary standard of defining forcing
the user to identify if the query should return data is a good way of
identifying dead code.Not to mention that there's tons of other ways to introduce unintended
inefficiencies. Off the top of my head, declaring variables that are never
referenced and have no assignment is a big one.
Yes. This comes up most often with dblink for me. Mainly because of
the slight wonkiness of dblink API, but totally agree this should
never have to be done. Anyways, I submitted the patch to the next
open commit fest.
Totally off topic aside: are you in dallas for the next PUG? I
unfortunately missed the last one and will likely miss the next one
too -- I coach the company kickball team and we play on Wednesdays --
oh well. Maybe beers afterwards?
merlin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi
2016-03-21 22:13 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
2016-03-21 21:24 GMT+01:00 Merlin Moncure <mmoncure@gmail.com>:
Patch is trivial (see below), discussion is not :-).
I see no useful reason to require INTO when returning data with
SELECT. However, requiring queries to indicate not needing data via
PERFORM causes some annoyances:*) converting routines back and forth between pl/pgsql and pl/sql
requires needless busywork and tends to cause errors to be thrown at
runtime*) as much as possible, (keywords begin/end remain a problem),
pl/pgsql should be a superset of sql*) it's much more likely to be burned by accidentally forgetting to
swap in PERFORM than to accidentally leave in a statement with no
actionable target. Even if you did so in the latter case, it stands
to reason you'd accidentally leave in the target variable, too.*) the PERFORM requirement hails from the days when only statements
starting with SELECT return data. There is no PERFORM equivalent for
WITH/INSERT/DELETE/UPDATE and there are real world scenarios where you
might have a RETURNING clause that does something but not necessarily
want to place the result in a variable (for example passing to
volatile function). Take a look at the errhint() clause below -- we
don't even have a suggestion in that case.This has come up before, and there was a fair amount of sympathy for
this argument albeit with some dissent -- notably Pavel. I'd like to
get a hearing on the issue -- thanks. If we decide to move forward,
this would effectively deprecate PERFORM and the documentation will be
suitably modified as well.
here is another argument why this idea is not good.
Now, when people coming from T-SQL world use some T-SQL constructs, then
usually the code should not work with the error "query has not destination
for data ... "
When PLpgSQL will be more tolerant, then their code will be executed
without any error, but will not work.
Regards
Pavel
Show quoted text
My negative opinion is known. The PERFORM statement is much more
workaround than well designed statement, but I would to see ANSI/SQL based
fix. I try to compare benefits and loss.Can you start with analyze what is possible, and what semantic is allowed
in standard and other well known SQL databases?Regards
Pavel
merlin
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index b7f44ca..a860066 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -3457,12 +3457,9 @@ exec_stmt_execsql(PLpgSQL_execstate *estate, } else { - /* If the statement returned a tuple table, complain */ + /* If the statement returned a tuple table, free it. */ if (SPI_tuptable != NULL) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("query has no destination for result data"), - (rc == SPI_OK_SELECT) ? errhint("If you want to discard the results of a SELECT, use PERFORM instead.") : 0)); + SPI_freetuptable(SPI_tuptable); }if (paramLI)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Apr 10, 2016 at 1:13 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:
Hi
2016-03-21 22:13 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
2016-03-21 21:24 GMT+01:00 Merlin Moncure <mmoncure@gmail.com>:
Patch is trivial (see below), discussion is not :-).
I see no useful reason to require INTO when returning data with
SELECT. However, requiring queries to indicate not needing data via
PERFORM causes some annoyances:*) converting routines back and forth between pl/pgsql and pl/sql
requires needless busywork and tends to cause errors to be thrown at
runtime*) as much as possible, (keywords begin/end remain a problem),
pl/pgsql should be a superset of sql*) it's much more likely to be burned by accidentally forgetting to
swap in PERFORM than to accidentally leave in a statement with no
actionable target. Even if you did so in the latter case, it stands
to reason you'd accidentally leave in the target variable, too.*) the PERFORM requirement hails from the days when only statements
starting with SELECT return data. There is no PERFORM equivalent for
WITH/INSERT/DELETE/UPDATE and there are real world scenarios where you
might have a RETURNING clause that does something but not necessarily
want to place the result in a variable (for example passing to
volatile function). Take a look at the errhint() clause below -- we
don't even have a suggestion in that case.This has come up before, and there was a fair amount of sympathy for
this argument albeit with some dissent -- notably Pavel. I'd like to
get a hearing on the issue -- thanks. If we decide to move forward,
this would effectively deprecate PERFORM and the documentation will be
suitably modified as well.here is another argument why this idea is not good.
Now, when people coming from T-SQL world use some T-SQL constructs, then
usually the code should not work with the error "query has not destination
for data ... "When PLpgSQL will be more tolerant, then their code will be executed
without any error, but will not work.
I would be inclined to require that DML returning tuples requires INTO
while a SELECT does not. Adding RETURNING is a deliberate user action that
we can and probably should be conservative for. Writing SELECT is default
user behavior and is quite often used only for its side-effects. Since SQL
proper doesn't offer a means to distinguish between the two modes adding
that distinction to pl/pgSQL, while useful, doesn't seem like something
that has to be forced upon the user.
On the last point I probably wouldn't bother to deprecate PERFORM for that
reason, let alone the fact we likely would never choose to actually remove
the capability.
I'm not convinced that allowing RETURNING to be target-less is needed.
With writable CTEs you can now get that capability by wrapping the DML in a
target-less SELECT. Again, coming back to "typical usage", I'd have no
problem making something like "RETURNING func(col) INTO /dev/null" work
for the exceptional cases that need returning but don't have any good
variables to assign the values to and don't want to make some up just to
ignore them.
David J.
2016-04-10 17:49 GMT+02:00 David G. Johnston <david.g.johnston@gmail.com>:
On Sun, Apr 10, 2016 at 1:13 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:Hi
2016-03-21 22:13 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
2016-03-21 21:24 GMT+01:00 Merlin Moncure <mmoncure@gmail.com>:
Patch is trivial (see below), discussion is not :-).
I see no useful reason to require INTO when returning data with
SELECT. However, requiring queries to indicate not needing data via
PERFORM causes some annoyances:*) converting routines back and forth between pl/pgsql and pl/sql
requires needless busywork and tends to cause errors to be thrown at
runtime*) as much as possible, (keywords begin/end remain a problem),
pl/pgsql should be a superset of sql*) it's much more likely to be burned by accidentally forgetting to
swap in PERFORM than to accidentally leave in a statement with no
actionable target. Even if you did so in the latter case, it stands
to reason you'd accidentally leave in the target variable, too.*) the PERFORM requirement hails from the days when only statements
starting with SELECT return data. There is no PERFORM equivalent for
WITH/INSERT/DELETE/UPDATE and there are real world scenarios where you
might have a RETURNING clause that does something but not necessarily
want to place the result in a variable (for example passing to
volatile function). Take a look at the errhint() clause below -- we
don't even have a suggestion in that case.This has come up before, and there was a fair amount of sympathy for
this argument albeit with some dissent -- notably Pavel. I'd like to
get a hearing on the issue -- thanks. If we decide to move forward,
this would effectively deprecate PERFORM and the documentation will be
suitably modified as well.here is another argument why this idea is not good.
Now, when people coming from T-SQL world use some T-SQL constructs, then
usually the code should not work with the error "query has not destination
for data ... "When PLpgSQL will be more tolerant, then their code will be executed
without any error, but will not work.I would be inclined to require that DML returning tuples requires INTO
while a SELECT does not. Adding RETURNING is a deliberate user action that
we can and probably should be conservative for. Writing SELECT is default
user behavior and is quite often used only for its side-effects. Since SQL
proper doesn't offer a means to distinguish between the two modes adding
that distinction to pl/pgSQL, while useful, doesn't seem like something
that has to be forced upon the user.
It doesn't help - SELECT is most often used construct.
We can be less strict for SELECT expr, but SELECT FROM should not be
allowed without INTO clause.
It is reason, why I dislike this proposal, because the rules when INTO is
allowed, required will be more complex. Now, the rules are pretty simple -
and it is safe for beginners. I accept so current behave should be limiting
for experts.
Regards
Pavel
Show quoted text
On the last point I probably wouldn't bother to deprecate PERFORM for that
reason, let alone the fact we likely would never choose to actually remove
the capability.I'm not convinced that allowing RETURNING to be target-less is needed.
With writable CTEs you can now get that capability by wrapping the DML in a
target-less SELECT. Again, coming back to "typical usage", I'd have no
problem making something like "RETURNING func(col) INTO /dev/null" work
for the exceptional cases that need returning but don't have any good
variables to assign the values to and don't want to make some up just to
ignore them.David J.
On Sun, Apr 10, 2016 at 9:16 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:
2016-04-10 17:49 GMT+02:00 David G. Johnston <david.g.johnston@gmail.com>:
On Sun, Apr 10, 2016 at 1:13 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:Hi
2016-03-21 22:13 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
2016-03-21 21:24 GMT+01:00 Merlin Moncure <mmoncure@gmail.com>:
Patch is trivial (see below), discussion is not :-).
I see no useful reason to require INTO when returning data with
SELECT. However, requiring queries to indicate not needing data via
PERFORM causes some annoyances:*) converting routines back and forth between pl/pgsql and pl/sql
requires needless busywork and tends to cause errors to be thrown at
runtime*) as much as possible, (keywords begin/end remain a problem),
pl/pgsql should be a superset of sql*) it's much more likely to be burned by accidentally forgetting to
swap in PERFORM than to accidentally leave in a statement with no
actionable target. Even if you did so in the latter case, it stands
to reason you'd accidentally leave in the target variable, too.*) the PERFORM requirement hails from the days when only statements
starting with SELECT return data. There is no PERFORM equivalent for
WITH/INSERT/DELETE/UPDATE and there are real world scenarios where you
might have a RETURNING clause that does something but not necessarily
want to place the result in a variable (for example passing to
volatile function). Take a look at the errhint() clause below -- we
don't even have a suggestion in that case.This has come up before, and there was a fair amount of sympathy for
this argument albeit with some dissent -- notably Pavel. I'd like to
get a hearing on the issue -- thanks. If we decide to move forward,
this would effectively deprecate PERFORM and the documentation will be
suitably modified as well.here is another argument why this idea is not good.
Now, when people coming from T-SQL world use some T-SQL constructs, then
usually the code should not work with the error "query has not destination
for data ... "When PLpgSQL will be more tolerant, then their code will be executed
without any error, but will not work.I would be inclined to require that DML returning tuples requires INTO
while a SELECT does not. Adding RETURNING is a deliberate user action that
we can and probably should be conservative for. Writing SELECT is default
user behavior and is quite often used only for its side-effects. Since SQL
proper doesn't offer a means to distinguish between the two modes adding
that distinction to pl/pgSQL, while useful, doesn't seem like something
that has to be forced upon the user.It doesn't help - SELECT is most often used construct.
We can be less strict for SELECT expr, but SELECT FROM should not be
allowed without INTO clause.
SELECT perform_this_action_for_every_user(user_id) FROM usertable;
I still only care about side-effects.
The rule remains (becomes?) simple: Use INTO if you need to capture the
SQL value into a pl/pgSQL variable - otherwise don't. WRT your prior post
I'd tell the user they are doing something really unusual if they write
INSERT RETURNING without INTO - which I have no problem doing.
We don't need to force the user to tell us they intentionally omitted the
INTO clause. The omission itself is sufficient. Using select without a
target pl/pgSQL variable is a valid and probably quite common construct and
hindering it because it might make debugging a function a bit harder (wrong
data instead of an error) doesn't seem worthwhile. You are making
accommodations for exceptional situations. I'm not convinced that it will
be significantly harder to spot a missing INTO in a world where one is
allowed to write such a statement without PERFORM. Yes, it will not be as
convenient. Its a usability trade-off.
There is value in having the non-procedural aspects of pl/pgSQL be as
close to pure SQL as possible.
I am not in a position to realistically judge the trade-offs involved here
as it pertains to something learning the language. I personally haven't
found the need to specify PERFORM particularly problematic but I've also
never been bit by the inverse - specifying PERFORM when in fact I needed to
assign to a variable. I guess my main point is I see no fundamental reason
to require a user to explicitly inform that they are omitting the INTO
clause but don't see that changing the status-quo will affect a significant
change in my quality of life. My experiences are quite limited though and
I'd be more inclined to side with the thoughts of those who are interacting
with less experienced (and generally a wider variety) developers on a daily
basis.
David J.
2016-04-10 18:49 GMT+02:00 David G. Johnston <david.g.johnston@gmail.com>:
On Sun, Apr 10, 2016 at 9:16 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:2016-04-10 17:49 GMT+02:00 David G. Johnston <david.g.johnston@gmail.com>
:On Sun, Apr 10, 2016 at 1:13 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:Hi
2016-03-21 22:13 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
2016-03-21 21:24 GMT+01:00 Merlin Moncure <mmoncure@gmail.com>:
Patch is trivial (see below), discussion is not :-).
I see no useful reason to require INTO when returning data with
SELECT. However, requiring queries to indicate not needing data via
PERFORM causes some annoyances:*) converting routines back and forth between pl/pgsql and pl/sql
requires needless busywork and tends to cause errors to be thrown at
runtime*) as much as possible, (keywords begin/end remain a problem),
pl/pgsql should be a superset of sql*) it's much more likely to be burned by accidentally forgetting to
swap in PERFORM than to accidentally leave in a statement with no
actionable target. Even if you did so in the latter case, it stands
to reason you'd accidentally leave in the target variable, too.*) the PERFORM requirement hails from the days when only statements
starting with SELECT return data. There is no PERFORM equivalent for
WITH/INSERT/DELETE/UPDATE and there are real world scenarios where you
might have a RETURNING clause that does something but not necessarily
want to place the result in a variable (for example passing to
volatile function). Take a look at the errhint() clause below -- we
don't even have a suggestion in that case.This has come up before, and there was a fair amount of sympathy for
this argument albeit with some dissent -- notably Pavel. I'd like to
get a hearing on the issue -- thanks. If we decide to move forward,
this would effectively deprecate PERFORM and the documentation will be
suitably modified as well.here is another argument why this idea is not good.
Now, when people coming from T-SQL world use some T-SQL constructs,
then usually the code should not work with the error "query has not
destination for data ... "When PLpgSQL will be more tolerant, then their code will be executed
without any error, but will not work.I would be inclined to require that DML returning tuples requires INTO
while a SELECT does not. Adding RETURNING is a deliberate user action that
we can and probably should be conservative for. Writing SELECT is default
user behavior and is quite often used only for its side-effects. Since SQL
proper doesn't offer a means to distinguish between the two modes adding
that distinction to pl/pgSQL, while useful, doesn't seem like something
that has to be forced upon the user.It doesn't help - SELECT is most often used construct.
We can be less strict for SELECT expr, but SELECT FROM should not be
allowed without INTO clause.SELECT perform_this_action_for_every_user(user_id) FROM usertable;
I still only care about side-effects.
The rule remains (becomes?) simple: Use INTO if you need to capture the
SQL value into a pl/pgSQL variable - otherwise don't. WRT your prior post
I'd tell the user they are doing something really unusual if they write
INSERT RETURNING without INTO - which I have no problem doing.
This is the problem. Any other databases doesn't allow it - or it has
pretty different semantic (in T-SQL)
I am skeptical if benefit is higher than costs.
We don't need to force the user to tell us they intentionally omitted the
INTO clause. The omission itself is sufficient. Using select without a
target pl/pgSQL variable is a valid and probably quite common construct and
hindering it because it might make debugging a function a bit harder (wrong
data instead of an error) doesn't seem worthwhile. You are making
accommodations for exceptional situations. I'm not convinced that it will
be significantly harder to spot a missing INTO in a world where one is
allowed to write such a statement without PERFORM. Yes, it will not be as
convenient. Its a usability trade-off.There is value in having the non-procedural aspects of pl/pgSQL be as
close to pure SQL as possible.
It is not valid (semantically) - you cannot throw result in pure SQL
Show quoted text
I am not in a position to realistically judge the trade-offs involved
here as it pertains to something learning the language. I personally
haven't found the need to specify PERFORM particularly problematic but I've
also never been bit by the inverse - specifying PERFORM when in fact I
needed to assign to a variable. I guess my main point is I see no
fundamental reason to require a user to explicitly inform that they are
omitting the INTO clause but don't see that changing the status-quo will
affect a significant change in my quality of life. My experiences are
quite limited though and I'd be more inclined to side with the thoughts of
those who are interacting with less experienced (and generally a wider
variety) developers on a daily basis.David J.
On Sun, Apr 10, 2016 at 10:01 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:
2016-04-10 18:49 GMT+02:00 David G. Johnston <david.g.johnston@gmail.com>:
On Sun, Apr 10, 2016 at 9:16 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:2016-04-10 17:49 GMT+02:00 David G. Johnston <david.g.johnston@gmail.com
:
On Sun, Apr 10, 2016 at 1:13 AM, Pavel Stehule <pavel.stehule@gmail.com
wrote:
Hi
2016-03-21 22:13 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
2016-03-21 21:24 GMT+01:00 Merlin Moncure <mmoncure@gmail.com>:
Patch is trivial (see below), discussion is not :-).
I see no useful reason to require INTO when returning data with
SELECT. However, requiring queries to indicate not needing data via
PERFORM causes some annoyances:*) converting routines back and forth between pl/pgsql and pl/sql
requires needless busywork and tends to cause errors to be thrown at
runtime*) as much as possible, (keywords begin/end remain a problem),
pl/pgsql should be a superset of sql*) it's much more likely to be burned by accidentally forgetting to
swap in PERFORM than to accidentally leave in a statement with no
actionable target. Even if you did so in the latter case, it stands
to reason you'd accidentally leave in the target variable, too.*) the PERFORM requirement hails from the days when only statements
starting with SELECT return data. There is no PERFORM equivalent for
WITH/INSERT/DELETE/UPDATE and there are real world scenarios where
you
might have a RETURNING clause that does something but not necessarily
want to place the result in a variable (for example passing to
volatile function). Take a look at the errhint() clause below -- we
don't even have a suggestion in that case.This has come up before, and there was a fair amount of sympathy for
this argument albeit with some dissent -- notably Pavel. I'd like to
get a hearing on the issue -- thanks. If we decide to move forward,
this would effectively deprecate PERFORM and the documentation will
be
suitably modified as well.here is another argument why this idea is not good.
Now, when people coming from T-SQL world use some T-SQL constructs,
then usually the code should not work with the error "query has not
destination for data ... "When PLpgSQL will be more tolerant, then their code will be executed
without any error, but will not work.I would be inclined to require that DML returning tuples requires INTO
while a SELECT does not. Adding RETURNING is a deliberate user action that
we can and probably should be conservative for. Writing SELECT is default
user behavior and is quite often used only for its side-effects. Since SQL
proper doesn't offer a means to distinguish between the two modes adding
that distinction to pl/pgSQL, while useful, doesn't seem like something
that has to be forced upon the user.It doesn't help - SELECT is most often used construct.
We can be less strict for SELECT expr, but SELECT FROM should not be
allowed without INTO clause.SELECT perform_this_action_for_every_user(user_id) FROM usertable;
I still only care about side-effects.
The rule remains (becomes?) simple: Use INTO if you need to capture the
SQL value into a pl/pgSQL variable - otherwise don't. WRT your prior post
I'd tell the user they are doing something really unusual if they write
INSERT RETURNING without INTO - which I have no problem doing.This is the problem. Any other databases doesn't allow it - or it has
pretty different semantic (in T-SQL)I am skeptical if benefit is higher than costs.
This isn't T-SQL, and if you are not going to explain how it works and why
its behavior is desirable I'm not going to be convinced that it matters.
We don't need to force the user to tell us they intentionally omitted the
INTO clause. The omission itself is sufficient. Using select without a
target pl/pgSQL variable is a valid and probably quite common construct and
hindering it because it might make debugging a function a bit harder (wrong
data instead of an error) doesn't seem worthwhile. You are making
accommodations for exceptional situations. I'm not convinced that it will
be significantly harder to spot a missing INTO in a world where one is
allowed to write such a statement without PERFORM. Yes, it will not be as
convenient. Its a usability trade-off.There is value in having the non-procedural aspects of pl/pgSQL be as
close to pure SQL as possible.It is not valid (semantically) - you cannot throw result in pure SQL
I have no idea what "throw result" means.
David J.
On Sun, Apr 10, 2016 at 3:13 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
Hi
2016-03-21 22:13 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
2016-03-21 21:24 GMT+01:00 Merlin Moncure <mmoncure@gmail.com>:
Patch is trivial (see below), discussion is not :-).
I see no useful reason to require INTO when returning data with
SELECT. However, requiring queries to indicate not needing data via
PERFORM causes some annoyances:*) converting routines back and forth between pl/pgsql and pl/sql
requires needless busywork and tends to cause errors to be thrown at
runtime*) as much as possible, (keywords begin/end remain a problem),
pl/pgsql should be a superset of sql*) it's much more likely to be burned by accidentally forgetting to
swap in PERFORM than to accidentally leave in a statement with no
actionable target. Even if you did so in the latter case, it stands
to reason you'd accidentally leave in the target variable, too.*) the PERFORM requirement hails from the days when only statements
starting with SELECT return data. There is no PERFORM equivalent for
WITH/INSERT/DELETE/UPDATE and there are real world scenarios where you
might have a RETURNING clause that does something but not necessarily
want to place the result in a variable (for example passing to
volatile function). Take a look at the errhint() clause below -- we
don't even have a suggestion in that case.This has come up before, and there was a fair amount of sympathy for
this argument albeit with some dissent -- notably Pavel. I'd like to
get a hearing on the issue -- thanks. If we decide to move forward,
this would effectively deprecate PERFORM and the documentation will be
suitably modified as well.here is another argument why this idea is not good.
Now, when people coming from T-SQL world use some T-SQL constructs, then usually the code should not work with the error "query has not destination for data ... "
When PLpgSQL will be more tolerant, then their code will be executed without any error, but will not work.
I don't think it's a problem requiring people to use RETURN in order
to return data from the function.
SQL functions BTW happily discard results and it's never been an issue
there FWICT. To address your other argument given below, there are
valid cases where you'd use RETURNING without having any interest in
capturing the set. For example, you might have a volatile function,
v_func() that does something and returns a value that may not be
essential to the caller (say, a count of rows adjusted).
INSERT INTO foo ...
RETURNING v_func(foo.x);
Scenarios (even if not very common) where dummy variables are required
and/or queries have to be written into more complex forms (say, into a
CTE) where you would not have to do so outside pl/pgsql greatly
outweigh your points that, 'users might do the wrong thing'. The
wrong thing is actually the right thing in some cases.
Small aside here: One thing that t-sql did right and pl/sql did wrong
was to make the language a proper superset of sql. pl/pgsql's
hijacking INTO, BEGIN, END, and EXECUTE are really unfortunate as are
any behaviors that are incompatible with the regular language (like
requiring PERFORM); they fork the language and make building stored
procedures in pl/pgsql much more difficult if not impossible. I'm not
sure this is a really solvable problem, but at least it can be nibbled
at.
What are the rules for pl/psm?
merlin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2016-04-11 15:37 GMT+02:00 Merlin Moncure <mmoncure@gmail.com>:
On Sun, Apr 10, 2016 at 3:13 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:Hi
2016-03-21 22:13 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
2016-03-21 21:24 GMT+01:00 Merlin Moncure <mmoncure@gmail.com>:
Patch is trivial (see below), discussion is not :-).
I see no useful reason to require INTO when returning data with
SELECT. However, requiring queries to indicate not needing data via
PERFORM causes some annoyances:*) converting routines back and forth between pl/pgsql and pl/sql
requires needless busywork and tends to cause errors to be thrown at
runtime*) as much as possible, (keywords begin/end remain a problem),
pl/pgsql should be a superset of sql*) it's much more likely to be burned by accidentally forgetting to
swap in PERFORM than to accidentally leave in a statement with no
actionable target. Even if you did so in the latter case, it stands
to reason you'd accidentally leave in the target variable, too.*) the PERFORM requirement hails from the days when only statements
starting with SELECT return data. There is no PERFORM equivalent for
WITH/INSERT/DELETE/UPDATE and there are real world scenarios where you
might have a RETURNING clause that does something but not necessarily
want to place the result in a variable (for example passing to
volatile function). Take a look at the errhint() clause below -- we
don't even have a suggestion in that case.This has come up before, and there was a fair amount of sympathy for
this argument albeit with some dissent -- notably Pavel. I'd like to
get a hearing on the issue -- thanks. If we decide to move forward,
this would effectively deprecate PERFORM and the documentation will be
suitably modified as well.here is another argument why this idea is not good.
Now, when people coming from T-SQL world use some T-SQL constructs, then
usually the code should not work with the error "query has not destination
for data ... "When PLpgSQL will be more tolerant, then their code will be executed
without any error, but will not work.
I don't think it's a problem requiring people to use RETURN in order
to return data from the function.SQL functions BTW happily discard results and it's never been an issue
there FWICT. To address your other argument given below, there are
valid cases where you'd use RETURNING without having any interest in
capturing the set. For example, you might have a volatile function,
v_func() that does something and returns a value that may not be
essential to the caller (say, a count of rows adjusted).INSERT INTO foo ...
RETURNING v_func(foo.x);Scenarios (even if not very common) where dummy variables are required
and/or queries have to be written into more complex forms (say, into a
CTE) where you would not have to do so outside pl/pgsql greatly
outweigh your points that, 'users might do the wrong thing'. The
wrong thing is actually the right thing in some cases.Small aside here: One thing that t-sql did right and pl/sql did wrong
was to make the language a proper superset of sql. pl/pgsql's
hijacking INTO, BEGIN, END, and EXECUTE are really unfortunate as are
any behaviors that are incompatible with the regular language (like
requiring PERFORM); they fork the language and make building stored
procedures in pl/pgsql much more difficult if not impossible. I'm not
sure this is a really solvable problem, but at least it can be nibbled
at.What are the rules for pl/psm?
SQL/PSM knows only "select statement: single row" - subclause 12.5 - and it
is reference to ANSI SQL foundation - subclause 14.7 - where is defined
SELECT INTO. INTO is mandatory. No other SELECT form is possible.
This is defined in ANSI SQL 2011 - I have not access to more current drafts.
I read a Oracle doc - there INTO or BULK COLLECT clauses are required.
Regards
Pavel
Show quoted text
merlin
On Tuesday, March 22, 2016, Merlin Moncure <mmoncure@gmail.com> wrote:
Anyways, here's the patch with documentation adjustments as promised.
I ended up keeping the 'without result' section because it contained
useful information about plan caching,merlin
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml new file mode 100644 index 9786242..512eaa7 *** a/doc/src/sgml/plpgsql.sgml --- b/doc/src/sgml/plpgsql.sgml *************** my_record.user_id := 20; *** 904,910 **** <title>Executing a Command With No Result</title><para> ! For any SQL command that does not return rows, for example <command>INSERT</> without a <literal>RETURNING</> clause, you can execute the command within a <application>PL/pgSQL</application> function just by writing the command. --- 904,910 ---- <title>Executing a Command With No Result</title><para>
! For any SQL command, for example
<command>INSERT</> without a <literal>RETURNING</> clause, you can
execute the command within a <application>PL/pgSQL</application>
function
just by writing the command.
*************** my_record.user_id := 20;
This just seems odd...a bit broader approach here would be nice.
Especially since now it's not the command that doesn't have a result but
the user decision to not capture any result that may be present. Using
insert returning for the example here is just, again, odd...
Removing PERFORM requires a bit more reframing of the docs than you've done
here; too much was influenced by its presence.
*** 925,972 ****
<xref linkend="plpgsql-plan-caching">.
</para>- <para>
- Sometimes it is useful to evaluate an expression or
<command>SELECT</>
- query but discard the result, for example when calling a function
- that has side-effects but no useful result value. To do
- this in <application>PL/pgSQL</application>, use the
- <command>PERFORM</command> statement:
-
- <synopsis>
- PERFORM <replaceable>query</replaceable>;
- </synopsis>
-
- This executes <replaceable>query</replaceable> and discards the
- result. Write the <replaceable>query</replaceable> the same
- way you would write an SQL <command>SELECT</> command, but replace
the
- initial keyword <command>SELECT</> with <command>PERFORM</command>.
- For <command>WITH</> queries, use <command>PERFORM</> and then
- place the query in parentheses. (In this case, the query can only
- return one row.)
- <application>PL/pgSQL</application> variables will be
- substituted into the query just as for commands that return no
result,
- and the plan is cached in the same way. Also, the special variable
- <literal>FOUND</literal> is set to true if the query produced at
- least one row, or false if it produced no rows (see
- <xref linkend="plpgsql-statements-diagnostics">).
- </para>
-
<note>
<para>
! One might expect that writing <command>SELECT</command> directly
! would accomplish this result, but at
! present the only accepted way to do it is
! <command>PERFORM</command>. A SQL command that can return rows,
! such as <command>SELECT</command>, will be rejected as an error
! unless it has an <literal>INTO</> clause as discussed in the
! next section.
</para>
</note><para> An example: <programlisting> ! PERFORM create_mv('cs_session_page_requests_mv', my_query); </programlisting> </para> </sect2> --- 925,944 ---- <xref linkend="plpgsql-plan-caching">. </para><note>
<para>
! In older versions of PostgreSQL, it was mandatory to use
! <command>PERFORM</command> instead of <command>SELECT</command>
! when the query could return data that was not captured into
! variables. This requirement has been relaxed and usage of
! <command>PERFORM</command> has been deprecated.
</para>
</note><para>
An example:
<programlisting>
! SELECT create_mv('cs_session_page_requests_mv', my_query);
I'd consider making the note into a paragraph and then saying that the
select form and perform form are equivalent by writing both out one after
the other. The note brings emphasis that is not necessary if we want to
de-emphasize perform.
</programlisting>
</para> </sect2> *************** GET DIAGNOSTICS integer_var = ROW_COUNT; *** 1480,1491 **** </listitem> <listitem> <para> ! A <command>PERFORM</> statement sets <literal>FOUND</literal> true if it produces (and discards) one or more rows, false if no row is produced. </para> </listitem> <listitem> <para> <command>UPDATE</>, <command>INSERT</>, and <command>DELETE</> statements set <literal>FOUND</literal> true if at least one --- 1452,1471 ---- </listitem> <listitem> <para> ! A <command>SELECT</> statement sets <literal>FOUND</literal> true if it produces (and discards) one or more rows, false if no row is produced. </para>
This could be worded better. It will return true regardless of whether the
result is discarded.
</listitem> <listitem> + <para> + A <command>PERFORM</> statement sets <literal>FOUND</literal> + true if it produces (and discards) one or more rows, false if + no row is produced. This statement is equivalent to + <command>SELECT</> without INTO. + </para> + </listitem> + <listitem> diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml new file mode 100644 index 9786242..512eaa7
Duplicate (x2)? copy-paste elided
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c new file mode 100644 index b63ecac..975e8fe *** a/src/pl/plpgsql/src/pl_exec.c --- b/src/pl/plpgsql/src/pl_exec.c *************** exec_stmt_assign(PLpgSQL_execstate *esta *** 1557,1562 **** --- 1557,1563 ---- * exec_stmt_perform Evaluate query and discard result (but set * FOUND depending on whether at least one row * was returned). + * This syntax is deprecated. * ---------- */ static int *************** exec_stmt_execsql(PLpgSQL_execstate *est *** 3647,3658 **** } else { ! /* If the statement returned a tuple table, complain */ if (SPI_tuptable != NULL) ! ereport(ERROR, ! (errcode(ERRCODE_SYNTAX_ERROR), ! errmsg("query has no destination for result data"), ! (rc == SPI_OK_SELECT) ? errhint("If you want to discard the results of a SELECT, use PERFORM instead.") : 0)); }return PLPGSQL_RC_OK; --- 3648,3656 ---- } else { ! /* If the statement returned a tuple table without INTO, free it. */ if (SPI_tuptable != NULL) ! SPI_freetuptable(SPI_tuptable); }return PLPGSQL_RC_OK;
It should include at least one new test.
The discussion talked about insert/returning behavior with respect to
into. Not necessarily for this patch related but how does that fit in?
+1 from Jim, Merlin, Robert, Tom and myself. I think the docs need work -
and the code looks ok though lacks at least one required test.
-1 from Pavel
Peter E. - you haven't commented but are on this as a reviewer...
Merlin, back in your court for polishing.
Adding myself as reviewer but leaving it in needs review for the moment.
David J.
On Tue, 22 Mar 2016 at 10:34 Robert Haas <robertmhaas@gmail.com> wrote:
Yeah, I think requiring PERFORM is stupid and annoying. +1 for
letting people write a SELECT with no target.
Apologies for being late on the thread, but another +1 from me. I've often
been frustrated by the dissonance of PERFORM against SQL.
Cheers,
BJ
2016-06-05 5:45 GMT+02:00 David G. Johnston <david.g.johnston@gmail.com>:
On Tuesday, March 22, 2016, Merlin Moncure <mmoncure@gmail.com> wrote:
Anyways, here's the patch with documentation adjustments as promised.
I ended up keeping the 'without result' section because it contained
useful information about plan caching,merlin
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml new file mode 100644 index 9786242..512eaa7 *** a/doc/src/sgml/plpgsql.sgml --- b/doc/src/sgml/plpgsql.sgml *************** my_record.user_id := 20; *** 904,910 **** <title>Executing a Command With No Result</title><para> ! For any SQL command that does not return rows, for example <command>INSERT</> without a <literal>RETURNING</> clause, you can execute the command within a <application>PL/pgSQL</application> function just by writing the command. --- 904,910 ---- <title>Executing a Command With No Result</title><para>
! For any SQL command, for example
<command>INSERT</> without a <literal>RETURNING</> clause, you can
execute the command within a <application>PL/pgSQL</application>
function
just by writing the command.
*************** my_record.user_id := 20;This just seems odd...a bit broader approach here would be nice.
Especially since now it's not the command that doesn't have a result but
the user decision to not capture any result that may be present. Using
insert returning for the example here is just, again, odd...Removing PERFORM requires a bit more reframing of the docs than you've
done here; too much was influenced by its presence.*** 925,972 ****
<xref linkend="plpgsql-plan-caching">.
</para>- <para>
- Sometimes it is useful to evaluate an expression or
<command>SELECT</>
- query but discard the result, for example when calling a function
- that has side-effects but no useful result value. To do
- this in <application>PL/pgSQL</application>, use the
- <command>PERFORM</command> statement:
-
- <synopsis>
- PERFORM <replaceable>query</replaceable>;
- </synopsis>
-
- This executes <replaceable>query</replaceable> and discards the
- result. Write the <replaceable>query</replaceable> the same
- way you would write an SQL <command>SELECT</> command, but replace
the
- initial keyword <command>SELECT</> with <command>PERFORM</command>.
- For <command>WITH</> queries, use <command>PERFORM</> and then
- place the query in parentheses. (In this case, the query can only
- return one row.)
- <application>PL/pgSQL</application> variables will be
- substituted into the query just as for commands that return no
result,
- and the plan is cached in the same way. Also, the special variable
- <literal>FOUND</literal> is set to true if the query produced at
- least one row, or false if it produced no rows (see
- <xref linkend="plpgsql-statements-diagnostics">).
- </para>
-
<note>
<para>
! One might expect that writing <command>SELECT</command> directly
! would accomplish this result, but at
! present the only accepted way to do it is
! <command>PERFORM</command>. A SQL command that can return rows,
! such as <command>SELECT</command>, will be rejected as an error
! unless it has an <literal>INTO</> clause as discussed in the
! next section.
</para>
</note><para> An example: <programlisting> ! PERFORM create_mv('cs_session_page_requests_mv', my_query); </programlisting> </para> </sect2> --- 925,944 ---- <xref linkend="plpgsql-plan-caching">. </para><note>
<para>
! In older versions of PostgreSQL, it was mandatory to use
! <command>PERFORM</command> instead of <command>SELECT</command>
! when the query could return data that was not captured into
! variables. This requirement has been relaxed and usage of
! <command>PERFORM</command> has been deprecated.
</para>
</note><para>
An example:
<programlisting>
! SELECT create_mv('cs_session_page_requests_mv', my_query);I'd consider making the note into a paragraph and then saying that the
select form and perform form are equivalent by writing both out one after
the other. The note brings emphasis that is not necessary if we want to
de-emphasize perform.</programlisting>
</para> </sect2> *************** GET DIAGNOSTICS integer_var = ROW_COUNT; *** 1480,1491 **** </listitem> <listitem> <para> ! A <command>PERFORM</> statement sets <literal>FOUND</literal> true if it produces (and discards) one or more rows, false if no row is produced. </para> </listitem> <listitem> <para> <command>UPDATE</>, <command>INSERT</>, and <command>DELETE</> statements set <literal>FOUND</literal> true if at least one --- 1452,1471 ---- </listitem> <listitem> <para> ! A <command>SELECT</> statement sets <literal>FOUND</literal> true if it produces (and discards) one or more rows, false if no row is produced. </para>This could be worded better. It will return true regardless of whether the
result is discarded.</listitem> <listitem> + <para> + A <command>PERFORM</> statement sets <literal>FOUND</literal> + true if it produces (and discards) one or more rows, false if + no row is produced. This statement is equivalent to + <command>SELECT</> without INTO. + </para> + </listitem> + <listitem> diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml new file mode 100644 index 9786242..512eaa7Duplicate (x2)? copy-paste elided
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c new file mode 100644 index b63ecac..975e8fe *** a/src/pl/plpgsql/src/pl_exec.c --- b/src/pl/plpgsql/src/pl_exec.c *************** exec_stmt_assign(PLpgSQL_execstate *esta *** 1557,1562 **** --- 1557,1563 ---- * exec_stmt_perform Evaluate query and discard result (but set * FOUND depending on whether at least one row * was returned). + * This syntax is deprecated. * ---------- */ static int *************** exec_stmt_execsql(PLpgSQL_execstate *est *** 3647,3658 **** } else { ! /* If the statement returned a tuple table, complain */ if (SPI_tuptable != NULL) ! ereport(ERROR, ! (errcode(ERRCODE_SYNTAX_ERROR), ! errmsg("query has no destination for result data"), ! (rc == SPI_OK_SELECT) ? errhint("If you want to discard the results of a SELECT, use PERFORM instead.") : 0)); }return PLPGSQL_RC_OK; --- 3648,3656 ---- } else { ! /* If the statement returned a tuple table without INTO, free it. */ if (SPI_tuptable != NULL) ! SPI_freetuptable(SPI_tuptable); }return PLPGSQL_RC_OK;
It should include at least one new test.
The discussion talked about insert/returning behavior with respect to
into. Not necessarily for this patch related but how does that fit in?+1 from Jim, Merlin, Robert, Tom and myself. I think the docs need work -
and the code looks ok though lacks at least one required test.-1 from Pavel
I didn't change my opinion - but I accepting so my opinion is minor
Regards
Pavel
Show quoted text
Peter E. - you haven't commented but are on this as a reviewer...
Merlin, back in your court for polishing.
Adding myself as reviewer but leaving it in needs review for the moment.
David J.
On Sat, Jun 4, 2016 at 10:45 PM, David G. Johnston
<david.g.johnston@gmail.com> wrote:
On Tuesday, March 22, 2016, Merlin Moncure <mmoncure@gmail.com> wrote:
Anyways, here's the patch with documentation adjustments as promised.
I ended up keeping the 'without result' section because it contained
useful information about plan caching,merlin
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml new file mode 100644 index 9786242..512eaa7 *** a/doc/src/sgml/plpgsql.sgml --- b/doc/src/sgml/plpgsql.sgml *************** my_record.user_id := 20; *** 904,910 **** <title>Executing a Command With No Result</title><para> ! For any SQL command that does not return rows, for example <command>INSERT</> without a <literal>RETURNING</> clause, you can execute the command within a <application>PL/pgSQL</application> function just by writing the command. --- 904,910 ---- <title>Executing a Command With No Result</title><para>
! For any SQL command, for example
<command>INSERT</> without a <literal>RETURNING</> clause, you can
execute the command within a <application>PL/pgSQL</application>
function
just by writing the command.
*************** my_record.user_id := 20;This just seems odd...a bit broader approach here would be nice. Especially
since now it's not the command that doesn't have a result but the user
decision to not capture any result that may be present. Using insert
returning for the example here is just, again, odd...Removing PERFORM requires a bit more reframing of the docs than you've done
here; too much was influenced by its presence.*** 925,972 ****
<xref linkend="plpgsql-plan-caching">.
</para>- <para>
- Sometimes it is useful to evaluate an expression or
<command>SELECT</>
- query but discard the result, for example when calling a function
- that has side-effects but no useful result value. To do
- this in <application>PL/pgSQL</application>, use the
- <command>PERFORM</command> statement:
-
- <synopsis>
- PERFORM <replaceable>query</replaceable>;
- </synopsis>
-
- This executes <replaceable>query</replaceable> and discards the
- result. Write the <replaceable>query</replaceable> the same
- way you would write an SQL <command>SELECT</> command, but replace
the
- initial keyword <command>SELECT</> with <command>PERFORM</command>.
- For <command>WITH</> queries, use <command>PERFORM</> and then
- place the query in parentheses. (In this case, the query can only
- return one row.)
- <application>PL/pgSQL</application> variables will be
- substituted into the query just as for commands that return no
result,
- and the plan is cached in the same way. Also, the special variable
- <literal>FOUND</literal> is set to true if the query produced at
- least one row, or false if it produced no rows (see
- <xref linkend="plpgsql-statements-diagnostics">).
- </para>
-
<note>
<para>
! One might expect that writing <command>SELECT</command> directly
! would accomplish this result, but at
! present the only accepted way to do it is
! <command>PERFORM</command>. A SQL command that can return rows,
! such as <command>SELECT</command>, will be rejected as an error
! unless it has an <literal>INTO</> clause as discussed in the
! next section.
</para>
</note><para> An example: <programlisting> ! PERFORM create_mv('cs_session_page_requests_mv', my_query); </programlisting> </para> </sect2> --- 925,944 ---- <xref linkend="plpgsql-plan-caching">. </para><note>
<para>
! In older versions of PostgreSQL, it was mandatory to use
! <command>PERFORM</command> instead of <command>SELECT</command>
! when the query could return data that was not captured into
! variables. This requirement has been relaxed and usage of
! <command>PERFORM</command> has been deprecated.
</para>
</note><para>
An example:
<programlisting>
! SELECT create_mv('cs_session_page_requests_mv', my_query);I'd consider making the note into a paragraph and then saying that the
select form and perform form are equivalent by writing both out one after
the other. The note brings emphasis that is not necessary if we want to
de-emphasize perform.</programlisting> </para> </sect2> *************** GET DIAGNOSTICS integer_var = ROW_COUNT; *** 1480,1491 **** </listitem> <listitem> <para> ! A <command>PERFORM</> statement sets <literal>FOUND</literal> true if it produces (and discards) one or more rows, false if no row is produced. </para> </listitem> <listitem> <para> <command>UPDATE</>, <command>INSERT</>, and <command>DELETE</> statements set <literal>FOUND</literal> true if at least one --- 1452,1471 ---- </listitem> <listitem> <para> ! A <command>SELECT</> statement sets <literal>FOUND</literal> true if it produces (and discards) one or more rows, false if no row is produced. </para>This could be worded better. It will return true regardless of whether the
result is discarded.</listitem> <listitem> + <para> + A <command>PERFORM</> statement sets <literal>FOUND</literal> + true if it produces (and discards) one or more rows, false if + no row is produced. This statement is equivalent to + <command>SELECT</> without INTO. + </para> + </listitem> + <listitem> diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml new file mode 100644 index 9786242..512eaa7Duplicate (x2)? copy-paste elided
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c new file mode 100644 index b63ecac..975e8fe *** a/src/pl/plpgsql/src/pl_exec.c --- b/src/pl/plpgsql/src/pl_exec.c *************** exec_stmt_assign(PLpgSQL_execstate *esta *** 1557,1562 **** --- 1557,1563 ---- * exec_stmt_perform Evaluate query and discard result (but set * FOUND depending on whether at least one row * was returned). + * This syntax is deprecated. * ---------- */ static int *************** exec_stmt_execsql(PLpgSQL_execstate *est *** 3647,3658 **** } else { ! /* If the statement returned a tuple table, complain */ if (SPI_tuptable != NULL) ! ereport(ERROR, ! (errcode(ERRCODE_SYNTAX_ERROR), ! errmsg("query has no destination for result data"), ! (rc == SPI_OK_SELECT) ? errhint("If you want to discard the results of a SELECT, use PERFORM instead.") : 0)); }return PLPGSQL_RC_OK; --- 3648,3656 ---- } else { ! /* If the statement returned a tuple table without INTO, free it. */ if (SPI_tuptable != NULL) ! SPI_freetuptable(SPI_tuptable); }return PLPGSQL_RC_OK;
It should include at least one new test.
The discussion talked about insert/returning behavior with respect to into.
Not necessarily for this patch related but how does that fit in?+1 from Jim, Merlin, Robert, Tom and myself. I think the docs need work -
and the code looks ok though lacks at least one required test.-1 from Pavel
Peter E. - you haven't commented but are on this as a reviewer...
Merlin, back in your court for polishing.
Adding myself as reviewer but leaving it in needs review for the moment.
Thanks for the review. All your comments look pretty reasonable. I'll
touch it up and resubmit.
merlin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 6/6/16 9:59 AM, Merlin Moncure wrote:
Thanks for the review. All your comments look pretty reasonable. I'll
touch it up and resubmit.
This is the last email in this thread that the commit fest app shows. I
think we are waiting on an updated patch, with docs and tests.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 9/9/16 2:57 PM, Peter Eisentraut wrote:
On 6/6/16 9:59 AM, Merlin Moncure wrote:
Thanks for the review. All your comments look pretty reasonable. I'll
touch it up and resubmit.This is the last email in this thread that the commit fest app shows. I
think we are waiting on an updated patch, with docs and tests.
I'm setting this patch to returned with feedback now.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers