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

On Tue, Oct 15, 2024 at 12:45 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> jian he <jian.universality@gmail.com> writes:
> > On Mon, Oct 14, 2024 at 1:13 AM Tom Lane <tgl@sss.pgh.pa.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.