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

From: Dinesh Chemuduru <dinesh(dot)kumar(at)migops(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PROPOSAL] new diagnostic items for the dynamic sql
Date: 2021-08-20 08:24:16
Message-ID: CALGdMEP1hUbSk0Oxu9-8UmGq7zH--LHm=hzXMDG_aZk+LYYzoQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

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

Attachment Content-Type Size
02_diag_parse_sql_statement.patch text/x-patch 7.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2021-08-20 09:08:26 Re: pg_amcheck: Fix block number parsing on command line
Previous Message Kyotaro Horiguchi 2021-08-20 07:47:15 Re: pg_veryfybackup can fail with a valid backup for TLI > 1