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

Hi Pavel,

On Tue, 24 Aug 2021 at 00:19, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:

> 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/
>
> This patch is registered

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

> 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
>>>>>>
>>>>>