Обсуждение: [HACKERS] Re: [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC callsvery much
[HACKERS] Re: [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC callsvery much
От
Andrew Dunstan
Дата:
On 10/22/2017 12:11 PM, Andrew Dunstan wrote: > > On 10/21/2017 07:33 PM, Michael Paquier wrote: >> On Sun, Oct 22, 2017 at 1:43 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> I don't think collecting all the arguments is particularly special --- >>> format() or concat() for instance could possibly use this. You might >>> need an option to say what to do with unknown. >> In this case, we could just use a boolean flag to decide if TEXTOID >> should be enforced or not. > A generic function is going to look a little more complicated than this, > though. The functions as coded assume that the function has a single > variadic argument. But for it to be useful generically it really needs > to be able to work where there are both fixed and variadic arguments (a > la printf style functions). > > I guess a simple way would be to make the caller tell the function where > the variadic arguments start, or how many to skip, something like that. > here's a patch that works that way, based on Michael's code. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC callsvery much
От
Michael Paquier
Дата:
On Mon, Oct 23, 2017 at 1:44 AM, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote: > > > On 10/22/2017 12:11 PM, Andrew Dunstan wrote: >> >> On 10/21/2017 07:33 PM, Michael Paquier wrote: >>> On Sun, Oct 22, 2017 at 1:43 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>>> I don't think collecting all the arguments is particularly special --- >>>> format() or concat() for instance could possibly use this. You might >>>> need an option to say what to do with unknown. >>> In this case, we could just use a boolean flag to decide if TEXTOID >>> should be enforced or not. >> A generic function is going to look a little more complicated than this, >> though. The functions as coded assume that the function has a single >> variadic argument. But for it to be useful generically it really needs >> to be able to work where there are both fixed and variadic arguments (a >> la printf style functions). >> >> I guess a simple way would be to make the caller tell the function where >> the variadic arguments start, or how many to skip, something like that. Sorry for the late reply, I was taking a long flight. Yes, that's actually the conclusion I am reaching when looking at the code by adding an argument to mark when the variadic arguments start. The headers of funcapi.h and funcapi.c need also a cleanup. > here's a patch that works that way, based on Michael's code. Patch not attached :) I still have a patch half-cooked, that I can send if necessary, but you are on it, right? -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC callsvery much
От
Andrew Dunstan
Дата:
On 10/22/2017 04:35 PM, Michael Paquier wrote: > On Mon, Oct 23, 2017 at 1:44 AM, Andrew Dunstan > <andrew.dunstan@2ndquadrant.com> wrote: > >> here's a patch that works that way, based on Michael's code. > Patch not attached :) > I still have a patch half-cooked, that I can send if necessary, but > you are on it, right? Sorry. Here it is. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Вложения
Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes: > Sorry. Here it is. This comment is neither correct nor intelligible: /* important for value, key cannot being NULL */ I'd say just drop it. The checks for "could not determine data type" errors seem rather duplicative, too. <compulsive-neatnik-ism> The extern declaration seems to have been dropped in a rather random place in the .h file. </compulsive-neatnik-ism> Looks good otherwise. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC callsvery much
От
Michael Paquier
Дата:
On Mon, Oct 23, 2017 at 6:11 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > This comment is neither correct nor intelligible: > > /* important for value, key cannot being NULL */ > > I'd say just drop it. Yep. > The checks for "could not determine data type" errors seem > rather duplicative, too. Yep. > <compulsive-neatnik-ism> > The extern declaration seems to have been dropped in a rather > random place in the .h file. > </compulsive-neatnik-ism> funcapi.h/c are nicely documented. I think that more is needed. > Looks good otherwise. My set of diffs for funcapi.h are actually that: * funcapi.h * Definitions for functions which return composite type and/orsets + * or work on VARIADIC inputs. [...] +/*---------- + * Support to ease writing of functions dealing with VARIADIC inputs + *---------- + * + * This function extracts a set of argument values, types and NULL markers + * for a given input function. This returns a set of data: + * - **values includes the set of Datum values extracted. + * - **types the data type OID for each element. + * - **nulls tracks if an element is NULL. + * + * convert_unknown set to true enforces conversion of unknown input type + * unknown to text. + * variadic_start tracks the argument number of the function call where the + * VARIADIC set of arguments begins. + * + * The return result is the number of elements stored. In the event of a + * NULL input, then the caller of this function ought to generate a NULL + * object as final result, and in this case a result value of -1 is used + * to be able to make the difference between an empty array or object. + */ +extern int extract_variadic_args(FunctionCallInfo fcinfo, int variadic_start, + bool convert_unknown, Datum **values, Oid **types, + bool **nulls); Got this bit as well: * funcapi.c * Utility and convenience functions for fmgr functions that return - * sets and/or composite types. + * sets and/or composite types, or deal with VARIADIC inputs. * -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC callsvery much
От
Michael Paquier
Дата:
On Mon, Oct 23, 2017 at 7:03 AM, Michael Paquier <michael.paquier@gmail.com> wrote: >> Looks good otherwise. > > My set of diffs for funcapi.h are actually that: > * funcapi.h > * Definitions for functions which return composite type and/or sets > + * or work on VARIADIC inputs. > [...] > +/*---------- > + * Support to ease writing of functions dealing with VARIADIC inputs > + *---------- > + * > + * This function extracts a set of argument values, types and NULL markers > + * for a given input function. This returns a set of data: > + * - **values includes the set of Datum values extracted. > + * - **types the data type OID for each element. > + * - **nulls tracks if an element is NULL. > + * > + * convert_unknown set to true enforces conversion of unknown input type > + * unknown to text. > + * variadic_start tracks the argument number of the function call where the > + * VARIADIC set of arguments begins. > + * > + * The return result is the number of elements stored. In the event of a > + * NULL input, then the caller of this function ought to generate a NULL > + * object as final result, and in this case a result value of -1 is used > + * to be able to make the difference between an empty array or object. > + */ > +extern int extract_variadic_args(FunctionCallInfo fcinfo, int variadic_start, > + bool convert_unknown, Datum **values, > Oid **types, > + bool **nulls); > > Got this bit as well: > * funcapi.c > * Utility and convenience functions for fmgr functions that return > - * sets and/or composite types. > + * sets and/or composite types, or deal with VARIADIC inputs. > * Okay, attached is what I think a fully implemented patch should look like. On top of what Andrew has done, I added and reworked the following: - removed duplicate error handling. - documented the function in funcapi.h and funcapi.c. - Added a new section in funcapi.h to outline that this is for support of VARIADIC inputs. I have added a commit message as well. Hope this helps. format, concat and potentially count_nulls could take advantage of this new function, though refactoring is left for later. I am fine to do the legwork on a different thread. Changing count_nulls would also switch it to a o(n^2), which is not cool either, so I think that it could be left out. Still let's discuss that on another thread. -- Michael -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Вложения
[HACKERS] Re: [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC callsvery much
От
Michael Paquier
Дата:
On Mon, Oct 23, 2017 at 6:50 AM, Michael Paquier <michael.paquier@gmail.com> wrote: > Okay, attached is what I think a fully implemented patch should look > like. On top of what Andrew has done, I added and reworked the > following: > - removed duplicate error handling. > - documented the function in funcapi.h and funcapi.c. > - Added a new section in funcapi.h to outline that this is for support > of VARIADIC inputs. > I have added a commit message as well. Hope this helps. For the sake of the archives, the introduction of extract_variadic_args is committed with f3c6e8a2, and the JSON fixes with 18fc4ecf. Thanks Andrew for the commit, and thanks Tom, Andrew and Dmitry for the reviews. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC callsvery much
От
Marko Tiikkaja
Дата:
On Wed, Oct 25, 2017 at 5:32 PM, Michael Paquier <michael.paquier@gmail.com> wrote:
On Mon, Oct 23, 2017 at 6:50 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> Okay, attached is what I think a fully implemented patch should look
> like. On top of what Andrew has done, I added and reworked the
> following:
> - removed duplicate error handling.
> - documented the function in funcapi.h and funcapi.c.
> - Added a new section in funcapi.h to outline that this is for support
> of VARIADIC inputs.
> I have added a commit message as well. Hope this helps.
For the sake of the archives, the introduction of
extract_variadic_args is committed with f3c6e8a2, and the JSON fixes
with 18fc4ecf. Thanks Andrew for the commit, and thanks Tom, Andrew
and Dmitry for the reviews.
Thx yo.
.m