On 06/08/2018 20:32, Jeremy Evans wrote:
> The current code's hint is misleading for procedures:
>
> CREATE OR REPLACE PROCEDURE a(in int)
> LANGUAGE SQL
> AS $$
> SELECT NULL;
> $$;
> # CREATE PROCEDURE
>
> CREATE OR REPLACE PROCEDURE a(inout int)
> LANGUAGE SQL
> AS $$
> SELECT NULL;
> $$;
> # ERROR: cannot change return type of existing function
> # HINT: Use DROP FUNCTION a(integer) first.
Yes, the hint should be changed. But I also think the error message
should be changed to be more appropriate to the procedure situation
(where is the return type?). Attached patch does both. Unlike your
patch, I kept the "DROP FUNCTION" message for the function case. It
might be too confusing otherwise. Thoughts?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 36f00184dcc7597f69466a6e9aa2db6eee1a6ef6 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Wed, 8 Aug 2018 20:39:26 +0200
Subject: [PATCH] Improve error messages for CREATE OR REPLACE PROCEDURE
Change the hint to recommend DROP PROCEDURE instead of FUNCTION. Also
make the error message when changing the return type more specific to
the case of procedures.
Reported-by: Jeremy Evans <c...@jeremyevans.net>
---
src/backend/catalog/pg_proc.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 9b4015d0d4..654ee943be 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -375,6 +375,7 @@ ProcedureCreate(const char *procedureName,
Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
Datum proargnames;
bool isnull;
+ const char *prokind_keyword;
if (!replace)
ereport(ERROR,
@@ -400,16 +401,25 @@ ProcedureCreate(const char *procedureName,
errdetail("\"%s\" is a window
function.", procedureName) :
0)));
+ prokind_keyword = (prokind == PROKIND_PROCEDURE ? "PROCEDURE" :
"FUNCTION");
+
/*
* Not okay to change the return type of the existing proc,
since
* existing rules, views, etc may depend on the return type.
+ *
+ * In case of a procedure, a changing return type means that
whether
+ * the procedure has output parameters was changed. Since
there is no
+ * user visible return type, we produce a more specific error
message.
*/
if (returnType != oldproc->prorettype ||
returnsSet != oldproc->proretset)
ereport(ERROR,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
- errmsg("cannot change return type of
existing function"),
- errhint("Use DROP FUNCTION %s first.",
+ prokind == PROKIND_PROCEDURE
+ ? errmsg("cannot change whether a
procedure has output parameters")
+ : errmsg("cannot change return type of
existing function"),
+ errhint("Use DROP %s %s first.",
+ prokind_keyword,
format_procedure(HeapTupleGetOid(oldtup)))));
/*
@@ -434,7 +444,8 @@ ProcedureCreate(const char *procedureName,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("cannot change return
type of existing function"),
errdetail("Row type defined by
OUT parameters is different."),
- errhint("Use DROP FUNCTION %s
first.",
+ errhint("Use DROP %s %s
first.",
+
prokind_keyword,
format_procedure(HeapTupleGetOid(oldtup)))));
}
@@ -477,7 +488,8 @@ ProcedureCreate(const char *procedureName,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("cannot change
name of input parameter \"%s\"",
old_arg_names[j]),
- errhint("Use DROP
FUNCTION %s first.",
+ errhint("Use DROP %s
%s first.",
+
prokind_keyword,
format_procedure(HeapTupleGetOid(oldtup)))));
}
}
@@ -501,7 +513,8 @@ ProcedureCreate(const char *procedureName,
ereport(ERROR,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("cannot remove
parameter defaults from existing function"),
- errhint("Use DROP FUNCTION %s
first.",
+ errhint("Use DROP %s %s
first.",
+
prokind_keyword,
format_procedure(HeapTupleGetOid(oldtup)))));
proargdefaults = SysCacheGetAttr(PROCNAMEARGSNSP,
oldtup,
@@ -527,7 +540,8 @@ ProcedureCreate(const char *procedureName,
ereport(ERROR,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("cannot change
data type of existing parameter default value"),
- errhint("Use DROP
FUNCTION %s first.",
+ errhint("Use DROP %s
%s first.",
+
prokind_keyword,
format_procedure(HeapTupleGetOid(oldtup)))));
newlc = lnext(newlc);
}
--
2.18.0
