mirror of
https://github.com/postgres/postgres.git
synced 2025-04-22 23:02:54 +03:00
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
This commit is contained in:
parent
facce1da91
commit
26ae660903
@ -1489,16 +1489,22 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
|
|||||||
if (!SPI_is_cursor_plan(plan))
|
if (!SPI_is_cursor_plan(plan))
|
||||||
{
|
{
|
||||||
/* try to give a good error message */
|
/* try to give a good error message */
|
||||||
|
const char *cmdtag;
|
||||||
|
|
||||||
if (list_length(plan->plancache_list) != 1)
|
if (list_length(plan->plancache_list) != 1)
|
||||||
ereport(ERROR,
|
ereport(ERROR,
|
||||||
(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
|
(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
|
||||||
errmsg("cannot open multi-query plan as cursor")));
|
errmsg("cannot open multi-query plan as cursor")));
|
||||||
plansource = (CachedPlanSource *) linitial(plan->plancache_list);
|
plansource = (CachedPlanSource *) linitial(plan->plancache_list);
|
||||||
|
/* A SELECT that fails SPI_is_cursor_plan() must be SELECT INTO */
|
||||||
|
if (plansource->commandTag == CMDTAG_SELECT)
|
||||||
|
cmdtag = "SELECT INTO";
|
||||||
|
else
|
||||||
|
cmdtag = GetCommandTagName(plansource->commandTag);
|
||||||
ereport(ERROR,
|
ereport(ERROR,
|
||||||
(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
|
(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
|
||||||
/* translator: %s is name of a SQL command, eg INSERT */
|
/* translator: %s is name of a SQL command, eg INSERT */
|
||||||
errmsg("cannot open %s query as cursor",
|
errmsg("cannot open %s query as cursor", cmdtag)));
|
||||||
GetCommandTagName(plansource->commandTag))));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
Assert(list_length(plan->plancache_list) == 1);
|
Assert(list_length(plan->plancache_list) == 1);
|
||||||
|
@ -73,8 +73,9 @@ PL/pgSQL function inline_code_block line 2 at assignment
|
|||||||
insert into onecol values(array[11]);
|
insert into onecol values(array[11]);
|
||||||
do $$ declare a int[];
|
do $$ declare a int[];
|
||||||
begin a := f1 from onecol; raise notice 'a = %', a; end$$;
|
begin a := f1 from onecol; raise notice 'a = %', a; end$$;
|
||||||
ERROR: query "a := f1 from onecol" returned more than one row
|
ERROR: query returned more than one row
|
||||||
CONTEXT: PL/pgSQL function inline_code_block line 2 at assignment
|
CONTEXT: query: a := f1 from onecol
|
||||||
|
PL/pgSQL function inline_code_block line 2 at assignment
|
||||||
do $$ declare a int[];
|
do $$ declare a int[];
|
||||||
begin a := f1 from onecol limit 1; raise notice 'a = %', a; end$$;
|
begin a := f1 from onecol limit 1; raise notice 'a = %', a; end$$;
|
||||||
NOTICE: a = {1,2}
|
NOTICE: a = {1,2}
|
||||||
|
@ -3557,9 +3557,22 @@ exec_stmt_return_query(PLpgSQL_execstate *estate,
|
|||||||
|
|
||||||
rc = SPI_execute_plan_extended(expr->plan, &options);
|
rc = SPI_execute_plan_extended(expr->plan, &options);
|
||||||
if (rc != SPI_OK_SELECT)
|
if (rc != SPI_OK_SELECT)
|
||||||
ereport(ERROR,
|
{
|
||||||
(errcode(ERRCODE_SYNTAX_ERROR),
|
/*
|
||||||
errmsg("query \"%s\" is not a SELECT", expr->query)));
|
* 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)));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
@ -5644,7 +5657,8 @@ exec_eval_expr(PLpgSQL_execstate *estate,
|
|||||||
if (rc != SPI_OK_SELECT)
|
if (rc != SPI_OK_SELECT)
|
||||||
ereport(ERROR,
|
ereport(ERROR,
|
||||||
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
|
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
|
||||||
errmsg("query \"%s\" did not return data", expr->query)));
|
errmsg("query did not return data"),
|
||||||
|
errcontext("query: %s", expr->query)));
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Check that the expression returns exactly one column...
|
* Check that the expression returns exactly one column...
|
||||||
@ -5652,11 +5666,11 @@ exec_eval_expr(PLpgSQL_execstate *estate,
|
|||||||
if (estate->eval_tuptable->tupdesc->natts != 1)
|
if (estate->eval_tuptable->tupdesc->natts != 1)
|
||||||
ereport(ERROR,
|
ereport(ERROR,
|
||||||
(errcode(ERRCODE_SYNTAX_ERROR),
|
(errcode(ERRCODE_SYNTAX_ERROR),
|
||||||
errmsg_plural("query \"%s\" returned %d column",
|
errmsg_plural("query returned %d column",
|
||||||
"query \"%s\" returned %d columns",
|
"query returned %d columns",
|
||||||
estate->eval_tuptable->tupdesc->natts,
|
estate->eval_tuptable->tupdesc->natts,
|
||||||
expr->query,
|
estate->eval_tuptable->tupdesc->natts),
|
||||||
estate->eval_tuptable->tupdesc->natts)));
|
errcontext("query: %s", expr->query)));
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* ... and get the column's datatype.
|
* ... and get the column's datatype.
|
||||||
@ -5680,8 +5694,8 @@ exec_eval_expr(PLpgSQL_execstate *estate,
|
|||||||
if (estate->eval_processed != 1)
|
if (estate->eval_processed != 1)
|
||||||
ereport(ERROR,
|
ereport(ERROR,
|
||||||
(errcode(ERRCODE_CARDINALITY_VIOLATION),
|
(errcode(ERRCODE_CARDINALITY_VIOLATION),
|
||||||
errmsg("query \"%s\" returned more than one row",
|
errmsg("query returned more than one row"),
|
||||||
expr->query)));
|
errcontext("query: %s", expr->query)));
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Return the single result Datum.
|
* Return the single result Datum.
|
||||||
@ -5748,9 +5762,22 @@ exec_run_select(PLpgSQL_execstate *estate,
|
|||||||
rc = SPI_execute_plan_with_paramlist(expr->plan, paramLI,
|
rc = SPI_execute_plan_with_paramlist(expr->plan, paramLI,
|
||||||
estate->readonly_func, maxtuples);
|
estate->readonly_func, maxtuples);
|
||||||
if (rc != SPI_OK_SELECT)
|
if (rc != SPI_OK_SELECT)
|
||||||
ereport(ERROR,
|
{
|
||||||
(errcode(ERRCODE_SYNTAX_ERROR),
|
/*
|
||||||
errmsg("query \"%s\" is not a SELECT", expr->query)));
|
* 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)));
|
||||||
|
}
|
||||||
|
|
||||||
/* Save query results for eventual cleanup */
|
/* Save query results for eventual cleanup */
|
||||||
Assert(estate->eval_tuptable == NULL);
|
Assert(estate->eval_tuptable == NULL);
|
||||||
|
Loading…
x
Reference in New Issue
Block a user