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

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.

I have not any other objections

all tests passed without any problem

Regards

Pavel

>
>
>> Regards
>>
>> Pavel
>>
>