Re: [PROPOSAL] new diagnostic items for the dynamic sql - Mailing list pgsql-hackers



Hi

po 8. 11. 2021 v 9:57 odesílatel Dinesh Chemuduru <dinesh.kumar@migops.com> napsal:

Thanks Zhihong/Pavel,

On Mon, 8 Nov 2021 at 10:03, Pavel Stehule <pavel.stehule@gmail.com> wrote:

po 8. 11. 2021 v 5:24 odesílatel Pavel Stehule <pavel.stehule@gmail.com> napsal:

po 8. 11. 2021 v 5:07 odesílatel Pavel Stehule <pavel.stehule@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.

I have not any other objections

all tests passed without any problem

Regards

Pavel

 

Regards

Pavel