8000 Improve error messages about misuse of SELECT INTO. · sehrope/postgres@26ae660 · GitHub
[go: up one dir, main page]
Skip to content

Commit 26ae660

Browse files
tglsfdctglsfdc
committed
Improve error messages about misuse of SELECT INTO.
Improve two places in plpgsql, and one in spi.c, where an error message would confusingly tell you that you couldn't use a SELECT query, when what you had written *was* a SELECT query. The actual problem is that you can't use SELECT ... INTO in these contexts, but the messages failed to make that apparent. Special-case SELECT INTO to make these errors more helpful. Also, fix the same spots in plpgsql, as well as several messages in exec_eval_expr(), to not quote the entire complained-of query or expression in the primary error message. That behavior very easily led to violating our message style guideline about keeping the primary error message short and single-line. Also, since the important part of the message was after the inserted text, it could make the real problem very hard to see. We can report the query or expression as the first line of errcontext instead. Per complaint from Roger Mason. Back-patch to v14, since (a) some of these messages are new in v14 and (b) v14's translatable strings are still somewhat in flux. The problem's older than that of course, but I'm hesitant to change the behavior further back. Discussion: https://postgr.es/m/1914708.1629474624@sss.pgh.pa.us
1 parent facce1d commit 26ae660

File tree

3 files changed

+51
-17
lines changed

‎src/backend/executor/spi.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1489,16 +1489,22 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
14891489
if (!SPI_is_cursor_plan(plan))
14901490
{
14911491
/* try to give a good error message */
1492+
const char *cmdtag;
1493+
14921494
if (list_length(plan->plancache_list) != 1)
14931495
ereport(ERROR,
14941496
(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
14951497
errmsg("cannot open multi-query plan as cursor")));
14961498
plansource = (CachedPlanSource *) linitial(plan->plancache_list);
1499+
/* A SELECT that fails SPI_is_cursor_plan() must be SELECT INTO */
1500+
if (plansource->commandTag == CMDTAG_SELECT)
1501+
cmdtag = "SELECT INTO";
1502+
else
1503+
cmdtag = GetCommandTagName(plansource->commandTag);
14971504
ereport(ERROR,
14981505
(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
14991506
/* translator: %s is name of a SQL command, eg INSERT */
1500-
errmsg("cannot open %s query as cursor",
1501-
GetCommandTagName(plansource->commandTag))));
1507+
errmsg("cannot open %s query as cursor", cmdtag)));
15021508
}
15031509

15041510
Assert(list_length(plan->plancache_list) == 1);

‎src/pl/plpgsql/src/expected/plpgsql_array.out

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,9 @@ PL/pgSQL function inline_code_block line 2 at assignment
7373
insert into onecol values(array[11]);
7474
do $$ declare a int[];
7575
begin a := f1 from onecol; raise notice 'a = %', a; end$$;
76-
ERROR: query "a := f1 from onecol" returned more than one row
77-
CONTEXT: PL/pgSQL function inline_code_block line 2 at assignment
76+
ERROR: query returned more than one row
77+
CONTEXT: query: a := f1 from onecol
78+
PL/pgSQL function inline_code_block line 2 at assignment
7879
do $$ declare a int[];
7980
begin a := f1 from onecol limit 1; raise notice 'a = %', a; end$$;
8081
NOTICE: a = {1,2}

‎src/pl/plpgsql/src/pl_exec.c

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3557,9 +3557,22 @@ exec_stmt_return_query(PLpgSQL_execstate *estate,
35573557

35583558
rc = SPI_execute_plan_extended(expr->plan, &options);
35593559
if (rc != SPI_OK_SELECT)
3560-
ereport(ERROR,
3561-
(errcode(ERRCODE_SYNTAX_ERROR),
3562-
errmsg("query \"%s\" is not a SELECT", expr->query)));
3560+
{
3561+
/*
3562+
* SELECT INTO deserves a special error message, because "query is
3563+
* not a SELECT" is not very helpful in that case.
3564+
*/
3565+
if (rc == SPI_OK_SELINTO)
3566+
ereport(ERROR,
3567+
(errcode(ERRCODE_SYNTAX_ERROR),
3568+
errmsg("query is SELECT INTO, but it should be plain SELECT"),
3569+
errcontext("query: %s", expr->query)));
3570+
else
3571+
ereport(ERROR,
3572+
(errcode(ERRCODE_SYNTAX_ERROR),
3573+
errmsg("query is not a SELECT"),
3574+
errcontext("query: %s", expr->query)));
3575+
}
35633576
}
35643577
else
35653578
{
@@ -5644,19 +5657,20 @@ exec_eval_expr(PLpgSQL_execstate *estate,
56445657
if (rc != SPI_OK_SELECT)
56455658
ereport(ERROR,
56465659
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
5647-
errmsg("query \"%s\" did not return data", expr->query)));
5660+
errmsg("query did not return data"),
5661+
errcontext("query: %s", expr->query)));
56485662

56495663
/*
56505664
* Check that the expression returns exactly one column...
56515665
*/
56525666
if (estate->eval_tuptable->tupdesc->natts != 1)
56535667
ereport(ERROR,
56545668
(errcode(ERRCODE_SYNTAX_ERROR),
5655-
errmsg_plural("query \"%s\" returned %d column",
5656-
"query \"%s\" returned %d columns",
5669+
errmsg_plural("query returned %d column",
5670+
"query returned %d columns",
56575671
estate->eval_tuptable->tupdesc->natts,
5658-
expr->query,
5659-
estate->eval_tuptable->tupdesc->natts)));
5672+
estate->eval_tuptable->tupdesc->natts),
5673+
errcontext("query: %s", expr->query)));
56605674

56615675
/*
56625676
* ... and get the column's datatype.
@@ -5680,8 +5694,8 @@ exec_eval_expr(PLpgSQL_execstate *estate,
56805694
if (estate->eval_processed != 1)
56815695
ereport(ERROR,
56825696
(errcode(ERRCODE_CARDINALITY_VIOLATION),
5683-
errmsg("query \"%s\" returned more than one row",
5684-
expr->query)));
5697+
errmsg("query returned more than one row"),
5698+
errcontext("query: %s", expr->query)));
56855699

56865700
/*
56875701
* Return the single result Datum.
@@ -5748,9 +5762,22 @@ exec_run_select(PLpgSQL_execstate *estate,
57485762
rc = SPI_execute_plan_with_paramlist(expr->plan, paramLI,
57495763
estate->readonly_func, maxtuples);
57505764
if (rc != SPI_OK_SELECT)
5751-
ereport(ERROR,
5752-
(errcode(ERRCODE_SYNTAX_ERROR),
5753-
errmsg("query \"%s\" is not a SELECT", expr->query)));
5765+
{
5766+
/*
5767+
* SELECT INTO deserves a special error message, because "query is not
5768+
* a SELECT" is not very helpful in that case.
5769+
*/
5770+
if (rc == SPI_OK_SELINTO)
5771+
ereport(ERROR,
5772+
(errcode(ERRCODE_SYNTAX_ERROR),
5773+
errmsg("query is SELECT INTO, but it should be plain SELECT"),
5774+
errcontext("query: %s", expr->query)));
5775+
else
5776+
ereport(ERROR,
5777+
(errcode(ERRCODE_SYNTAX_ERROR),
5778+
errmsg("query is not a SELECT"),
5779+
errcontext("query: %s", expr->query)));
5780+
}
57545781

57555782
/* Save query results for eventual cleanup */
57565783
Assert(estate->eval_tuptable == NULL);

0 commit comments

Comments
 (0)
0