Fix checking of query type in plpgsql's RETURN QUERY command.

Prior to v14, we insisted that the query in RETURN QUERY be of a type
that returns tuples.  (For instance, INSERT RETURNING was allowed,
but not plain INSERT.)  That happened indirectly because we opened a
cursor for the query, so spi.c checked SPI_is_cursor_plan().  As a
consequence, the error message wasn't terribly on-point, but at least
it was there.

Commit 2f48ede0 lost this detail.  Instead, plain RETURN QUERY
insisted that the query be a SELECT (by checking for SPI_OK_SELECT)
while RETURN QUERY EXECUTE failed to check the query type at all.
Neither of these changes was intended.

The only convenient place to check this in the EXECUTE case is inside
_SPI_execute_plan, because we haven't done parse analysis until then.
So we need to pass down a flag saying whether to enforce that the
query returns tuples.  Fortunately, we can squeeze another boolean
into struct SPIExecuteOptions without an ABI break, since there's
padding space there.  (It's unlikely that any extensions would
already be using this new struct, but preserving ABI in v14 seems
like a smart idea anyway.)

Within spi.c, it seemed like _SPI_execute_plan's parameter list
was already ridiculously long, and I didn't want to make it longer.
So I thought of passing SPIExecuteOptions down as-is, allowing that
parameter list to become much shorter.  This makes the patch a bit
more invasive than it might otherwise be, but it's all internal to
spi.c, so that seems fine.

Per report from Marc Bachmann.  Back-patch to v14 where the
faulty code came in.

Discussion: https://postgr.es/m/1F2F75F0-27DF-406F-848D-8B50C7EEF06A@gmail.com

doc/src/sgml/spi.sgml

@@ -739,6 +739,17 @@ int SPI_execute_extended(const char *<parameter>command</parameter>,
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>bool <parameter>must_return_tuples</parameter></literal></term>
+    <listitem>
+     <para>
+      if <literal>true</literal>, raise error if the query is not of a kind
+      that returns tuples (this does not forbid the case where it happens to
+      return zero tuples)
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><literal>uint64 <parameter>tcount</parameter></literal></term>
     <listitem>
@@ -1869,6 +1880,17 @@ int SPI_execute_plan_extended(SPIPlanPtr <parameter>plan</parameter>,
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>bool <parameter>must_return_tuples</parameter></literal></term>
+    <listitem>
+     <para>
+      if <literal>true</literal>, raise error if the query is not of a kind
+      that returns tuples (this does not forbid the case where it happens to
+      return zero tuples)
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><literal>uint64 <parameter>tcount</parameter></literal></term>
     <listitem>

src/include/executor/spi.h

@@ -48,6 +48,7 @@ typedef struct SPIExecuteOptions
 	ParamListInfo params;
 	bool		read_only;
 	bool		allow_nonatomic;
+	bool		must_return_tuples;
 	uint64		tcount;
 	DestReceiver *dest;
 	ResourceOwner owner;

src/pl/plpgsql/src/pl_exec.c

@@ -3553,26 +3553,13 @@ exec_stmt_return_query(PLpgSQL_execstate *estate,
 		memset(&options, 0, sizeof(options));
 		options.params = paramLI;
 		options.read_only = estate->readonly_func;
+		options.must_return_tuples = true;
 		options.dest = treceiver;
 
 		rc = SPI_execute_plan_extended(expr->plan, &options);
-		if (rc != SPI_OK_SELECT)
-		{
-			/*
-			 * SELECT INTO deserves a special error message, because "query is
-			 * not a SELECT" is not very helpful in that case.
-			 */
-			if (rc == SPI_OK_SELINTO)
-				ereport(ERROR,
-						(errcode(ERRCODE_SYNTAX_ERROR),
-						 errmsg("query is SELECT INTO, but it should be plain SELECT"),
-						 errcontext("query: %s", expr->query)));
-			else
-				ereport(ERROR,
-						(errcode(ERRCODE_SYNTAX_ERROR),
-						 errmsg("query is not a SELECT"),
-						 errcontext("query: %s", expr->query)));
-		}
+		if (rc < 0)
+			elog(ERROR, "SPI_execute_plan_extended failed executing query \"%s\": %s",
+				 expr->query, SPI_result_code_string(rc));
 	}
 	else
 	{
@@ -3609,6 +3596,7 @@ exec_stmt_return_query(PLpgSQL_execstate *estate,
 		options.params = exec_eval_using_params(estate,
 												stmt->params);
 		options.read_only = estate->readonly_func;
+		options.must_return_tuples = true;
 		options.dest = treceiver;
 
 		rc = SPI_execute_extended(querystr, &options);

src/test/regress/expected/plpgsql.out

@@ -4227,7 +4227,7 @@ select * from tftest(10);
 (2 rows)
 
 drop function tftest(int);
-create or replace function rttest()
+create function rttest()
 returns setof int as $$
 declare rc int;
 begin
@@ -4258,6 +4258,31 @@ NOTICE:  f 0
      20
 (4 rows)
 
+-- check some error cases, too
+create or replace function rttest()
+returns setof int as $$
+begin
+  return query select 10 into no_such_table;
+end;
+$$ language plpgsql;
+select * from rttest();
+ERROR:  SELECT INTO query does not return tuples
+CONTEXT:  SQL statement "select 10 into no_such_table"
+PL/pgSQL function rttest() line 3 at RETURN QUERY
+create or replace function rttest()
+returns setof int as $$
+begin
+  return query execute 'select 10 into no_such_table';
+end;
+$$ language plpgsql;
+select * from rttest();
+ERROR:  SELECT INTO query does not return tuples
+CONTEXT:  SQL statement "select 10 into no_such_table"
+PL/pgSQL function rttest() line 3 at RETURN QUERY
+select * from no_such_table;
+ERROR:  relation "no_such_table" does not exist
+LINE 1: select * from no_such_table;
+                      ^
 drop function rttest();
 -- Test for proper cleanup at subtransaction exit.  This example
 -- exposed a bug in PG 8.2.

src/test/regress/sql/plpgsql.sql

@@ -3494,7 +3494,7 @@ select * from tftest(10);
 
 drop function tftest(int);
 
-create or replace function rttest()
+create function rttest()
 returns setof int as $$
 declare rc int;
 begin
@@ -3515,6 +3515,28 @@ $$ language plpgsql;
 
 select * from rttest();
 
+-- check some error cases, too
+
+create or replace function rttest()
+returns setof int as $$
+begin
+  return query select 10 into no_such_table;
+end;
+$$ language plpgsql;
+
+select * from rttest();
+
+create or replace function rttest()
+returns setof int as $$
+begin
+  return query execute 'select 10 into no_such_table';
+end;
+$$ language plpgsql;
+
+select * from rttest();
+
+select * from no_such_table;
+
 drop function rttest();
 
 -- Test for proper cleanup at subtransaction exit.  This example