Postgres Pro
Downloads
Home   >   mailing lists

Re: concerns around pg_lsn - Mailing list pgsql-hackers

From Jeevan Ladhe
Subject Re: concerns around pg_lsn
Date
Msg-id CAOgcT0OLnNo0p1wBiL3+F1Jrk2Y9N=nhE_YTmz_xXCJsmOZd+g@mail.gmail.com
Whole thread Raw
In response to Re: concerns around pg_lsn  (Michael Paquier <michael@paquier.xyz>)
Responses Re: concerns around pg_lsn  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers
Tree view
Hi Michael,

On Thu, Aug 1, 2019 at 1:51 PM Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Aug 01, 2019 at 12:39:26PM +0530, Jeevan Ladhe wrote:
> Here is a patch that takes care of addressing the flag issue including
> pg_lsn_in_internal() and others.

Your original patch for pg_lsn_in_internal() was right IMO, and the
new one is not.  In the numeric and float code paths, we have this
kind of pattern:
if (have_error)
{
    *have_error = true;
    return;
}
else
    elog(ERROR, "Boom. Show is over.");

But the pg_lsn.c portion does not have that.  have_error cannot be
NULL or the caller may fall into the trap of setting it to NULL and
miss some errors at parsing-time.  So I think that keeping the
assertion on (have_error != NULL) is necessary.

Thanks for your concern.

In pg_lsn_in_internal() changes, the caller will get the invalid lsn
in case there are errors:

{code}
    if (len1 < 1 || len1 > MAXPG_LSNCOMPONENT || str[len1] != '/')
    {
        if (have_error)
            *have_error = true;

        return InvalidXLogRecPtr;
    }
{code}

The only thing is that, if the caller cares about the error during
the parsing or not. For some callers just making sure if the given
string was valid lsn or not might be ok, and the return value
'InvalidXLogRecPtr' will tell that. That caller may not unnecessary
declare the flag and pass a pointer to it.

Regards,
Jeevan Ladhe

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: range_agg
Next
From: Takuma Hoshiai
Date:
Subject: Re: Proposal to suppress errors thrown by to_reg*()