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

Thanks for your time Pavel

> On 09-Nov-2021, at 13:32, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
> 
> Hi
>
> po 8. 11. 2021 v 9:57 odesílatel Dinesh Chemuduru <dinesh(dot)kumar(at)migops(dot)com> napsal:
>> Thanks Zhihong/Pavel,
>>
>>> On Mon, 8 Nov 2021 at 10:03, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>>>
>>>
>>> po 8. 11. 2021 v 5:24 odesílatel Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> napsal:
>>>>
>>>>
>>>> po 8. 11. 2021 v 5:07 odesílatel Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> napsal:
>>>>>>
>>>>>>
>>>>>> +set_errcurrent_query (const char *query)
>>>>>>
>>>>>> You can remove the space prior to (.
>>>>>> I wonder if the new field can be named current_err_query because that's what the setter implies.
>>>>>> current_query may give the impression that the field can store normal query (which doesn't cause exception).
>>>>>> The following code implies that only one of internalquery and current_query would be set.
>>>>>
>>>>> yes, I think so current_query is not a good name too. Maybe query can be good enough - all in ErrorData is related to error
>>>>
>>>> so the name of field can be query, and routine for setting errquery or set_errquery
>>>
>>> and this part is not correct
>>>
>>> <--><-->switch (carg->mode)
>>> <--><-->{
>>> <--><--><-->case RAW_PARSE_PLPGSQL_EXPR:
>>> <--><--><--><-->errcontext("SQL expression \"%s\"", query);
>>> <--><--><--><-->break;
>>> <--><--><-->case RAW_PARSE_PLPGSQL_ASSIGN1:
>>> <--><--><-->case RAW_PARSE_PLPGSQL_ASSIGN2:
>>> <--><--><-->case RAW_PARSE_PLPGSQL_ASSIGN3:
>>> <--><--><--><-->errcontext("PL/pgSQL assignment \"%s\"", query);
>>> <--><--><--><-->break;
>>> <--><--><-->default:
>>> <--><--><--><-->set_errcurrent_query(query);
>>> <--><--><--><-->errcontext("SQL statement \"%s\"", query);
>>> <--><--><--><-->break;
>>> <--><-->}
>>> <-->}
>>>
>>> set_errcurrent_query should be outside the switch
>>>
>>> We want PG_SQL_TEXT for assign statements too
>>>
>>> _t := (select ...);
>>>
>> Please find the new patch, which has the suggested changes.
>
> Now, I have only minor objection
> + <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>
>
> I think so an these text should be little bit enhanced
>
> "the text of query or command of invalid sql statement (dynamic or embedded)"
>
> and
>
> "the location of error of invalid dynamic text, if any"
>
> I am not a native speaker, so I am sure my proposal can be enhanced too.

The proposed statements are much clear, but will wait for other’s suggestion, and will fix it accordingly.

> I have not any other objections
>
> all tests passed without any problem
>
> Regards
>
> Pavel
>
>
>>
>>
>>> Regards
>>>
>>> Pavel