Re: [PROPOSAL] new diagnostic items for the dynamic sql

Hi

pá 20. 8. 2021 v 10:24 odesílatel Dinesh Chemuduru <dinesh(dot)kumar(at)migops(dot)com>
napsal:

> On Sun, 25 Jul 2021 at 16:34, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
>

please, can you register this patch to commitfest app

https://commitfest.postgresql.org/34/

Regards

Pavel

>
>>
>> ne 25. 7. 2021 v 12:52 odesílatel Dinesh Chemuduru <
>> dinesh(dot)kumar(at)migops(dot)com> napsal:
>>
>>> On Sat, 17 Jul 2021 at 01:29, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
>>> wrote:
>>>
>>>> Hi
>>>>
>>>> pá 16. 7. 2021 v 21:47 odesílatel Dinesh Chemuduru <
>>>> dinesh(dot)kumar(at)migops(dot)com> 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
>>>>>
>>>>