From: | Nikhil Benesch <nikhil(dot)benesch(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Mohamed Wael Khobalatte <mkhobalatte(at)grubhub(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Why does to_json take "anyelement" rather than "any"? |
Date: | 2020-11-06 03:51:36 |
Message-ID: | d190ee62-add5-34a1-a1dd-d40a3fdca29c@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 11/5/20 8:58 PM, Tom Lane wrote:
> "any" is a dinosaur IMO. It's definitely lower-level than anyelement;
> for example the function has to be prepared to deal with raw "unknown"
> literals. So I feel like the proposed solution here is a bit of a hack.
I see what you are saying, but since the code for to_jsonb is shared with
the code for jsonb_build_array and jsonb_build_object, which already
handle the pitfalls of "any", the patch seems to be literally this
simple:
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index c01da4bf01..11adf748c9 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8466,7 +8466,7 @@
prosrc => 'json_object_two_arg' },
{ oid => '3176', descr => 'map input to json',
proname => 'to_json', provolatile => 's', prorettype => 'json',
- proargtypes => 'anyelement', prosrc => 'to_json' },
+ proargtypes => 'any', prosrc => 'to_json' },
{ oid => '3261', descr => 'remove object fields with null values from json',
proname => 'json_strip_nulls', prorettype => 'json', proargtypes => 'json',
prosrc => 'json_strip_nulls' },
@@ -9289,7 +9289,7 @@
proargtypes => '_text _text', prosrc => 'jsonb_object_two_arg' },
{ oid => '3787', descr => 'map input to jsonb',
proname => 'to_jsonb', provolatile => 's', prorettype => 'jsonb',
- proargtypes => 'anyelement', prosrc => 'to_jsonb' },
+ proargtypes => 'any', prosrc => 'to_jsonb' },
{ oid => '3265', descr => 'jsonb aggregate transition function',
proname => 'jsonb_agg_transfn', proisstrict => 'f', provolatile => 's',
prorettype => 'internal', proargtypes => 'internal anyelement',
I think my argument is that regardless of which of
{any,anyelement,anycompatible} is best, it seems like to_jsonb,
jsonb_build_array, and jsonb_build_object should all use the same
type.
> What I'm wondering about as I think about this is why we don't allow
> unknown literals to be resolved as text when matching to anyelement.
> Maybe that was intentional, or maybe just overly conservative; or maybe
> there is a good reason for it. I don't recall, but it would be worth
> excavating in the list archives to see if it was discussed when the
> polymorphic types were being designed.
I excavated two separate threads from 2003 from when you and Joe Conway
were designing SQL99 array support and the initial polymorphic types:
https://www.postgresql.org/message-id/3E701869.4020301%40joeconway.com
https://www.postgresql.org/message-id/1272.1048633920%40sss.pgh.pa.us
I didn't see anything obvious about unknown coercions, though I
certainly could have overlooked something. For what it's worth, the
error message "could not determine polymorphic type because input has
type unknown" has existed, with slightly different wording, since
the very first commit of the feature:
https://github.com/postgres/postgres/commit/730840c9b (parse_coerce.c, L840)
> A relevant data point is that we *do* allow the case with the more
> recently added "anycompatible" polymorphics:
>
> <...snipped examples...>
>
> So even if we decide that changing the rules for "anyelement" is
> too scary, I think switching to_json to anycompatible would be
> preferable to switching it to "any".
Oh these new polymorphic types are interesting. I hadn't seen these.
So, I don't feel particularly qualified to determine how to proceed.
These are the options that I would be excited about:
1. Switch to_jsonb to take "any", as in the above patch.
2. Convert all of to_jsonb, jsonb_build_array, and jsonb_build_object
to use the new "anycompatible" type.
3. Switch to_jsonb to take "anyelement", but change "anyelement" and
friends so that "unknown" arguments are coereced to text.
Would someone care to offer guidance on which path to choose?
Nikhil
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2020-11-06 03:55:07 | Re: Dumping/restoring fails on inherited generated column |
Previous Message | k.jamison@fujitsu.com | 2020-11-06 03:44:50 | RE: [Patch] Optimize dropping of relation buffers using dlist |