Thanks for your time Pavel

> On 09-Nov-2021, at 13:32, Pavel Stehule <pavel.steh...@gmail.com> wrote:
> 
> 
> Hi
> 
> po 8. 11. 2021 v 9:57 odesílatel Dinesh Chemuduru <dinesh.ku...@migops.com> 
> napsal:
>> Thanks Zhihong/Pavel,
>> 
>>> On Mon, 8 Nov 2021 at 10:03, Pavel Stehule <pavel.steh...@gmail.com> wrote:
>>> 
>>> 
>>> po 8. 11. 2021 v 5:24 odesílatel Pavel Stehule <pavel.steh...@gmail.com> 
>>> napsal:
>>>> 
>>>> 
>>>> po 8. 11. 2021 v 5:07 odesílatel Pavel Stehule <pavel.steh...@gmail.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

Reply via email to