]> granicus.if.org Git - postgresql/commitdiff
Fix plpgsql's exec_eval_expr() to ensure it returns a sane type OID
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 14 Apr 2010 23:52:10 +0000 (23:52 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 14 Apr 2010 23:52:10 +0000 (23:52 +0000)
even when the expression is a query that returns no rows.

So far as I can tell, the only caller that actually fails when a garbage
OID is returned is exec_stmt_case(), which is new in 8.4 --- in all other
cases, we might make a useless trip through casting logic, but we won't
fail since the isnull flag will be set.  Hence, backpatch only to 8.4,
just in case there are apps out there that aren't expecting an error to
be thrown if the query returns more or less than one column.  (Which seems
unlikely, since the error would be thrown if the query ever did return a
row; but it's possible there's some never-exercised code out there.)

Per report from Mario Splivalo.

src/pl/plpgsql/src/pl_exec.c

index bcbf6126da5ae44706349e413e0c7e8fc7b4aa0a..47808a419151f9da5db55c8c9073856a81f2dbdb 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.256 2010/02/26 02:01:34 momjian Exp $
+ *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.257 2010/04/14 23:52:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -4158,7 +4158,24 @@ exec_eval_expr(PLpgSQL_execstate *estate,
                                 errmsg("query \"%s\" did not return data", expr->query)));
 
        /*
-        * If there are no rows selected, the result is NULL.
+        * Check that the expression returns exactly one column...
+        */
+       if (estate->eval_tuptable->tupdesc->natts != 1)
+               ereport(ERROR,
+                               (errcode(ERRCODE_SYNTAX_ERROR),
+                                errmsg_plural("query \"%s\" returned %d column",
+                                                          "query \"%s\" returned %d columns",
+                                                          estate->eval_tuptable->tupdesc->natts,
+                                                          expr->query,
+                                                          estate->eval_tuptable->tupdesc->natts)));
+
+       /*
+        * ... and get the column's datatype.
+        */
+       *rettype = SPI_gettypeid(estate->eval_tuptable->tupdesc, 1);
+
+       /*
+        * If there are no rows selected, the result is a NULL of that type.
         */
        if (estate->eval_processed == 0)
        {
@@ -4167,26 +4184,17 @@ exec_eval_expr(PLpgSQL_execstate *estate,
        }
 
        /*
-        * Check that the expression returned one single Datum
+        * Check that the expression returned no more than one row.
         */
-       if (estate->eval_processed > 1)
+       if (estate->eval_processed != 1)
                ereport(ERROR,
                                (errcode(ERRCODE_CARDINALITY_VIOLATION),
                                 errmsg("query \"%s\" returned more than one row",
                                                expr->query)));
-       if (estate->eval_tuptable->tupdesc->natts != 1)
-               ereport(ERROR,
-                               (errcode(ERRCODE_SYNTAX_ERROR),
-                                errmsg_plural("query \"%s\" returned %d column",
-                                                          "query \"%s\" returned %d columns",
-                                                          estate->eval_tuptable->tupdesc->natts,
-                                                          expr->query,
-                                                          estate->eval_tuptable->tupdesc->natts)));
 
        /*
-        * Return the result and its type
+        * Return the single result Datum.
         */
-       *rettype = SPI_gettypeid(estate->eval_tuptable->tupdesc, 1);
        return SPI_getbinval(estate->eval_tuptable->vals[0],
                                                 estate->eval_tuptable->tupdesc, 1, isNull);
 }