public inbox for [email protected]
help / color / mirror / Atom feedFrom: Corey Huinker <[email protected]>
To: zengman <[email protected]>
To: [email protected]
Cc: jian he <[email protected]>
Cc: David G. Johnston <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Vik Fearing <[email protected]>
Subject: Re: implement CAST(expr AS type FORMAT 'template')
Date: Mon, 23 Mar 2026 16:20:14 -0400
Message-ID: <CADkLM=fm35Nuf7++P815o+qHpa3WhHg=Fw=WzRS7NCrkRF2p5g@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CACJufxGqm7cYQ5C65Eoh1z-f+aMdhv9_7V=NoLH_p6uuyesi6A@mail.gmail.com>
<[email protected]>
<CACJufxF4OW=x2rCwa+ZmcgopDwGKDXha09qTfTpCj3QSTG6Y9Q@mail.gmail.com>
<[email protected]>
<CACJufxEH-8UPdbPoUoqNRaiOePw+s2W2DG4OpXtoSYDaW30oAg@mail.gmail.com>
<CAKFQuwZXYxPw4wofq+zZ5D7nH0sHDBwr--qku9jWYv7dS+P8RQ@mail.gmail.com>
<CACJufxGTeSnHn7w8r-qoQrqwCFQSC+CbiEthzpHKUd+bPr_0mA@mail.gmail.com>
<[email protected]>
<CACJufxGbsc3g6RZf0rZpKYxffK6vGkc_OsTmX54uS0kFcVBtvA@mail.gmail.com>
<CAKFQuwaG00tsXc2oSQwjdSnPQ=JuH+T8Oeejb3sO5TeqUHsfUQ@mail.gmail.com>
<CADkLM=cvhJBrCj3GV+jXfpb5+U1Z33nsvCXo-=ZkDyaSPbS-1g@mail.gmail.com>
<CAKFQuwYrwM64YnvZ5aL1DEWG50uW-1SrXesSKkmvpxXXrxTW-g@mail.gmail.com>
<[email protected]>
<CACJufxH+soqLj_AuMQj-_jxunVQKX-HBQA_3_3vmV1jTRyZ1hA@mail.gmail.com>
<CACJufxGhOOYnPtmsuVocXVNyx2gB3K68ze67CM4LUqQeW--S8g@mail.gmail.com>
<[email protected]>
>
>
> Typos
> - parse_expr.c:2745: "formmatted" → "formatted"
> - parse_coerce.c:156: "Cocerce" → "Coerce"
>
> In `coerce_type_with_format()` (parse_coerce.c):
>
> 1. The check `if (fmtcategory != TYPCATEGORY_STRING && fmtcategory !=
> TYPCATEGORY_UNKNOWN)`
> could be moved earlier. It doesn't depend on any other
> calculations, so failing fast here would avoid unnecessary work.
>
> 2. consider using` list_make2(node, format)` instead of `list_make1() +
> lappend()`.
>
> --
> Regards,
> Man Zeng
Surya and I did a pair-review of this. In addition to the notes above
(which we agree with), we have the following notes:
+ if (inputBaseTypeId == targetBaseTypeId)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot cast type %s to %s while
using a format template",
+ format_type_be(inputBaseTypeId),
+
format_type_be(targetBaseTypeId)),
+ errdetail("binary coercible type cast is
not supported while using a format template"),
+ parser_coercion_errposition(pstate,
location, node));
This could use a bit more explanation in a comment - is it because there is
no plausible type that can take a FORMAT and be cast to itself?
+ if (s_typcategory != TYPCATEGORY_NUMERIC &&
+ s_typcategory != TYPCATEGORY_STRING &&
+ s_typcategory != TYPCATEGORY_DATETIME &&
+ s_typcategory != TYPCATEGORY_TIMESPAN)
In situations like this, the committers have shown a strong preference for
switch() statements. Though it may make more sense to package this if into
a static function is_formattable_type() or similar.
+ if (t_typcategory == TYPCATEGORY_STRING)
+ funcname = list_make2(makeString("pg_catalog"),
+
makeString("to_char"));
...
+ else
+ elog(ERROR, "failed to find conversion function from %s to
%s while using a format template",
+ format_type_be(inputTypeId),
format_type_be(targetTypeId));
Similar thing here, committers will probably want to see a switch.
+create function imm_const() returns text as $$ begin return 'YYYY-MM-DD';
end; $$ language plpgsql immutable;;
+select cast(col1 as date format imm_const()) from tcast;
double-;
Overall, the test coverage appears complete and the close mirroring of
existing tests makes the intention very clear.
view thread (56+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: implement CAST(expr AS type FORMAT 'template')
In-Reply-To: <CADkLM=fm35Nuf7++P815o+qHpa3WhHg=Fw=WzRS7NCrkRF2p5g@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox