[PATCH] GET DIAGNOSTICS FUNCTION_NAME

Started by Yugo Nagataabout 8 years ago11 messages
#1Yugo Nagata
nagata@sraoss.co.jp
1 attachment(s)

Hi,

Attached is a patch to implement a feature to get the current function
name by GET DIAGNOSTICS in PL/pgSQL function.

Currentyly, we can get call stack by GET DIAGNOSTICS PG_CONTEXT, but
we cannot get the function name directly. One of our clients wanted
this feature for debugging, and this was realized by creating a
function that extracts the function name string from call stack.
However, the overhead of function calls was not small, and it
caused performance regression.

I found that there are other needs for this feature[1]https://stackoverflow.com/questions/12611596/getting-name-of-the-current-function-inside-of-the-function-with-plpgsql, so I have
implemented this.

[1]: https://stackoverflow.com/questions/12611596/getting-name-of-the-current-function-inside-of-the-function-with-plpgsql

Example:

postgres=# CREATE FUNCTION test() RETURNS void
LANGUAGE plpgsql AS $$
DECLARE t text;
BEGIN
GET DIAGNOSTICS t = FUNCTION_NAME;
RAISE INFO 'function name: %', t;
END;
$$;
CREATE FUNCTION

postgres=# select test();
INFO: function name: test()
test
------

(1 row)

Regards,

--
Yugo Nagata <nagata@sraoss.co.jp>

Attachments:

getdiag_function.patchtext/x-diff; name=getdiag_function.patchDownload
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 7d23ed4..c0daa14 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1506,6 +1506,12 @@ GET DIAGNOSTICS integer_var = ROW_COUNT;
          <entry>line(s) of text describing the current call stack
           (see <xref linkend="plpgsql-call-stack"/>)</entry>
         </row>
+        <row>
+         <entry><literal>FUNCTION_NAME</literal></entry>
+         <entry><type>text</type></entry>
+         <entry>text describing the current function signature 
+          (see <xref linkend="plpgsql-call-stack"/>)</entry>
+        </row>
        </tbody>
       </tgroup>
      </table>
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index dd575e7..4404aac 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -1812,6 +1812,11 @@ exec_stmt_getdiag(PLpgSQL_execstate *estate, PLpgSQL_stmt_getdiag *stmt)
 				}
 				break;
 
+			case PLPGSQL_GETDIAG_FUNCTION_NAME:
+				exec_assign_c_string(estate, var,
+									 estate->func->fn_signature);
+				break;
+
 			default:
 				elog(ERROR, "unrecognized diagnostic item kind: %d",
 					 diag_item->kind);
diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c
index be779b6..9d9a493 100644
--- a/src/pl/plpgsql/src/pl_funcs.c
+++ b/src/pl/plpgsql/src/pl_funcs.c
@@ -323,6 +323,8 @@ plpgsql_getdiag_kindname(PLpgSQL_getdiag_kind kind)
 			return "TABLE_NAME";
 		case PLPGSQL_GETDIAG_SCHEMA_NAME:
 			return "SCHEMA_NAME";
+		case PLPGSQL_GETDIAG_FUNCTION_NAME:
+			return "FUNCTION_NAME";
 	}
 
 	return "unknown";
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index e802440..adf20e7 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -287,6 +287,7 @@ static	void			check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 %token <keyword>	K_FOREACH
 %token <keyword>	K_FORWARD
 %token <keyword>	K_FROM
+%token <keyword>	K_FUNCTION_NAME
 %token <keyword>	K_GET
 %token <keyword>	K_HINT
 %token <keyword>	K_IF
@@ -949,6 +950,7 @@ stmt_getdiag	: K_GET getdiag_area_opt K_DIAGNOSTICS getdiag_list ';'
 								/* these fields are disallowed in stacked case */
 								case PLPGSQL_GETDIAG_ROW_COUNT:
 								case PLPGSQL_GETDIAG_RESULT_OID:
+								case PLPGSQL_GETDIAG_FUNCTION_NAME:
 									if (new->is_stacked)
 										ereport(ERROR,
 												(errcode(ERRCODE_SYNTAX_ERROR),
@@ -1065,6 +1067,9 @@ getdiag_item :
 												K_SCHEMA_NAME, "schema_name"))
 							$$ = PLPGSQL_GETDIAG_SCHEMA_NAME;
 						else if (tok_is_keyword(tok, &yylval,
+												K_FUNCTION_NAME, "function_name"))
+							$$ = PLPGSQL_GETDIAG_FUNCTION_NAME;
+						else if (tok_is_keyword(tok, &yylval,
 												K_RETURNED_SQLSTATE, "returned_sqlstate"))
 							$$ = PLPGSQL_GETDIAG_RETURNED_SQLSTATE;
 						else
