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