Re: Better error reporting from extension scripts (Was: Extend ALTER OPERATOR)

On Tue, Oct 15, 2024 at 12:45 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> jian he <jian(dot)universality(at)gmail(dot)com> writes:
> > On Mon, Oct 14, 2024 at 1:13 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> Right, but we might not have entered either of those previous
> >> if-blocks.
>
> > in src/backend/parser/gram.y
> > your makeRawStmt changes (v4) seem to guarantee that
> > RawStmt->stmt_location >= 0.
>
> Yes, I would expect that any RawStmt we see here will have valid
> stmt_location. What you seem to be missing is that an error could
> be thrown from
>
> > raw_parsetree_list = pg_parse_query(sql);
>
> before execute_sql_string reaches its loop over RawStmts. In that
> case we'll reach script_error_callback with callback_arg.stmt_location
> still being -1.
>
Thanks for your reply.
I think I got it.

if (location < 0 || syntaxerrposition < location ||
(len > 0 && syntaxerrposition > location + len))
{
int slen = strlen(query);
location = len = 0;
.....
}
else
elog(INFO, "should not reached here");

just found out the"elog(INFO, "should not reached here");" part never reached.
in script_error_callback, for syntax error (error while parsing the raw string.)
we can bookkeeping the line number.

so we don't need to loop it again for syntax error in:
for (query = callback_arg->sql; *query; query++)
{
if (--location < 0)
break;
if (*query == '\n')
linenumber++;
}
since we already did it in loop
"for (int loc = 0; loc < slen; loc++)"

i guess, we don't need performance in script_error_callback,
but in script_error_callback arrange code seperate syntax error(raw
parser) and other error seems good.
please check the attached minor refactor.