@@ -2407,6 +2412,7 @@ unreserved_keyword	:
 				| K_FETCH
 				| K_FIRST
 				| K_FORWARD
+				| K_FUNCTION_NAME
 				| K_GET
 				| K_HINT
 				| K_IMPORT
diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c
index 553be8c..2b624f6 100644
--- a/src/pl/plpgsql/src/pl_scanner.c
+++ b/src/pl/plpgsql/src/pl_scanner.c
@@ -127,6 +127,7 @@ static const ScanKeyword unreserved_keywords[] = {
 	PG_KEYWORD("fetch", K_FETCH, UNRESERVED_KEYWORD)
 	PG_KEYWORD("first", K_FIRST, UNRESERVED_KEYWORD)
 	PG_KEYWORD("forward", K_FORWARD, UNRESERVED_KEYWORD)
+	PG_KEYWORD("function_name", K_FUNCTION_NAME, UNRESERVED_KEYWORD)
 	PG_KEYWORD("get", K_GET, UNRESERVED_KEYWORD)
 	PG_KEYWORD("hint", K_HINT, UNRESERVED_KEYWORD)
 	PG_KEYWORD("import", K_IMPORT, UNRESERVED_KEYWORD)
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index 43d7d7d..f515f50 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -136,7 +136,8 @@ typedef enum PLpgSQL_getdiag_kind
 	PLPGSQL_GETDIAG_DATATYPE_NAME,
 	PLPGSQL_GETDIAG_MESSAGE_TEXT,
 	PLPGSQL_GETDIAG_TABLE_NAME,
-	PLPGSQL_GETDIAG_SCHEMA_NAME
+	PLPGSQL_GETDIAG_SCHEMA_NAME,
+	PLPGSQL_GETDIAG_FUNCTION_NAME
 } PLpgSQL_getdiag_kind;
 
 /*
#2Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Yugo Nagata (#1)
Re: [PATCH] GET DIAGNOSTICS FUNCTION_NAME

Attached is a patch to implement a feature to get the current function
name by GET DIAGNOSTICS in PL/pgSQL function.

Could you add it to the nexf CF, I have not seen it there? Maybe the
deadline is tonight...

--
Fabien.

#3Tatsuo Ishii
ishii@sraoss.co.jp
In reply to: Fabien COELHO (#2)
Re: [PATCH] GET DIAGNOSTICS FUNCTION_NAME

Attached is a patch to implement a feature to get the current function
name by GET DIAGNOSTICS in PL/pgSQL function.

Could you add it to the nexf CF, I have not seen it there? Maybe the
deadline is tonight...

I have added this to the next CF.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

#4Yugo Nagata
nagata@sraoss.co.jp
In reply to: Tatsuo Ishii (#3)
Re: [PATCH] GET DIAGNOSTICS FUNCTION_NAME

On Sun, 31 Dec 2017 17:54:06 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

Attached is a patch to implement a feature to get the current function
name by GET DIAGNOSTICS in PL/pgSQL function.

Could you add it to the nexf CF, I have not seen it there? Maybe the
deadline is tonight...

I have added this to the next CF.

Thank you.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

--
Yugo Nagata <nagata@sraoss.co.jp>

#5Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Yugo Nagata (#1)
Re: [PATCH] GET DIAGNOSTICS FUNCTION_NAME

Attached is a patch to implement a feature to get the current function
name by GET DIAGNOSTICS in PL/pgSQL function.

Shouldn't it be tested somewhere?

--
Fabien.

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Yugo Nagata (#1)
Re: [PATCH] GET DIAGNOSTICS FUNCTION_NAME

Yugo Nagata <nagata@sraoss.co.jp> writes:

Attached is a patch to implement a feature to get the current function
name by GET DIAGNOSTICS in PL/pgSQL function.

While this is certainly not a very large patch, it's still code that
we'd have to maintain forever, so I think it's appropriate to ask
some harder questions before accepting it.

1. I'm having a hard time visualizing an actual concrete use case for
this --- exactly when would a function not know its own name? Neither
your "our client wanted it" justification nor the cited stackoverflow
question provide anything close to an adequate rationale. I can think of
concrete uses for an operation like "give me the name of my immediate
caller", but that's not what this is.

2. The specific semantics you've chosen --- in effect, regprocedureout
results --- seem to be more because that was already available than
anything else. I can imagine wanting just the bare name, or the
schema-qualified name, or even the numeric OID (if we're in the
business of introspection, being able to look up the function's own
pg_proc entry might be useful). I'm not proposing that we offer
all those variants, certainly, but without concrete use cases it's
pretty hard to be sure we picked the most useful behavior.

3. In connection with #2, I'm dubious that FUNCTION_NAME is le mot
juste, because that would seem to imply that it is just the name,
which it isn't. If we stick with the regprocedureout semantics
I'd be inclined to propose FUNCTION_SIGNATURE.

regards, tom lane

#7Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Tom Lane (#6)
Re: [PATCH] GET DIAGNOSTICS FUNCTION_NAME

On 12/31/17 11:57, Tom Lane wrote:

3. In connection with #2, I'm dubious that FUNCTION_NAME is le mot
juste, because that would seem to imply that it is just the name,
which it isn't. If we stick with the regprocedureout semantics
I'd be inclined to propose FUNCTION_SIGNATURE.

The SQL standard has diagnostics items ROUTINE_CATALOG, ROUTINE_SCHEMA,
ROUTINE_NAME, which are used when there is an error invoking another
routine. Using notionally similar FUNCTION_* keys to mean my own
function seems pretty confusing (and also of questionable usefulness, as
you have commented).

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#8Yugo Nagata
nagata@sraoss.co.jp
In reply to: Tom Lane (#6)
Re: [PATCH] GET DIAGNOSTICS FUNCTION_NAME

On Sun, 31 Dec 2017 11:57:02 -0500
Tom Lane <tgl@sss.pgh.pa.us> wrote:

Yugo Nagata <nagata@sraoss.co.jp> writes:

Attached is a patch to implement a feature to get the current function
name by GET DIAGNOSTICS in PL/pgSQL function.

While this is certainly not a very large patch, it's still code that
we'd have to maintain forever, so I think it's appropriate to ask
some harder questions before accepting it.

1. I'm having a hard time visualizing an actual concrete use case for
this --- exactly when would a function not know its own name? Neither
your "our client wanted it" justification nor the cited stackoverflow
question provide anything close to an adequate rationale. I can think of
concrete uses for an operation like "give me the name of my immediate
caller", but that's not what this is.

Our client's use case was mainly to output debug messages at begining and
end of functions by using the same code. In addition, names of cursors
declared in the function were based on the function name, and they wanted
to get the function name to handle cursors.

However, I don't inisist on this patch, so If anyone other don't need this
feature, I'll withdraw this.

Regards,

2. The specific semantics you've chosen --- in effect, regprocedureout
results --- seem to be more because that was already available than
anything else. I can imagine wanting just the bare name, or the
schema-qualified name, or even the numeric OID (if we're in the
business of introspection, being able to look up the function's own
pg_proc entry might be useful). I'm not proposing that we offer
all those variants, certainly, but without concrete use cases it's
pretty hard to be sure we picked the most useful behavior.

3. In connection with #2, I'm dubious that FUNCTION_NAME is le mot
juste, because that would seem to imply that it is just the name,
which it isn't. If we stick with the regprocedureout semantics
I'd be inclined to propose FUNCTION_SIGNATURE.

regards, tom lane

--
Yugo Nagata <nagata@sraoss.co.jp>

#9Andres Freund
andres@anarazel.de
In reply to: Yugo Nagata (#8)
Re: [PATCH] GET DIAGNOSTICS FUNCTION_NAME

Hi,

On 2018-01-11 11:03:26 +0900, Yugo Nagata wrote:

However, I don't inisist on this patch, so If anyone other don't need this
feature, I'll withdraw this.

Given this is where the discussion dried up more than a month ago I'm
inclined to mark this as rejected unless somebody wants to argue
otherwise?

Greetings,

Andres Freund

#10Yugo Nagata
nagata@sraoss.co.jp
In reply to: Andres Freund (#9)
Re: [PATCH] GET DIAGNOSTICS FUNCTION_NAME

On Thu, 1 Mar 2018 14:29:58 -0800
Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2018-01-11 11:03:26 +0900, Yugo Nagata wrote:

However, I don't inisist on this patch, so If anyone other don't need this
feature, I'll withdraw this.

Given this is where the discussion dried up more than a month ago I'm
inclined to mark this as rejected unless somebody wants to argue
otherwise?

I have no objection.

Thans,

Greetings,

Andres Freund

--
Yugo Nagata <nagata@sraoss.co.jp>

#11David Steele
david@pgmasters.net
In reply to: Yugo Nagata (#10)
Re: Re: [PATCH] GET DIAGNOSTICS FUNCTION_NAME

On 3/5/18 10:09 PM, Yugo Nagata wrote:

On Thu, 1 Mar 2018 14:29:58 -0800
Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2018-01-11 11:03:26 +0900, Yugo Nagata wrote:

However, I don't inisist on this patch, so If anyone other don't need this
feature, I'll withdraw this.

Given this is where the discussion dried up more than a month ago I'm
inclined to mark this as rejected unless somebody wants to argue
otherwise?

I have no objection.

Marked as Rejected.

Regards,
--
-David
david@pgmasters.net