Обсуждение: BUG #15940: json_populate_recordset fails with ERROR: record type has not been registered
BUG #15940: json_populate_recordset fails with ERROR: record type has not been registered
От
PG Bug reporting form
Дата:
The following bug has been logged on the website: Bug reference: 15940 Logged by: Jaroslav Sivy Email address: yarexeray@gmail.com PostgreSQL version: 11.2 Operating system: freebsd Description: Following query works fine in previous freebsd versions SELECT id_item FROM json_populate_recordset(null::record, '[{"id_item":776}]') AS ( id_item int );
> On 06-Aug-2019, at 12:11 PM, PG Bug reporting form <noreply@postgresql.org> wrote: > > The following bug has been logged on the website: > > Bug reference: 15940 > Logged by: Jaroslav Sivy > Email address: yarexeray@gmail.com > PostgreSQL version: 11.2 > Operating system: freebsd > Description: > > Following query works fine in previous freebsd versions > > SELECT > id_item > FROM json_populate_recordset(null::record, '[{"id_item":776}]') > AS > ( > id_item int > ); >
Re: BUG #15940: json_populate_recordset fails with ERROR: recordtype has not been registered
От
Michael Paquier
Дата:
On Tue, Aug 06, 2019 at 06:41:34AM +0000, PG Bug reporting form wrote: > Following query works fine in previous freebsd versions > > SELECT > id_item > FROM json_populate_recordset(null::record, '[{"id_item":776}]') > AS > ( > id_item int > ); This visibly is a regression between 11 and 10, and one bisect later here is the culprit: commit: 37a795a60b4f4b1def11c615525ec5e0e9449e05 author: Tom Lane <tgl@sss.pgh.pa.us> date: Thu, 26 Oct 2017 13:47:45 -0400 Support domains over composite types. -- Michael
Вложения
Re: BUG #15940: json_populate_recordset fails with ERROR: record typehas not been registered
От
Dmitry Dolgov
Дата:
> On Tue, Aug 6, 2019 at 9:32 AM Michael Paquier <michael@paquier.xyz> wrote: > > On Tue, Aug 06, 2019 at 06:41:34AM +0000, PG Bug reporting form wrote: > > Following query works fine in previous freebsd versions > > > > SELECT > > id_item > > FROM json_populate_recordset(null::record, '[{"id_item":776}]') > > AS > > ( > > id_item int > > ); > > This visibly is a regression between 11 and 10, and one bisect later > here is the culprit: > commit: 37a795a60b4f4b1def11c615525ec5e0e9449e05 > author: Tom Lane <tgl@sss.pgh.pa.us> > date: Thu, 26 Oct 2017 13:47:45 -0400 > Support domains over composite types. Looks like there are tests for such behaviour with exactly this error, is there a chance it was a feature? But anyway before this change there was an invocation of get_call_result_type in the branch if (have_record_arg && PG_ARGISNULL(0)) Adding similar stuff to the current implementation make this error to disappear for me and passes tests (except those where we actually expect this error): --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -3647,7 +3647,19 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname, } } else + { + TupleDesc tupdesc; + rec = NULL; + if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("function returning record called in context " + "that cannot accept type record"))); + + cache->c.io.composite.base_typid = tupdesc->tdtypeid; + cache->c.io.composite.base_typmod = tupdesc->tdtypmod; + } /* if the json is null send back an empty set */ if (PG_ARGISNULL(json_arg_num))
Dmitry Dolgov <9erthalion6@gmail.com> writes: > On Tue, Aug 6, 2019 at 9:32 AM Michael Paquier <michael@paquier.xyz> wrote: >> On Tue, Aug 06, 2019 at 06:41:34AM +0000, PG Bug reporting form wrote: >>> Following query works fine in previous freebsd versions >>> >>> SELECT >>> id_item >>> FROM json_populate_recordset(null::record, '[{"id_item":776}]') >>> AS >>> ( >>> id_item int >>> ); > Looks like there are tests for such behaviour with exactly this error, > is there a chance it was a feature? Yeah, my first reaction to this bug report was "didn't we fix this already?" --- it's real close to some other behaviors we fixed in that code. In an ideal world we'd decide that this query is wrong and we should not support it. The point of json_populate_recordset is to take the result rowtype from its first argument; if you want to take the result rowtype from the call context, you should be using json_to_recordset. I really don't like semantics as squishy as "we'll take the result type from the first argument except if it's exactly a null of type RECORD, and then we'll look somewhere else for the type". Yeah, it worked that way before v11, but IMO that was a bug. Now, it's surely true that if we are going to take that attitude we ought to throw some better error about it than "record type has not been registered"; that's my fault for not thinking harder about the UX. (I think that there are some related cases where < v11 already threw that error, and it seemed sufficient to keep on doing so.) However, the bigger part of the UX problem is that if you leave off the AS, you get regression=# SELECT * FROM json_populate_recordset(null::record, '[{"id_item":776}]'); ERROR: a column definition list is required for functions returning "record" which is a bit misleading; the parser is seeing that the resolved polymorphic result type of json_populate_recordset is just "record" and complaining about that. Ideally it would counsel that you should use json_to_recordset plus an AS clause, but I don't see any very sane way to do that. If people do what the error tells them to, and it still doesn't work, they're not being unreasonable to complain. Maybe we have to stay bug-compatible with the old behavior just because we can't generate a reasonable error report. But ugh. A related issue that it'd be nice to have a better answer for is "what happens if the two type-info sources conflict?" For example, regression=# SELECT id_item FROM json_populate_recordset(row(1,2), '[{"id_item":776}]') AS ( id_item int ); ERROR: function return row and query-specified return row do not match DETAIL: Returned row contains 2 attributes, but query expects 1. This is clearly pilot error, but it could be pretty confusing. Thoughts? regards, tom lane
Re: BUG #15940: json_populate_recordset fails with ERROR: record typehas not been registered
От
Merlin Moncure
Дата:
On Tue, Aug 6, 2019 at 2:38 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Dmitry Dolgov <9erthalion6@gmail.com> writes: > > On Tue, Aug 6, 2019 at 9:32 AM Michael Paquier <michael@paquier.xyz> wrote: > >> On Tue, Aug 06, 2019 at 06:41:34AM +0000, PG Bug reporting form wrote: > >>> Following query works fine in previous freebsd versions > >>> > >>> SELECT > >>> id_item > >>> FROM json_populate_recordset(null::record, '[{"id_item":776}]') > >>> AS > >>> ( > >>> id_item int > >>> ); > > > Looks like there are tests for such behaviour with exactly this error, > > is there a chance it was a feature? > > Yeah, my first reaction to this bug report was "didn't we fix this > already?" --- it's real close to some other behaviors we fixed in > that code. > > In an ideal world we'd decide that this query is wrong and we should > not support it. The point of json_populate_recordset is to take the > result rowtype from its first argument; if you want to take the result > rowtype from the call context, you should be using json_to_recordset. > I really don't like semantics as squishy as "we'll take the result > type from the first argument except if it's exactly a null of type > RECORD, and then we'll look somewhere else for the type". Yeah, it > worked that way before v11, but IMO that was a bug. > > Now, it's surely true that if we are going to take that attitude we > ought to throw some better error about it than "record type has not been > registered"; that's my fault for not thinking harder about the UX. > (I think that there are some related cases where < v11 already > threw that error, and it seemed sufficient to keep on doing so.) still does. postgres=# select version(); version ───────────────────────────────────────────────────────────────────────────────────────────────────────── PostgreSQL 11.1 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28), 64-bit (1 row) postgres=# select (row(1,2,3)).*; ERROR: record type has not been registered For posterity I agree that OP was essentially exploiting undefined, or at least poorly defined, behavior. For my money, I'd advise using this function for cases where you don't want to use an in place type, just a column list: postgres=# SELECT * from json_to_recordset('[{"id_item":776}]') as (id_item int); id_item ───────── 776 merlin
Re: BUG #15940: json_populate_recordset fails with ERROR: recordtype has not been registered
От
Michael Paquier
Дата:
On Tue, Aug 06, 2019 at 02:53:45PM -0500, Merlin Moncure wrote: > For posterity I agree that OP was essentially exploiting undefined, or > at least poorly defined, behavior. For my money, I'd advise using > this function for cases where you don't want to use an in place type, > just a column list: If I were to change something here, that would be this error string exposed to the user. With the current error, there is no real way that the user knows what is wrong and why he/she should not do that. Could it be possible to add some recommendation for example on top of an error "cannot do that because blah"? -- Michael
Вложения
Re: BUG #15940: json_populate_recordset fails with ERROR: record typehas not been registered
От
Merlin Moncure
Дата:
On Thu, Aug 8, 2019 at 9:35 PM Michael Paquier <michael@paquier.xyz> wrote: > > On Tue, Aug 06, 2019 at 02:53:45PM -0500, Merlin Moncure wrote: > > For posterity I agree that OP was essentially exploiting undefined, or > > at least poorly defined, behavior. For my money, I'd advise using > > this function for cases where you don't want to use an in place type, > > just a column list: > > If I were to change something here, that would be this error string > exposed to the user. With the current error, there is no real way > that the user knows what is wrong and why he/she should not do that. > Could it be possible to add some recommendation for example on top of > an error "cannot do that because blah"? Good question. Maybe something like, "ERROR: Unable to expand generic rowtype. HINT: Try using explicit cast on rowtype" merlin
Michael Paquier <michael@paquier.xyz> writes: > On Tue, Aug 06, 2019 at 02:53:45PM -0500, Merlin Moncure wrote: >> For posterity I agree that OP was essentially exploiting undefined, or >> at least poorly defined, behavior. For my money, I'd advise using >> this function for cases where you don't want to use an in place type, >> just a column list: > If I were to change something here, that would be this error string > exposed to the user. With the current error, there is no real way > that the user knows what is wrong and why he/she should not do that. > Could it be possible to add some recommendation for example on top of > an error "cannot do that because blah"? I concluded that we'd better restore the former behavior, or people will complain that this is a regression. We can however do better than the "record type has not been registered" error, at least for the case where the error is being thrown at runtime. I went with ERROR: could not determine row type for result of json_populate_record HINT: Provide a non-null record argument, or call the function in the FROM clause using a column definition list. regards, tom lane