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