Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1vGo72-004Q8I-9E for pgsql-hackers@arkaria.postgresql.org; Thu, 06 Nov 2025 00:44:27 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1vGlaH-004HjR-HB for pgsql-hackers@arkaria.postgresql.org; Wed, 05 Nov 2025 22:02:28 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1vGlaH-004HiS-5W for pgsql-hackers@lists.postgresql.org; Wed, 05 Nov 2025 22:02:28 +0000 Received: from mail-il1-x130.google.com ([2607:f8b0:4864:20::130]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1vGla8-006BSf-0w for pgsql-hackers@lists.postgresql.org; Wed, 05 Nov 2025 22:02:22 +0000 Received: by mail-il1-x130.google.com with SMTP id e9e14a558f8ab-4332b4368ecso1927995ab.1 for ; Wed, 05 Nov 2025 14:02:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1762380016; x=1762984816; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=OuFHmzoLMAvwmuSEIuufe4GT0V3ZWlIiHJbl+AOe8rw=; b=ZvmxtzTYWA32qwH/jsEUQ1yiA+obMwGB8xjNUbyfTht2Pgmb4r3dv3umFiV/W+fFkq r8sBVTwVUx1o0+s521CbEeKdZUTiHcz1YG8KSql7/y86ZVYaokdxmJ0dYxrLzX5+MC71 8aNGwtyTLx8gwLAQZOJOQw7NGjslzLXcGC8XXfwJ06wVzD5ExiJdgpQL8ngo12YQyGqE TLAmJIeVJL1JbBdRgPo07iVXuqL2vtM7T/84dj0tPzS/HUq0kfp7TpzzDf6VeC3IMJLQ 5LwKAP5Eqs1uSLY8s53/CYJMoGyRKaN+9XbKmQ6OGEOZxh/sEI6snCfB+4xCwDuum+oF +KRg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1762380016; x=1762984816; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=OuFHmzoLMAvwmuSEIuufe4GT0V3ZWlIiHJbl+AOe8rw=; b=Pdgrn/4lTtGfGHbeXcBK8DEUwatwKGYvZRukEbouo/G2k6vO4HgJ8/ccT5GnLvkwN9 6egoU9+lGDdpy7gwgOXPF3WZeT9OfvuI4kafjXJ0fuALecA3ueqZYlPl4S/RhkO0ta/J fzgedkOcu/81pdhjRojoVZqD/H8+UwrNqtJDXI2cdb7e/XjCuY6PdVJESAS7LtTkMsCk vLHepNXLEEWlbjPcQPlrdmSPN0dHzTauWaLhKz1RL1RednG2YJ3S0izc7Zc/fIyrVxCI 0XN3b4le7Sg8mY0hWIRQdPIpv2RVk6ioVVJjXYYpC99bfvAYbi/nIzMMMFx4tYkHNbc7 bRRg== X-Forwarded-Encrypted: i=1; AJvYcCXoOn5e3JrAhjPHHKnnW7IXPCRRy1V6i+DBh126Tlu80yyy/hVuNeu989KGTf/x3H05ai7VxSWZvWe8Q7+V@lists.postgresql.org X-Gm-Message-State: AOJu0Yzm0SUEH2mTrdOjDoHqkUcmsYVXkJmuVVpZqwHpg28Ki96w/yRs CWbtGdoVIopzbXLVXtqTaLPu7ybhgySz+8uGI7zy8RcDl5I6L6i3/DCve4+r5cjjXkZrPLXLIlC UzQc5hHnPXV6nEr3TwayKAcTsF8cxVZU= X-Gm-Gg: ASbGnctTjIOVmDz2ydfND8YHX2H+1ev6SAl9tSEPwX4vA1sMqYV9EslONloRHfINiLH YJl+kSI3lnulAkU++yEYKugCIpoXTo4AKXSENU0wKRoBthD+yQNE7yzr8DaBk4EV91Cg/L6/I0c PppepljcPxHAP6LFY93Aw5edCmQI1IQ7zflhpUiUuyql6dyB0rqwcsVpwcxZUQ3VHIR5ETkEEvF cTsMxBaePOiEi6ynmxvbzAXGg0hD6ZwbeMEKivsMTMzbu6pQtcFHTWUPg0r X-Google-Smtp-Source: AGHT+IHUiInzUlYV3dA0k2hUFLAWHeFl2WhaUZkCg6wOqCxMnvPbnnTVt6obppUGrvEuklNAMBCmumSgfjXsLUdISyg= X-Received: by 2002:a05:6e02:2589:b0:433:2a39:1b92 with SMTP id e9e14a558f8ab-4334079ff03mr74330385ab.11.1762380014850; Wed, 05 Nov 2025 14:00:14 -0800 (PST) MIME-Version: 1.0 References: <04afcd1f-ed7d-4c0a-add1-50e3719ccbf9@postgresfriends.org> <762ae707-7fdc-43d8-a77a-3a10d12ce21d@postgresfriends.org> In-Reply-To: From: Corey Huinker Date: Wed, 5 Nov 2025 17:00:01 -0500 X-Gm-Features: AWmQ_bmBZOx4kqMZDUmaHAOeImDpDFdr8nkMHofOrEA0gFKKv87fswr7CVDD9qQ Message-ID: Subject: Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions To: jian he Cc: Vik Fearing , Isaac Morland , pgsql-hackers@lists.postgresql.org Content-Type: multipart/alternative; boundary="000000000000be95570642e0126d" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000be95570642e0126d Content-Type: text/plain; charset="UTF-8" > > The above result shows type casts using functions which cannot be error > safe. > Money type related casts still can not be error safe. > > Cast from circle to polygon cannot be error safe because the associated > cast > function (pg_cast.castfunc) is written in SQL > (see src/backend/catalog/system_functions.sql LINE 112). > It appears impossible to make SQL language functions error safe, because > fmgr_sql ignores fcinfo->context. > It is inevitable that some typecast functions are not type safe, either because they cannot easily be made so, or because they come from an extension that has not yet made them so. > eval_const_expressions cannot be error safe, so we need to handle > source_expr as an UNKNOWN constant in an error safe beforehand. > For example, we need handle ('1' AS DATE) in an error safe way > for > SELECT CAST('1' AS date DEFAULT '2011-01-01' ON ERROR); > > Since we must handle the source_expr when it is an UNKNOWN constant in an > error safe way, we can apply the same handling when source_expr is a > Const whose type is not UNKNOWN. > For example: > SELECT CAST('[(1,2),(3,4)]'::path AS polygon DEFAULT NULL ON CONVERSION > ERROR); > In the case you've presented here, the cast to type "path" doesn't need to be safe, but the cast path->polygon does. Or rather, if it isn't safe we should raise an error. > > so I introduced: > evaluate_expr_safe: error safe version of evaluate_expr > CoerceUnknownConstSafe: tests whether an UNKNOWN Const can be coerced to > the > target type. > Good. That's the missing bit I knew we needed to add. Sorry I wasn't able to find time. Issue 1: +++ b/doc/src/sgml/syntax.sgml @@ -2106,6 +2106,10 @@ CAST ( expression AS type The CAST syntax conforms to SQL; the syntax with :: is historical PostgreSQL usage. + It can also be written as: + +CAST ( expression AS type ERROR ON CONVERSION ERROR ) + The wording implies that this syntax is a new shortcut to a previously established ON CONVERSION ERROR syntax, when it was the only way to do it until this patch. Issue 2: s/is historical/is the original/ s/It can be also be written as:/The equivalent ON CONVERSION ERROR behavior is:/ + /* + * Use evaluate_expr_safe to pre-evaluate simple constant cast + * expressions early, in a way that tolter errors. typo on tolter? This happens in 2 places. Issue 3: + errhint("Currently CAST ... DEFAULT ON CONVERSION ERROR does not support this cast"), Suggest: errhint("Explicit cast is defined but definition is not declared as safe") This hint helps distinguish the fact that it's the cast function getting in the way of this cast-on-default working. If the cast had been defined as IO then we wouldn't have had a problem. It is true that we presently have no means of declaring a cast safe aside from making it so in pg_proc.dat, but that's coming shortly. Issue 4: catversion.h is not updated. Which isn't a bad thing because that brings me to... Issue 5: I think 0019 is a bit big for a committer to digest all in one sitting. Currently it: - introduces the type-safe cast node - introduces the cast on default syntax - redefines - adds in test cases for all safe functions defined in patches 1-18. As tedious as it might be, I think we want to move this patch to the front, move all pg_cast.dat changes to their respective patches that introduce that datatype's safe typecast function, as well as the test cases that are made possible by that new safe typecast. That will make it easier to ensure that each new cast has test coverage. Yes, that's going to be a lot of catversion bumps, requiring somebody to fudge the dates as we presently only allow for 10 catversion bumps in a day, but the committer will either combine a few of the patches or spread the work out over a few days. Overall: I like where this patchset is going. The introduction of error contexts has eliminated a lot of the issues I was having carrying the error state forward into the next eval step. --000000000000be95570642e0126d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
The above result shows type casts using functions which cannot be error saf= e.
Money type related casts still can not be error safe.

Cast from circle to polygon cannot be error safe because the associated cas= t
function (pg_cast.castfunc) is written in SQL
(see src/backend/catalog/system_functions.sql LINE 112).
It appears impossible to make SQL language functions error safe, because fmgr_sql ignores fcinfo->context.

It= is inevitable that some typecast functions are not type safe, either becau= se they cannot easily be made so, or because=C2=A0they come from an extensi= on that has not yet made them so.


= =C2=A0
eval_const_expressions cannot be error safe, so we need to handle
source_expr as an UNKNOWN constant in an error safe beforehand.
For example, we need handle ('1' AS DATE) in an error safe way
for
SELECT CAST('1' AS date=C2=A0 DEFAULT '2011-01-01' ON ERROR= );

Since we must handle the source_expr when it is an UNKNOWN constant in an error safe way, we can apply the same handling when source_expr is a
Const whose type is not UNKNOWN.
For example:
SELECT CAST('[(1,2),(3,4)]'::path AS polygon DEFAULT NULL ON CONVER= SION ERROR);

In the case you've pre= sented here, the cast to type "path" doesn't need to be safe,= but the cast path->polygon does. Or rather, if it isn't safe we sho= uld raise an error.
=C2=A0

so I introduced:
evaluate_expr_safe: error safe version of evaluate_expr
CoerceUnknownConstSafe: tests whether an UNKNOWN Const can be coerced to th= e
target type.

Good. That's the missi= ng bit I knew we needed to add. Sorry I wasn't able to find time.
=

Issue 1:
+++ b/doc/src/sgml/syntax.sgml
@@ -210= 6,6 +2106,10 @@ CAST ( <replaceable>expression</replaceable> AS= <replaceable>type</replaceable>
=C2=A0 =C2=A0 =C2=A0The <= ;literal>CAST</literal> syntax conforms to SQL; the syntax with=C2=A0 =C2=A0 =C2=A0<literal>::</literal> is historical <pr= oductname>PostgreSQL</productname>
=C2=A0 =C2=A0 =C2=A0usage.+ =C2=A0 =C2=A0It can also be written as:
+<synopsis>
+CAST (= <replaceable>expression</replaceable> AS <replaceable>ty= pe</replaceable> ERROR ON CONVERSION ERROR )
+</synopsis>
The wording imp= lies that this syntax is a new=C2=A0shortcut to a previously established ON= CONVERSION ERROR syntax, when it was the only way to do it until this patc= h.


Issue 2:
s/is historical/is the original/
s/It can be also be written as:/The equivalent ON = CONVERSION ERROR behavior is:/

+ /= *
+ * Use evaluate_expr_safe to pre-evaluate simple constant cast+ * expressions early, in a way that tolter errors.

typo on tolter? This happens in= 2 places.


Issue 3:
+ = errhint("Currently CAST ... DEFAULT ON CONVERSION ERROR does not su= pport this cast"),

Suggest: errhint("Explicit c= ast is defined but definition is not declared as safe")

=
This hint helps distinguish the fact that it's the cast func= tion getting in the way of this cast-on-default working. If the cast had be= en defined as IO then we wouldn't have had a problem.

It is true that we presently have no means of declaring a cast safe= aside from making it so in pg_proc.dat, but that's coming shortly.


=
=C2=A0
Issue 4:

catversion.h is not updat= ed. Which isn't a bad thing because that brings me to...


= Issue 5:

I think 0019 is a bit big for a committer to dig= est all in one sitting. Currently it:

- introduces the ty= pe-safe cast node
- introduces the cast on default syntax
- redefines=C2=A0
- adds in test cases for all safe functions = defined in patches 1-18.

As tedious as it might be= , I think we want to move this patch to the front, move all pg_cast.dat cha= nges to their respective patches that introduce that datatype's safe ty= pecast function, as well as the test cases that are made possible by that n= ew safe typecast. That will make it easier to ensure that each new cast has= test coverage.

Yes, that's going to be a lot = of catversion=C2=A0bumps, requiring somebody to fudge the dates as we prese= ntly only allow for 10 catversion=C2=A0bumps in a day, but the committer wi= ll either combine a few of the patches or spread the work out over a few da= ys.

Overall:

I like where= this patchset is going. The introduction of error contexts has eliminated = a lot of the issues I was having carrying the error state forward into the = next eval step.
--000000000000be95570642e0126d--