On Tue, Sep 02, 2025 at 02:41:23PM +0530, Amul Sul wrote:
> The updated version is attached. In addition to the *_opt_error()
> functions, it also renames pg_lsn_in_internal to pg_lsn_in_safe and
> incorporates soft error handling.
Looks globally sensible to me. I was wondering for a bit if the JSON
path parts should be fed pieces of the soft errors that could be
retrieved when the numeric value parsing fails, but that does not seem
worth the extra information.
-pg_lsn_in_internal(const char *str, bool *have_error)
+pg_lsn_in_safe(const char *str, Node *escontext)
[...]
+ ereturn(escontext, InvalidXLogRecPtr,
+ (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+ errmsg("invalid input syntax for type %s: \"%s\"",
+ "pg_lsn", str)));
The same error message is repeated twice. How about using some gotos
and one single ereport instead of two? The same can be said for
numeric_div_safe() and numeric_mod_safe(), for the division-by-0
messages.
@@ -5629,11 +5629,11 @@ timestamp_part_common(PG_FUNCTION_ARGS, bool retnumeric)
[...]
+ PG_RETURN_NUMERIC(numeric_add_safe(int64_to_numeric(date2j(tm->tm_year, tm->tm_mon, tm->tm_mday)),
This part with DTK_JULIAN is hard to parse. Not your responsibility
here as you just replace a function name, just a remark in passing.
--
Michael
