Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions

On Tue, Dec 2, 2025 at 1:23 PM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> On Mon, Dec 1, 2025 at 8:09 PM Amul Sul <sulamul(at)gmail(dot)com> wrote:
> >
> > Since you declared float8_div_safe() as static inline, I believe it
> > wouldn't have any performance degradation since most compilers
> > optimize it. Also, I suggest you pass the ErrorSafeContext to
> > float_overflow_error(), float_underflow_error(), and
> > float_zero_divide_error() so that you can avoid duplicating error
> > messages.
> >
> hi.
>
> First I want to use ereturn, then I found out
> float_overflow_error, float_underflow_error, float_zero_divide_error
> used both in float4, float8.
> ereturn would not be appropriate for both types.
> so I choose errsave.
> for these 3 functions, now it looks like:

Make sense, thanks for updating the patch.

Regarding v13-0019 and v13-0020 patches, I have a bunch of comments
for the code, but I'd like to understand the implemented design first.
I have some basic questions regarding the commit message and code
comment, as follows:

"
We cannot simply prohibit user-defined functions in pg_cast for safe cast
evaluation because CREATE CAST can also utilize built-in functions. So, to
completely disallow custom casts created via CREATE CAST used in safe cast
evaluation, a new field in pg_cast would unfortunately be necessary.
"

I might not have understood the implementation completely -- can't we
use fmgr_last_builtin_oid to detect built-in functions?
--

+ /*
+ * We have to to use CoerceUnknownConstSafe rather than
+ * coerce_to_target_type. because coerce_to_target_type is not error
+ * safe.
+ */

How difficult would it be to modify coerce_to_target_type() to make it
error safe?

Could we utilize the existing type casting infrastructure for this, perhaps by
simply considering the evolution and use of the additional default
expression? If we could, the resulting implementation would be very
clean, IMHO.

Kindly excuse and point me if that is already discussed.
--

Here are few comments for v13-0018:

static inline void point_add_point(Point *result, Point *pt1, Point *pt2);
+static inline bool point_add_point_safe(Point *result, Point *pt1, Point *pt2,
+ Node *escontext);

+static inline float8 point_dt_safe(Point *pt1, Point *pt2, Node *escontext);
static inline float8 point_sl(Point *pt1, Point *pt2);

I think we should avoid introducing the "_safe" version of static
routines in the .c file. Instead, we can add the Node *escontext
argument to the existing routines, similar to how single_decode() was
previously modified to accept it.
--

-static void poly_to_circle(CIRCLE *result, POLYGON *poly);
+static bool poly_to_circle_safe(CIRCLE *result, POLYGON *poly, Node
*escontext);

Following the previous suggestion, please keep the existing function
as it is and just add one more argument to it.
--

}

+
static inline float4
float4_mi(const float4 val1, const float4 val2)
{

A spurious change.
--

Regards,
Amul