Hi Daniel,
Thank you for your follow up, and attaching a new patch which addresses
Pavel's comments.
Let me know If I miss anything here.
On Thu, 4 Nov 2021 at 17:40, Daniel Gustafsson <dan...@yesql.se> wrote:
> > On 9 Sep 2021, at 08:23, Dinesh Chemuduru <dinesh.ku...@migops.com>
> wrote:
>
> > Let me try to fix the suggested case(I tried to fix this case in the
> past, but this time I will try to spend more time on this), and will submit
> a new patch.
>
> This CF entry is marked Waiting on Author, have you had the chance to
> prepare a
> new version of the patch addressing the comments from Pavel?
>
> --
> Daniel Gustafsson https://vmware.com/
>
>
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..d705bf9e1c 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2868,6 +2868,7 @@ _SPI_error_callback(void *arg)
errcontext("PL/pgSQL assignment \"%s\"", query);
break;
default:
+ set_current_err_query(query);
errcontext("SQL statement \"%s\"", query);
break;
}
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index f33729513a..42e2422e10 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->current_query)
+ pfree(edata->current_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->current_query)
+ newedata->current_query = pstrdup(newedata->current_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->current_query)
+ pfree(edata->current_query);
pfree(edata);
}
@@ -1718,6 +1724,8 @@ ThrowErrorData(ErrorData *edata)
newedata->internalpos = edata->internalpos;
if (edata->internalquery)
newedata->internalquery = pstrdup(edata->internalquery);
+ if (edata->current_query)
+ newedata->current_query = pstrdup(edata->current_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->current_query)
+ newedata->current_query = pstrdup(newedata->current_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_current_err_query --- set the current 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. current_query, which always returns the sql query which is causing the current exception
+ */
+int
+set_current_err_query(const char *query)
+{
+ ErrorData *edata = &errordata[errordata_stack_depth];
+
+ /* we don't bother incrementing recursion_depth */
+ CHECK_STACK_DEPTH();
+
+ if (edata->current_query)
+ {
+ pfree(edata->current_query);
+ edata->current_query = NULL;
+ }
+
+ if (query)
+ edata->current_query = MemoryContextStrdup(edata->assoc_context, query);
+
+ return 0;
+}
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index f53607e12e..9343a1441a 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -395,7 +395,7 @@ typedef struct ErrorData
int internalpos; /* cursor index into internalquery */
char *internalquery; /* text of internally-generated query */
int saved_errno; /* errno at entry */
-
+ char *current_query; /* text of current 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..d0aeaee736 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->current_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;
+$$;
