| From: | jian he <jian(dot)universality(at)gmail(dot)com> |
|---|---|
| To: | Amul Sul <sulamul(at)gmail(dot)com> |
| Cc: | Peter Eisentraut <peter(at)eisentraut(dot)org>, Corey Huinker <corey(dot)huinker(at)gmail(dot)com>, Kirill Reshke <reshkekirill(at)gmail(dot)com>, Vik Fearing <vik(at)postgresfriends(dot)org>, Isaac Morland <isaac(dot)morland(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions |
| Date: | 2026-03-30 14:47:32 |
| Message-ID: | CACJufxFOQkmH9KeQNTD0qASbL3i03cLeoyVZdcZ8Fm0rk6eJ5Q@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Mar 30, 2026 at 9:10 PM Amul Sul <sulamul(at)gmail(dot)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.
| Attachment | Content-Type | Size |
|---|---|---|
| v26-0002-CAST-expr-AS-newtype-DEFAULT-expr-ON-CONVERSION-ERROR.patch | text/x-patch | 141.1 KB |
| v26-0001-error-safe-for-casting-text-to-other-types-per-pg_cast.patch | text/x-patch | 9.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jim Jones | 2026-03-30 14:50:22 | Re: [PoC] XMLCast (SQL/XML X025) |
| Previous Message | vignesh C | 2026-03-30 14:44:30 | Re: Skipping schema changes in publication |