On Sun, 25 Jul 2021 at 16:34, Pavel Stehule <[email protected]> wrote:
>
>
> ne 25. 7. 2021 v 12:52 odesílatel Dinesh Chemuduru <
> [email protected]> napsal:
>
>> On Sat, 17 Jul 2021 at 01:29, Pavel Stehule <[email protected]>
>> wrote:
>>
>>> Hi
>>>
>>> pá 16. 7. 2021 v 21:47 odesílatel Dinesh Chemuduru <
>>> [email protected]> napsal:
>>>
>>>> Hi Everyone,
>>>>
>>>> We would like to propose the below 2 new plpgsql diagnostic items,
>>>> related to parsing. Because, the current diag items are not providing
>>>> the useful diagnostics about the dynamic SQL statements.
>>>>
>>>> 1. PG_PARSE_SQL_STATEMENT (returns parse failed sql statement)
>>>> 2. PG_PARSE_SQL_STATEMENT_POSITION (returns parse failed sql text
>>>> cursor position)
>>>>
>>>> Consider the below example, which is an invalid SQL statement.
>>>>
>>>> postgres=# SELECT 1 JOIN SELECT 2;
>>>> ERROR: syntax error at or near "JOIN"
>>>> LINE 1: SELECT 1 JOIN SELECT 2;
>>>> ^
>>>> Here, there is a syntax error at JOIN clause,
>>>> and also we are getting the syntax error position(^ symbol, the
>>>> position of JOIN clause).
>>>> This will be helpful, while dealing with long queries.
>>>>
>>>> Now, if we run the same statement as a dynamic SQL(by using EXECUTE
>>>> <sql statement>),
>>>> then it seems we are not getting the text cursor position,
>>>> and the SQL statement which is failing at parse level.
>>>>
>>>> Please find the below example.
>>>>
>>>> postgres=# SELECT exec_me('SELECT 1 JOIN SELECT 2');
>>>> NOTICE: RETURNED_SQLSTATE 42601
>>>> NOTICE: COLUMN_NAME
>>>> NOTICE: CONSTRAINT_NAME
>>>> NOTICE: PG_DATATYPE_NAME
>>>> NOTICE: MESSAGE_TEXT syntax error at or near "JOIN"
>>>> NOTICE: TABLE_NAME
>>>> NOTICE: SCHEMA_NAME
>>>> NOTICE: PG_EXCEPTION_DETAIL
>>>> NOTICE: PG_EXCEPTION_HINT
>>>> NOTICE: PG_EXCEPTION_CONTEXT PL/pgSQL function exec_me(text) line 18
>>>> at EXECUTE
>>>> NOTICE: PG_CONTEXT PL/pgSQL function exec_me(text) line 21 at GET
>>>> STACKED DIAGNOSTICS
>>>> exec_me
>>>> ---------
>>>>
>>>> (1 row)
>>>>
>>>> From the above results, by using all the existing diag items, we are
>>>> unable to get the position of "JOIN" in the submitted SQL statement.
>>>> By using these proposed diag items, we will be getting the required
>>>> information,
>>>> which will be helpful while running long SQL statements as dynamic SQL
>>>> statements.
>>>>
>>>> Please find the below example.
>>>>
>>>> postgres=# SELECT exec_me('SELECT 1 JOIN SELECT 2');
>>>> NOTICE: PG_PARSE_SQL_STATEMENT SELECT 1 JOIN SELECT 2
>>>> NOTICE: PG_PARSE_SQL_STATEMENT_POSITION 10
>>>> exec_me
>>>> ---------
>>>>
>>>> (1 row)
>>>>
>>>> From the above results, by using these diag items,
>>>> we are able to get what is failing and it's position as well.
>>>> This information will be much helpful to debug the issue,
>>>> while a long running SQL statement is running as a dynamic SQL
>>>> statement.
>>>>
>>>> We are attaching the patch for this proposal, and will be looking for
>>>> your inputs.
>>>>
>>>
>>> +1 It is good idea. I am not sure if the used names are good. I propose
>>>
>>> PG_SQL_TEXT and PG_ERROR_LOCATION
>>>
>>> Regards
>>>
>>> Pavel
>>>
>>>
>> Thanks Pavel,
>>
>> Sorry for the late reply.
>>
>> The proposed diag items are `PG_SQL_TEXT`, `PG_ERROR_LOCATION` are much
>> better and generic.
>>
>> But, as we are only dealing with the parsing failure, I thought of adding
>> that to the diag name.
>>
>
> I understand. But parsing is only one case - and these variables can be
> used for any case. Sure, ***we don't want*** to have PG_PARSE_SQL_TEXT,
> PG_ANALYZE_SQL_TEXT, PG_EXECUTION_SQL_TEXT ...
>
> The idea is good, and you found the case, where it has benefits for users.
> Naming is hard.
>
>
Thanks for your time and suggestions Pavel.
I updated the patch as per the suggestions, and attached it here for
further inputs.
Regards,
Dinesh Kumar
> Regards
>
> Pavel
>
>
>> Regards,
>> Dinesh Kumar
>>
>>
>>>
>>>
>>>> Regards,
>>>> Dinesh Kumar
>>>>
>>>
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 4cd4bcba80..3f732453e2 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -2985,6 +2985,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 dynamic 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/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index b5c208d83d..b6a977aa5c 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2402,6 +2402,14 @@ exec_stmt_getdiag(PLpgSQL_execstate *estate, PLpgSQL_stmt_getdiag *stmt)
estate->cur_error->hint);
break;
+ case PLPGSQL_GETDIAG_SQL_TEXT:
+ exec_assign_c_string(estate, var, estate->cur_error->internalquery);
+ 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 3fcca43b90..804faabb97 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 fcbfcd678b..6120837740 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 6ea169d9ad..4344d2f048 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -5701,3 +5701,46 @@ 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 empty results, if the dynamic sql is a valid
+--
+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
+NOTICE: exception sql position 0
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 781666a83a..3a53dc9e67 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -4645,3 +4645,44 @@ 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 empty results, if the dynamic sql is a valid
+--
+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;
+$$;
