Thanks Zhihong/Pavel,
On Mon, 8 Nov 2021 at 10:03, Pavel Stehule <pavel.steh...@gmail.com> wrote:
>
>
> po 8. 11. 2021 v 5:24 odesÃlatel Pavel Stehule <pavel.steh...@gmail.com>
> napsal:
>
>>
>>
>> po 8. 11. 2021 v 5:07 odesÃlatel Pavel Stehule <pavel.steh...@gmail.com>
>> napsal:
>>
>>>
>>>>
>>>> +set_errcurrent_query (const char *query)
>>>>
>>>> You can remove the space prior to (.
>>>> I wonder if the new field can be named current_err_query because that's
>>>> what the setter implies.
>>>> current_query may give the impression that the field can store normal
>>>> query (which doesn't cause exception).
>>>> The following code implies that only one of internalquery and
>>>> current_query would be set.
>>>>
>>>
>>> yes, I think so current_query is not a good name too. Maybe query can be
>>> good enough - all in ErrorData is related to error
>>>
>>
>> so the name of field can be query, and routine for setting errquery or
>> set_errquery
>>
>
> and this part is not correct
>
> <--><-->switch (carg->mode)
> <--><-->{
> <--><--><-->case RAW_PARSE_PLPGSQL_EXPR:
> <--><--><--><-->errcontext("SQL expression \"%s\"", query);
> <--><--><--><-->break;
> <--><--><-->case RAW_PARSE_PLPGSQL_ASSIGN1:
> <--><--><-->case RAW_PARSE_PLPGSQL_ASSIGN2:
> <--><--><-->case RAW_PARSE_PLPGSQL_ASSIGN3:
> <--><--><--><-->errcontext("PL/pgSQL assignment \"%s\"", query);
> <--><--><--><-->break;
> <--><--><-->default:
> <--><--><--><-->set_errcurrent_query(query);
> <--><--><--><-->errcontext("SQL statement \"%s\"", query);
> <--><--><--><-->break;
> <--><-->}
> <-->}
>
> set_errcurrent_query should be outside the switch
>
> We want PG_SQL_TEXT for assign statements too
>
> _t := (select ...);
>
> Please find the new patch, which has the suggested changes.
> Regards
>
> Pavel
>
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index e5c1356d8c..f402c9f012 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -2995,6 +2995,16 @@ GET STACKED DIAGNOSTICS <replaceable>variable</replaceable> { = | := } <replacea
<entry><type>text</type></entry>
<entry>the name of the schema related to exception</entry>
</row>
+ <row>
+ <entry><literal>PG_SQL_TEXT</literal></entry>
+ <entry><type>text</type></entry>
+ <entry>invalid sql statement, if any</entry>
+ </row>
+ <row>
+ <entry><literal>PG_ERROR_LOCATION</literal></entry>
+ <entry><type>text</type></entry>
+ <entry>invalid dynamic sql statement's text cursor position, if any</entry>
+ </row>
<row>
<entry><literal>PG_EXCEPTION_DETAIL</literal></entry>
<entry><type>text</type></entry>
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 0568ae123f..a504bf9ed4 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2856,6 +2856,7 @@ _SPI_error_callback(void *arg)
}
else
{
+ set_errquery(query);
/* Use the parse mode to decide how to describe the query */
switch (carg->mode)
{
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index f33729513a..03bf3e7b45 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -621,6 +621,8 @@ errfinish(const char *filename, int lineno, const char *funcname)
pfree(edata->constraint_name);
if (edata->internalquery)
pfree(edata->internalquery);
+ if (edata->query)
+ pfree(edata->query);
errordata_stack_depth--;
@@ -1599,6 +1601,8 @@ CopyErrorData(void)
newedata->constraint_name = pstrdup(newedata->constraint_name);
if (newedata->internalquery)
newedata->internalquery = pstrdup(newedata->internalquery);
+ if (newedata->query)
+ newedata->query = pstrdup(newedata->query);
/* Use the calling context for string allocation */
newedata->assoc_context = CurrentMemoryContext;
@@ -1639,6 +1643,8 @@ FreeErrorData(ErrorData *edata)
pfree(edata->constraint_name);
if (edata->internalquery)
pfree(edata->internalquery);
+ if (edata->query)
+ pfree(edata->query);
pfree(edata);
}
@@ -1718,6 +1724,8 @@ ThrowErrorData(ErrorData *edata)
newedata->internalpos = edata->internalpos;
if (edata->internalquery)
newedata->internalquery = pstrdup(edata->internalquery);
+ if (edata->query)
+ newedata->query = pstrdup(edata->query);
MemoryContextSwitchTo(oldcontext);
recursion_depth--;
@@ -1784,6 +1792,8 @@ ReThrowError(ErrorData *edata)
newedata->constraint_name = pstrdup(newedata->constraint_name);
if (newedata->internalquery)
newedata->internalquery = pstrdup(newedata->internalquery);
+ if (newedata->query)
+ newedata->query = pstrdup(newedata->query);
/* Reset the assoc_context to be ErrorContext */
newedata->assoc_context = ErrorContext;
@@ -3602,3 +3612,34 @@ trace_recovery(int trace_level)
return trace_level;
}
+
+/*
+ * set_errquery --- set the query in the PL/PGSQL block, which is causing the exception
+ *
+ * We have PG_CONTEXT diagnostic item, which returns a text string with line(s) of text describing the call stack.
+ * To get the exact SQL query which is causing the error requires some text processing.
+ *
+ * To skip the text processing to get the sql query, we store the SQL statement in the 'ErrorData' object, and return the plain SQL when the user requests for it.
+ * In PL/PGSQL context, at this moment the `ErrorData` holds two types of SQL error members.
+ * 1. internalquery, which always returns the sql query which is not a valid sql statement
+ * 2. query, which always returns the sql query which is causing the current exception
+ */
+int
+set_errquery(const char *query)
+{
+ ErrorData *edata = &errordata[errordata_stack_depth];
+
+ /* we don't bother incrementing recursion_depth */
+ CHECK_STACK_DEPTH();
+
+ if (edata->query)
+ {
+ pfree(edata->query);
+ edata->query = NULL;
+ }
+
+ if (query)
+ edata->query = MemoryContextStrdup(edata->assoc_context, query);
+
+ return 0;
+}
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index f53607e12e..e2cf49671e 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -216,6 +216,7 @@ extern int errposition(int cursorpos);
extern int internalerrposition(int cursorpos);
extern int internalerrquery(const char *query);
+extern int set_errquery(const char* query);
extern int err_generic_string(int field, const char *str);
@@ -395,7 +396,7 @@ typedef struct ErrorData
int internalpos; /* cursor index into internalquery */
char *internalquery; /* text of internally-generated query */
int saved_errno; /* errno at entry */
-
+ char *query; /* text query */
/* context containing associated non-constant strings */
struct MemoryContextData *assoc_context;
} ErrorData;
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 6dbfdb7be0..203e3a0596 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2416,6 +2416,17 @@ exec_stmt_getdiag(PLpgSQL_execstate *estate, PLpgSQL_stmt_getdiag *stmt)
estate->cur_error->hint);
break;
+ case PLPGSQL_GETDIAG_SQL_TEXT:
+ if (estate->cur_error->internalquery)
+ exec_assign_c_string(estate, var, estate->cur_error->internalquery);
+ else
+ exec_assign_c_string(estate, var, estate->cur_error->query);
+ break;
+
+ case PLPGSQL_GETDIAG_ERROR_LOCATION:
+ exec_assign_value(estate, var, UInt64GetDatum(estate->cur_error->internalpos), false, INT8OID, -1);
+ break;
+
case PLPGSQL_GETDIAG_RETURNED_SQLSTATE:
exec_assign_c_string(estate, var,
unpack_sql_state(estate->cur_error->sqlerrcode));
diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c
index e0863acb3d..f00a745e9a 100644
--- a/src/pl/plpgsql/src/pl_funcs.c
+++ b/src/pl/plpgsql/src/pl_funcs.c
@@ -311,6 +311,10 @@ plpgsql_getdiag_kindname(PLpgSQL_getdiag_kind kind)
return "PG_EXCEPTION_DETAIL";
case PLPGSQL_GETDIAG_ERROR_HINT:
return "PG_EXCEPTION_HINT";
+ case PLPGSQL_GETDIAG_SQL_TEXT:
+ return "PG_SQL_TEXT";
+ case PLPGSQL_GETDIAG_ERROR_LOCATION:
+ return "PG_ERROR_LOCATION";
case PLPGSQL_GETDIAG_RETURNED_SQLSTATE:
return "RETURNED_SQLSTATE";
case PLPGSQL_GETDIAG_COLUMN_NAME:
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 0f6a5b30b1..0fd0d9c179 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -321,6 +321,8 @@ static void check_raise_parameters(PLpgSQL_stmt_raise *stmt);
%token <keyword> K_PG_CONTEXT
%token <keyword> K_PG_DATATYPE_NAME
%token <keyword> K_PG_EXCEPTION_CONTEXT
+%token <keyword> K_PG_SQL_TEXT
+%token <keyword> K_PG_ERROR_LOCATION
%token <keyword> K_PG_EXCEPTION_DETAIL
%token <keyword> K_PG_EXCEPTION_HINT
%token <keyword> K_PRINT_STRICT_PARAMS
@@ -1047,6 +1049,8 @@ stmt_getdiag : K_GET getdiag_area_opt K_DIAGNOSTICS getdiag_list ';'
case PLPGSQL_GETDIAG_ERROR_CONTEXT:
case PLPGSQL_GETDIAG_ERROR_DETAIL:
case PLPGSQL_GETDIAG_ERROR_HINT:
+ case PLPGSQL_GETDIAG_SQL_TEXT:
+ case PLPGSQL_GETDIAG_ERROR_LOCATION:
case PLPGSQL_GETDIAG_RETURNED_SQLSTATE:
case PLPGSQL_GETDIAG_COLUMN_NAME:
case PLPGSQL_GETDIAG_CONSTRAINT_NAME:
@@ -1130,6 +1134,10 @@ getdiag_item :
else if (tok_is_keyword(tok, &yylval,
K_PG_EXCEPTION_CONTEXT, "pg_exception_context"))
$$ = PLPGSQL_GETDIAG_ERROR_CONTEXT;
+ else if (tok_is_keyword(tok, &yylval, K_PG_SQL_TEXT, "pg_sql_text"))
+ $$ = PLPGSQL_GETDIAG_SQL_TEXT;
+ else if (tok_is_keyword(tok, &yylval, K_PG_ERROR_LOCATION, "pg_error_location"))
+ $$ = PLPGSQL_GETDIAG_ERROR_LOCATION;
else if (tok_is_keyword(tok, &yylval,
K_COLUMN_NAME, "column_name"))
$$ = PLPGSQL_GETDIAG_COLUMN_NAME;
diff --git a/src/pl/plpgsql/src/pl_unreserved_kwlist.h b/src/pl/plpgsql/src/pl_unreserved_kwlist.h
index fcb34f7c7f..233cadf59c 100644
--- a/src/pl/plpgsql/src/pl_unreserved_kwlist.h
+++ b/src/pl/plpgsql/src/pl_unreserved_kwlist.h
@@ -81,9 +81,11 @@ PG_KEYWORD("option", K_OPTION)
PG_KEYWORD("perform", K_PERFORM)
PG_KEYWORD("pg_context", K_PG_CONTEXT)
PG_KEYWORD("pg_datatype_name", K_PG_DATATYPE_NAME)
+PG_KEYWORD("pg_error_location", K_PG_ERROR_LOCATION)
PG_KEYWORD("pg_exception_context", K_PG_EXCEPTION_CONTEXT)
PG_KEYWORD("pg_exception_detail", K_PG_EXCEPTION_DETAIL)
PG_KEYWORD("pg_exception_hint", K_PG_EXCEPTION_HINT)
+PG_KEYWORD("pg_sql_text", K_PG_SQL_TEXT)
PG_KEYWORD("print_strict_params", K_PRINT_STRICT_PARAMS)
PG_KEYWORD("prior", K_PRIOR)
PG_KEYWORD("query", K_QUERY)
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index 3936866bb7..6bb4f2c9e4 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -151,6 +151,8 @@ typedef enum PLpgSQL_getdiag_kind
PLPGSQL_GETDIAG_ERROR_CONTEXT,
PLPGSQL_GETDIAG_ERROR_DETAIL,
PLPGSQL_GETDIAG_ERROR_HINT,
+ PLPGSQL_GETDIAG_SQL_TEXT,
+ PLPGSQL_GETDIAG_ERROR_LOCATION,
PLPGSQL_GETDIAG_RETURNED_SQLSTATE,
PLPGSQL_GETDIAG_COLUMN_NAME,
PLPGSQL_GETDIAG_CONSTRAINT_NAME,
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 33eb5e54b9..c9845248c9 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -5752,3 +5752,67 @@ END; $$ LANGUAGE plpgsql;
ERROR: "x" is not a scalar variable
LINE 3: GET DIAGNOSTICS x = ROW_COUNT;
^
+--
+-- PG_SQL_TEXT, PG_ERROR_LOCATION
+-- should return sql and text cursor position, if the dynamic sql is syntactically incorrect
+--
+DO
+$$
+DECLARE
+ v_sql TEXT:='SELECT 1 JOIN SELECT 2';
+ v_err_sql_stmt TEXT;
+ v_err_sql_pos INT;
+BEGIN
+ EXECUTE v_sql;
+EXCEPTION
+WHEN OTHERS THEN
+ GET STACKED DIAGNOSTICS v_err_sql_stmt = PG_SQL_TEXT,
+ v_err_sql_pos = PG_ERROR_LOCATION;
+ RAISE NOTICE 'exception sql %', v_err_sql_stmt;
+ RAISE NOTICE 'exception sql position %', v_err_sql_pos;
+END;
+$$;
+NOTICE: exception sql SELECT 1 JOIN SELECT 2
+NOTICE: exception sql position 15
+--
+-- PG_SQL_TEXT, PG_ERROR_LOCATION
+-- should return sql and an empty cursor position, if the dynamic sql is valid and throw the exception
+--
+DO
+$$
+DECLARE
+ v_err_sql_pos INT;
+ v_err_sql_stmt TEXT;
+BEGIN
+ EXECUTE 'SELECT 1/0';
+EXCEPTION
+WHEN OTHERS THEN
+ GET STACKED DIAGNOSTICS v_err_sql_stmt = PG_SQL_TEXT,
+ v_err_sql_pos = PG_ERROR_LOCATION;
+ RAISE NOTICE 'exception sql %', v_err_sql_stmt;
+ RAISE NOTICE 'exception sql position %', v_err_sql_pos;
+END;
+$$;
+NOTICE: exception sql SELECT 1/0
+NOTICE: exception sql position 0
+--
+-- PG_SQL_TEXT, PG_ERROR_LOCATION
+-- should return sql and an empty cursor position, if the sql is a valid and throw the exception
+--
+DO
+$$
+DECLARE
+ v_err_sql_pos INT;
+ v_err_sql_stmt TEXT;
+BEGIN
+ PERFORM 1/0;
+EXCEPTION
+WHEN OTHERS THEN
+ GET STACKED DIAGNOSTICS v_err_sql_stmt = PG_SQL_TEXT,
+ v_err_sql_pos = PG_ERROR_LOCATION;
+ RAISE NOTICE 'exception sql %', v_err_sql_stmt;
+ RAISE NOTICE 'exception sql position %', v_err_sql_pos;
+END;
+$$;
+NOTICE: exception sql SELECT 1/0
+NOTICE: exception sql position 0
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index bffb7b703b..1d9a8b043a 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -4687,3 +4687,64 @@ BEGIN
GET DIAGNOSTICS x = ROW_COUNT;
RETURN;
END; $$ LANGUAGE plpgsql;
+
+--
+-- PG_SQL_TEXT, PG_ERROR_LOCATION
+-- should return sql and text cursor position, if the dynamic sql is syntactically incorrect
+--
+DO
+$$
+DECLARE
+ v_sql TEXT:='SELECT 1 JOIN SELECT 2';
+ v_err_sql_stmt TEXT;
+ v_err_sql_pos INT;
+BEGIN
+ EXECUTE v_sql;
+EXCEPTION
+WHEN OTHERS THEN
+ GET STACKED DIAGNOSTICS v_err_sql_stmt = PG_SQL_TEXT,
+ v_err_sql_pos = PG_ERROR_LOCATION;
+ RAISE NOTICE 'exception sql %', v_err_sql_stmt;
+ RAISE NOTICE 'exception sql position %', v_err_sql_pos;
+END;
+$$;
+
+--
+-- PG_SQL_TEXT, PG_ERROR_LOCATION
+-- should return sql and an empty cursor position, if the dynamic sql is valid and throw the exception
+--
+DO
+$$
+DECLARE
+ v_err_sql_pos INT;
+ v_err_sql_stmt TEXT;
+BEGIN
+ EXECUTE 'SELECT 1/0';
+EXCEPTION
+WHEN OTHERS THEN
+ GET STACKED DIAGNOSTICS v_err_sql_stmt = PG_SQL_TEXT,
+ v_err_sql_pos = PG_ERROR_LOCATION;
+ RAISE NOTICE 'exception sql %', v_err_sql_stmt;
+ RAISE NOTICE 'exception sql position %', v_err_sql_pos;
+END;
+$$;
+
+--
+-- PG_SQL_TEXT, PG_ERROR_LOCATION
+-- should return sql and an empty cursor position, if the sql is a valid and throw the exception
+--
+DO
+$$
+DECLARE
+ v_err_sql_pos INT;
+ v_err_sql_stmt TEXT;
+BEGIN
+ PERFORM 1/0;
+EXCEPTION
+WHEN OTHERS THEN
+ GET STACKED DIAGNOSTICS v_err_sql_stmt = PG_SQL_TEXT,
+ v_err_sql_pos = PG_ERROR_LOCATION;
+ RAISE NOTICE 'exception sql %', v_err_sql_stmt;
+ RAISE NOTICE 'exception sql position %', v_err_sql_pos;
+END;
+$$;
