On Mon, Mar 30, 2026 at 9:10 PM Amul Sul <sulamul@gmail.com> wrote:
>
> I’ve had a glance over the remaining patches; here are a few comments:
>
> V25-0001:
>
> - if (relation->schemaname)
> + if (escontext == NULL)
> ereport(elevel,
> - (errcode(ERRCODE_UNDEFINED_TABLE),
> - errmsg("relation \"%s.%s\" does not exist",
> - relation->schemaname, relation->relname)));
> + errcode(ERRCODE_LOCK_NOT_AVAILABLE),
> + relation->schemaname ?
> + errmsg("relation \"%s.%s\" does not exist",
> + relation->schemaname, relation->relname) :
> + errmsg("relation \"%s\" does not exist",
> + relation->relname));
> else
> - ereport(elevel,
> - (errcode(ERRCODE_UNDEFINED_TABLE),
> - errmsg("relation \"%s\" does not exist",
> - relation->relname)));
> + ereturn(escontext, InvalidOid,
> + errcode(ERRCODE_LOCK_NOT_AVAILABLE),
> + relation->schemaname ?
> + errmsg("relation \"%s.%s\" does not exist",
> + relation->schemaname, relation->relname) :
> + errmsg("relation \"%s\" does not exist",
> + relation->relname));
> }
>
> The error code changed here -- was this intentional?
it's a copy-paste error.
> --
>
> v25-0009 :
>
> +SELECT CAST('abc'::bpchar AS citext DEFAULT NULL ON CONVERSION ERROR); -- error
> +ERROR: cannot cast type character to citext when DEFAULT expression
> is specified in CAST ... ON CONVERSION ERROR
> +LINE 1: SELECT CAST('abc'::bpchar AS citext DEFAULT NULL ON CONVERSI...
> + ^
> +HINT: Safe type cast for user-defined types are not yet supported.
>
>
> "Should the HINT be this DETAIL? I believe a hint should provide
> guidance for the user to handle the error, but here the user cannot do
> anything with the given information.
> --
>
ok.
> /* Generic macro for applying evaluate_expr */
> -#define ece_evaluate_expr(node) \
> +#define ece_evaluate_expr(node, escontext) \
> ((Node *) evaluate_expr((Expr *) (node), \
> exprType((Node *) (node)), \
> exprTypmod((Node *) (node)), \
> - exprCollation((Node *) (node))))
> + exprCollation((Node *) (node)), \
> + escontext))
>
>
> I think we can simply cast escontext to Node * here, instead of
> having the caller do it.
>
ok.
I found a segmentation fault case in v25, it can be produced by:
SELECT CAST(('{' || sum(1 + 2) || '}')::int[] AS int[] DEFAULT NULL ON
CONVERSION ERROR) FROM (VALUES (1), (2)) sub;
We need set SafeTypeCastExpr->source to NULL. We can do this, because
SafeTypeCastExpr->castexpr sure contain all the
information of SafeTypeCastExpr->source.
If we don't nullify it, preprocess_aggref ends up processing both fields, then
parts of the Aggref node being initialized twice, which can ultimately
lead to a segfault.
--
jian
https://www.enterprisedb.com/
