builtins: use IdentityReturnType when possible #70469
Conversation
|
This change is |
rafiss
force-pushed
the
use-identity-return-type
branch
from
027e4b2 to
7857b72
Compare
| } | ||
| return args[0].ResolvedType() | ||
| }, | ||
| Types: tree.ArgTypes{{"base", types.AnyTuple}, {"from_json", types.Jsonb}}, |
There was a problem hiding this comment.
The reason I set this to be types.Any is to match the argument type in Postgres itself, which is any, and not tuple. I'm not sure if this matters, but that's why I did it:
postgres=# select proargtypes[0]::regtype from pg_proc where proname = 'jsonb_populate_record';
proargtypes
-------------
anyelement
(1 row)
There was a problem hiding this comment.
hm, so looks like PG must do some other validation on the input args
rafiss@127:postgres> select * from json_populate_record(null::int, '{"a":1,"b":2}')
first argument of json_populate_record must be a row type
if you try this in crdb:
root@:26257/defaultdb> select * from json_populate_record(null::int, '{"a":1,"b":2}');
ERROR: anonymous records cannot be used with json_populate_record
which seems like a misleading message
There was a problem hiding this comment.
interesting, postgres caches records, wonder if we should do something similar:
https://github.com/postgres/postgres/blob/4548c76738b368a11a5dad052f9653a349eeb52c/src/backend/utils/adt/jsonfuncs.c#L3815-L3825
anyway yeah it's checked internally i think? the NULL is checked here
https://github.com/postgres/postgres/blob/4548c76738b368a11a5dad052f9653a349eeb52c/src/backend/utils/adt/jsonfuncs.c#L3416-L3422
though i can't find where it returns the signature error if it is a not-null not-tuple. maybe we should just change the tree.MustBeDTuple check here:
There was a problem hiding this comment.
It looks like PG checks the not-null not-tuple case in the same place as above. there's also this fun message that PG has, which probably doesn't apply to CRDB's type system:
rafiss@127:postgres> select * from json_populate_recordset(null, '{"a":1,"b":2}')
could not determine polymorphic type because input has type unknown
Release note: None
rafiss
force-pushed
the
use-identity-return-type
branch
from
7857b72 to
e820ea2
Compare
|
this is ready for another review |
The function accepts types.Any, but at execution time it actually requires a tuple type. This is now checked. The behavior of checking at execution time matches PostgreSQL. Release note: None
rafiss
force-pushed
the
use-identity-return-type
branch
from
e820ea2 to
23f4ee3
Compare
|
bors r=otan |
|
@rafiss thanks for fixing this issue! |
|
Build succeeded: |
fixes #70475
see individual commits
Release note: None