Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
I wrote: Well, I forgot that an aggregate involves more than one catalog row ;-). So it's a bit bigger patch than that, but still pretty small and safe. See attached. Applied to HEAD and 9.0. The mistaken case will now yield this: regression=# select string_agg(f1 order by f1, ',') from text_tbl; ERROR: function string_agg(text) does not exist LINE 1: select string_agg(f1 order by f1, ',') from text_tbl; ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts. It's not perfect (I don't think it's practical to get the HINT to read Put the ORDER BY at the end ;-)) but at least it should get people pointed in the right direction when they do this. 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
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On ons, 2010-08-04 at 18:19 -0400, Tom Lane wrote: This policy also implies that we are never going to allow default arguments for aggregates, or at least never have any built-in ones that use such a feature. By my count the following people had offered an opinion on making this change: for: tgl, josh, badalex, mmoncure against: rhaas, thom Anybody else want to vote, or change their vote after seeing the patch? I vote against this patch. There are plenty of other places that SQL is confusing, and this move seems excessive to me, and I find the functionality that is proposed for removal quite useful. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
Peter Eisentraut pete...@gmx.net writes: I vote against this patch. There are plenty of other places that SQL is confusing, and this move seems excessive to me, and I find the functionality that is proposed for removal quite useful. Huh? The functionality proposed for removal is only that of omitting an explicit delimiter argument for string_agg(). Since the default value (an empty string) doesn't seem to be the right thing all that often anyway, I'm not following why you think this is a significant downgrade. 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
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On Aug 5, 2010, at 11:25 AM, Tom Lane wrote: Applied to HEAD and 9.0. The mistaken case will now yield this: regression=# select string_agg(f1 order by f1, ',') from text_tbl; ERROR: function string_agg(text) does not exist LINE 1: select string_agg(f1 order by f1, ',') from text_tbl; ^ I'm confused: that looks like the two-argument form to me. Have I missed something? HINT: No function matches the given name and argument types. You might need to add explicit type casts. It's not perfect (I don't think it's practical to get the HINT to read Put the ORDER BY at the end ;-)) but at least it should get people pointed in the right direction when they do this. It confuses the shit out of me. It says string_agg(text) doesn't exist when that clearly is not the name of the function you've called. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On 5 August 2010 19:39, David E. Wheeler da...@kineticode.com wrote: On Aug 5, 2010, at 11:25 AM, Tom Lane wrote: Applied to HEAD and 9.0. The mistaken case will now yield this: regression=# select string_agg(f1 order by f1, ',') from text_tbl; ERROR: function string_agg(text) does not exist LINE 1: select string_agg(f1 order by f1, ',') from text_tbl; ^ I'm confused: that looks like the two-argument form to me. Have I missed something? HINT: No function matches the given name and argument types. You might need to add explicit type casts. It's not perfect (I don't think it's practical to get the HINT to read Put the ORDER BY at the end ;-)) but at least it should get people pointed in the right direction when they do this. It confuses the shit out of me. It says string_agg(text) doesn't exist when that clearly is not the name of the function you've called. What function name do you believe was called? -- Thom Brown Registered Linux user: #516935 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On Thu, Aug 5, 2010 at 12:25, Tom Lane t...@sss.pgh.pa.us wrote: regression=# select string_agg(f1 order by f1, ',') from text_tbl; ERROR: function string_agg(text) does not exist LINE 1: select string_agg(f1 order by f1, ',') from text_tbl; ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts. It's not perfect (I don't think it's practical to get the HINT to read Put the ORDER BY at the end ;-)) but at least it should get people pointed in the right direction when they do this. Not to mention I think most of the confusion came from using the 1 argument version first (with an order by) and then jumping straight to the 2 arg version. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
David E. Wheeler da...@kineticode.com writes: On Aug 5, 2010, at 11:25 AM, Tom Lane wrote: Applied to HEAD and 9.0. The mistaken case will now yield this: regression=# select string_agg(f1 order by f1, ',') from text_tbl; ERROR: function string_agg(text) does not exist I'm confused: that looks like the two-argument form to me. Have I missed something? Yeah, the whole point of the thread: that's not a call of a two-argument aggregate. It's a call of a one-argument aggregate, using a two-column sort key to order the aggregate input rows. It confuses the shit out of me. It says string_agg(text) doesn't exist when that clearly is not the name of the function you've called. Well, maybe we need to expend some more sweat on the error message then. But this patch was still a prerequisite thing, because without it there is no error that we can complain about. 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
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On Aug 5, 2010, at 11:42 AM, Thom Brown wrote: LINE 1: select string_agg(f1 order by f1, ',') from text_tbl; ^ I'm confused: that looks like the two-argument form to me. Have I missed something? HINT: No function matches the given name and argument types. You might need to add explicit type casts. It's not perfect (I don't think it's practical to get the HINT to read Put the ORDER BY at the end ;-)) but at least it should get people pointed in the right direction when they do this. It confuses the shit out of me. It says string_agg(text) doesn't exist when that clearly is not the name of the function you've called. What function name do you believe was called? The message says: string_agg(f1 order by f1, ',') That looks like string_agg(text, text) or string_agg(anyelement, text). Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On Aug 5, 2010, at 11:45 AM, Tom Lane wrote: I'm confused: that looks like the two-argument form to me. Have I missed something? Yeah, the whole point of the thread: that's not a call of a two-argument aggregate. It's a call of a one-argument aggregate, using a two-column sort key to order the aggregate input rows. OH!. Wow, weird. I never noticed that. It confuses the shit out of me. It says string_agg(text) doesn't exist when that clearly is not the name of the function you've called. Well, maybe we need to expend some more sweat on the error message then. But this patch was still a prerequisite thing, because without it there is no error that we can complain about. Yeah, understood. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
Well, maybe we need to expend some more sweat on the error message then. But this patch was still a prerequisite thing, because without it there is no error that we can complain about. Yes, I'd say an addition to the HINT is in order *assuming* at that stage we can tell if the user passed an ORDER BY or not. -- -- Josh Berkus PostgreSQL Experts Inc. http://www.pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
Josh Berkus j...@agliodbs.com writes: Well, maybe we need to expend some more sweat on the error message then. But this patch was still a prerequisite thing, because without it there is no error that we can complain about. Yes, I'd say an addition to the HINT is in order *assuming* at that stage we can tell if the user passed an ORDER BY or not. I was just looking at this, and realized I was mistaken earlier: the error is issued in ParseFuncOrColumn, which already is passed the agg_order list, so actually it's completely trivial to tell whether a variant error message is appropriate. I suggest that we key it off there being not just an ORDER BY, but an ORDER BY with more than one element; if there's only one then this cannot be the source of confusion. Next question: exactly how should the variant HINT be phrased? I'm inclined to drop the bit about explicit casts and make it read something like HINT: No aggregate function matches the given name and argument types. Perhaps you misplaced ORDER BY; ORDER BY must appear after all regular arguments of the aggregate. 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
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On Thu, Aug 5, 2010 at 3:16 PM, Tom Lane t...@sss.pgh.pa.us wrote: Josh Berkus j...@agliodbs.com writes: Well, maybe we need to expend some more sweat on the error message then. But this patch was still a prerequisite thing, because without it there is no error that we can complain about. Yes, I'd say an addition to the HINT is in order *assuming* at that stage we can tell if the user passed an ORDER BY or not. I was just looking at this, and realized I was mistaken earlier: the error is issued in ParseFuncOrColumn, which already is passed the agg_order list, so actually it's completely trivial to tell whether a variant error message is appropriate. I suggest that we key it off there being not just an ORDER BY, but an ORDER BY with more than one element; if there's only one then this cannot be the source of confusion. Next question: exactly how should the variant HINT be phrased? I'm inclined to drop the bit about explicit casts and make it read something like HINT: No aggregate function matches the given name and argument types. Perhaps you misplaced ORDER BY; ORDER BY must appear after all regular arguments of the aggregate. Could we arrange to emit this error message only when there is an aggregate with the same name but different arguments? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On Aug 5, 2010, at 12:16 PM, Tom Lane wrote: HINT: No aggregate function matches the given name and argument types. Perhaps you misplaced ORDER BY; ORDER BY must appear after all regular arguments of the aggregate. +1 David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
Robert Haas robertmh...@gmail.com writes: On Thu, Aug 5, 2010 at 3:16 PM, Tom Lane t...@sss.pgh.pa.us wrote: Next question: exactly how should the variant HINT be phrased? I'm inclined to drop the bit about explicit casts and make it read something like HINT: No aggregate function matches the given name and argument types. Perhaps you misplaced ORDER BY; ORDER BY must appear after all regular arguments of the aggregate. Could we arrange to emit this error message only when there is an aggregate with the same name but different arguments? That'd move it into the category of needing significant restructuring, I'm afraid. At the moment it's not apparent that it's worth it. We're already able to limit the use of the variant hint to a pretty darn narrow set of cases, and it is only a hint after all. 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
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On 8/5/10 12:18 PM, Robert Haas wrote: Could we arrange to emit this error message only when there is an aggregate with the same name but different arguments? Personally, I don't see this as really necessary. Just mentioning ORDER BY in the hint will be enough to give people the right place to look. -- -- Josh Berkus PostgreSQL Experts Inc. http://www.pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
Josh Berkus j...@agliodbs.com writes: On 8/5/10 12:18 PM, Robert Haas wrote: Could we arrange to emit this error message only when there is an aggregate with the same name but different arguments? Personally, I don't see this as really necessary. Just mentioning ORDER BY in the hint will be enough to give people the right place to look. I suppose Robert is more concerned about the possibility that we emit the ORDER BY hint when that isn't really the source of the problem. But after all, the reason it's a hint and not the primary error message is that it's not certain to be helpful. 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
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On tor, 2010-08-05 at 14:38 -0400, Tom Lane wrote: Huh? The functionality proposed for removal is only that of omitting an explicit delimiter argument for string_agg(). Since the default value (an empty string) doesn't seem to be the right thing all that often anyway, I'm not following why you think this is a significant downgrade. I just think it's useful to have the one-argument version. I understand the functionality is available in other ways. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
Peter Eisentraut pete...@gmx.net writes: On tor, 2010-08-05 at 14:38 -0400, Tom Lane wrote: Huh? The functionality proposed for removal is only that of omitting an explicit delimiter argument for string_agg(). Since the default value (an empty string) doesn't seem to be the right thing all that often anyway, I'm not following why you think this is a significant downgrade. I just think it's useful to have the one-argument version. I understand the functionality is available in other ways. Well, other things being equal I'd have preferred to keep the one-argument version too. But this thread has made it even clearer than before that we will get continuing bug reports if we leave the behavior alone. I don't think the ability to leave off the delimiter value is worth the amount of confusion it'll cause. 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
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
Alex Hunsaker bada...@gmail.com writes: Either way, I don't have strong feelings on this other than if we dont fix it now when will we? Well, we won't. If 9.0 ships with both forms of string_agg, we're stuck with it IMO. It's not exactly a bug, so I won't cry if that's how things go; but it is striking that already two different people have gotten confused enough to file bug reports because of this. If we don't pull the one-argument form then I think we can look forward to many more of those in future years. 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
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
Robert Haas robertmh...@gmail.com writes: On Wed, Aug 4, 2010 at 3:25 PM, Alex Hunsaker bada...@gmail.com wrote: I think forcing an initdb might be more trouble than this wart is worth. +1. I would not make this change unless we have to force an initdb anyway. And I really hope we don't, because I'm sort of hoping the next 9.0 release will be rc1. Hm? I don't think that an initdb here would have any impact on whether we can call the next drop RC1 or not. We're talking about removing a single built-in entry in pg_proc --- it's one of the safest changes we could possibly make. The only argument I can see against it is not wanting to force beta testers through an initdb. But it seems like that might actually be a positive thing not a negative one, in this cycle, because we're trying to get test coverage on pg_upgrade. 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
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
Well, it'd take an initdb to get rid of it. In the past we've avoided forcing initdb post-beta1 unless it was Really Necessary. OTOH, we seem to be in the mode of encouraging beta testers to test pg_upgrade, so maybe that concern isn't worth much at the moment. If it's causing bugs, drop it now. If we include it in 9.0, we're stuck with it for years. I'm OK with forcing an initDB for RC1. -- -- Josh Berkus PostgreSQL Experts Inc. http://www.pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
Josh Berkus j...@agliodbs.com writes: If it's causing bugs, drop it now. If we include it in 9.0, we're stuck with it for years. Well, it's causing bug reports, which is not exactly the same thing as bugs. But yeah, I'm thinking we should get rid of it. 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
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
04.Ağu.2010 tarihinde 22:44 saatinde, Josh Berkus j...@agliodbs.com şunları yazdı: I'm OK with forcing an initDB for RC1. I think beta5 will be a better choice than RC 1 here. -- Devrim GÜNDÜZ PostgreSQL DBA @ Akinon/Markafoni, Red Hat Certified Engineer devrim~gunduz.org, devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org Twitter: http://twitter.com/devrimgunduz -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
I wrote: Hm? I don't think that an initdb here would have any impact on whether we can call the next drop RC1 or not. We're talking about removing a single built-in entry in pg_proc --- it's one of the safest changes we could possibly make. Well, I forgot that an aggregate involves more than one catalog row ;-). So it's a bit bigger patch than that, but still pretty small and safe. See attached. What we are doing here, IMO, is not just changing string_agg() but instituting a project policy that we are not going to offer built-in aggregates with the same names and different numbers of arguments --- otherwise the problem will come right back. Therefore, the attached patch adds an opr_sanity regression test that will complain if anyone tries to add such. Of course, this isn't preventing users from creating such aggregates, but it's on their own heads to not get confused if they do. This policy also implies that we are never going to allow default arguments for aggregates, or at least never have any built-in ones that use such a feature. By my count the following people had offered an opinion on making this change: for: tgl, josh, badalex, mmoncure against: rhaas, thom Anybody else want to vote, or change their vote after seeing the patch? regards, tom lane Index: doc/src/sgml/func.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/func.sgml,v retrieving revision 1.522 diff -c -r1.522 func.sgml *** doc/src/sgml/func.sgml 29 Jul 2010 19:34:40 - 1.522 --- doc/src/sgml/func.sgml 4 Aug 2010 22:01:12 - *** *** 9731,9737 thead row entryFunction/entry ! entryArgument Type/entry entryReturn Type/entry entryDescription/entry /row --- 9731,9737 thead row entryFunction/entry ! entryArgument Type(s)/entry entryReturn Type/entry entryDescription/entry /row *** *** 9901,9917 primarystring_agg/primary /indexterm function ! string_agg(replaceable class=parameterexpression/replaceable ! [, replaceable class=parameterdelimiter/replaceable ] ) /function /entry entry !typetext/type /entry entry typetext/type /entry ! entryinput values concatenated into a string, optionally with delimiters/entry /row row --- 9901,9917 primarystring_agg/primary /indexterm function ! string_agg(replaceable class=parameterexpression/replaceable, ! replaceable class=parameterdelimiter/replaceable) /function /entry entry !typetext/type, typetext/type /entry entry typetext/type /entry ! entryinput values concatenated into a string, separated by delimiter/entry /row row Index: doc/src/sgml/syntax.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/syntax.sgml,v retrieving revision 1.149 diff -c -r1.149 syntax.sgml *** doc/src/sgml/syntax.sgml 4 Aug 2010 15:27:57 - 1.149 --- doc/src/sgml/syntax.sgml 4 Aug 2010 22:01:12 - *** *** 1589,1598 /programlisting not this: programlisting ! SELECT string_agg(a ORDER BY a, ',') FROM table; -- not what you want /programlisting ! The latter syntax will be accepted, but literal','/ will be ! treated as a (useless) sort key. /para para --- 1589,1599 /programlisting not this: programlisting ! SELECT string_agg(a ORDER BY a, ',') FROM table; -- incorrect /programlisting ! The latter is syntactically valid, but it represents a call of a ! single-argument aggregate function with two literalORDER BY/ keys ! (the second one being rather useless since it's a constant). /para para Index: src/backend/utils/adt/varlena.c === RCS file: /cvsroot/pgsql/src/backend/utils/adt/varlena.c,v retrieving revision 1.177 diff -c -r1.177 varlena.c *** src/backend/utils/adt/varlena.c 26 Feb 2010 02:01:10 - 1.177 --- src/backend/utils/adt/varlena.c 4 Aug 2010 22:01:12 - *** *** 3320,3326 /* * string_agg - Concatenates values and returns string. * ! * Syntax: string_agg(value text, delimiter text = '') RETURNS text * * Note: Any NULL values are ignored. The first-call delimiter isn't * actually used at all, and on subsequent calls the delimiter precedes --- 3320,3326 /* * string_agg - Concatenates values and returns string. * ! * Syntax: string_agg(value text, delimiter text) RETURNS text * * Note: Any NULL values are ignored. The first-call delimiter isn't * actually used at all, and on subsequent calls the
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On 4 August 2010 23:19, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: Hm? I don't think that an initdb here would have any impact on whether we can call the next drop RC1 or not. We're talking about removing a single built-in entry in pg_proc --- it's one of the safest changes we could possibly make. Well, I forgot that an aggregate involves more than one catalog row ;-). So it's a bit bigger patch than that, but still pretty small and safe. See attached. What we are doing here, IMO, is not just changing string_agg() but instituting a project policy that we are not going to offer built-in aggregates with the same names and different numbers of arguments --- otherwise the problem will come right back. Therefore, the attached patch adds an opr_sanity regression test that will complain if anyone tries to add such. Of course, this isn't preventing users from creating such aggregates, but it's on their own heads to not get confused if they do. Yes, I think that's sensible. This policy also implies that we are never going to allow default arguments for aggregates, or at least never have any built-in ones that use such a feature. By my count the following people had offered an opinion on making this change: for: tgl, josh, badalex, mmoncure against: rhaas, thom Anybody else want to vote, or change their vote after seeing the patch? regards, tom lane I was afraid that the function would be pulled completely, but from looking at the patch, you're only removing the function with a single-parameter signature, which is quite innocuous. So I'm for now. -- Thom Brown Registered Linux user: #516935 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
Thom Brown t...@linux.com writes: I was afraid that the function would be pulled completely, but from looking at the patch, you're only removing the function with a single-parameter signature, which is quite innocuous. Yes, of course, sorry if I confused anyone. It's the combination of having both one- and two-argument forms that is the problem. Since the one-argument form isn't doing anything but offering a rather useless default, we won't lose functionality if we pull it. 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
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On Wed, Aug 04, 2010 at 06:19:49PM -0400, Tom Lane wrote: I wrote: Hm? I don't think that an initdb here would have any impact on whether we can call the next drop RC1 or not. We're talking about removing a single built-in entry in pg_proc --- it's one of the safest changes we could possibly make. Well, I forgot that an aggregate involves more than one catalog row ;-). So it's a bit bigger patch than that, but still pretty small and safe. See attached. What we are doing here, IMO, is not just changing string_agg() but instituting a project policy that we are not going to offer built-in aggregates with the same names and different numbers of arguments --- otherwise the problem will come right back. Therefore, the attached patch adds an opr_sanity regression test that will complain if anyone tries to add such. Of course, this isn't preventing users from creating such aggregates, but it's on their own heads to not get confused if they do. This policy also implies that we are never going to allow default arguments for aggregates, or at least never have any built-in ones that use such a feature. By my count the following people had offered an opinion on making this change: for: tgl, josh, badalex, mmoncure against: rhaas, thom Anybody else want to vote, or change their vote after seeing the patch? +1 for removing the single-argument version. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On Wed, Aug 4, 2010 at 16:33, Thom Brown t...@linux.com wrote: I was afraid that the function would be pulled completely, but from looking at the patch, you're only removing the function with a single-parameter signature, which is quite innocuous. So I'm for now. Ahh, Now I see why you were worried about people calling you a witch :) On another note, I do wonder if we could avoid more confusion by just reordering the arguments. In-fact I bet the original argument ordering was done precisely so it would match the 1 argument version. I dunno about anyone else but (a, ',' order by a) just looks weird. Or in other words, any thoughts on: select string_agg(delim, expression); I don't want to stretch this out, but while we are making changes... -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
Alex Hunsaker bada...@gmail.com writes: I dunno about anyone else but (a, ',' order by a) just looks weird. I suppose, but aren't you just focusing on the argument being constant? In the more general case I don't think there's anything unnatural about this syntax. Or in other words, any thoughts on: select string_agg(delim, expression); That looks pretty weird to me anyway, with or without use of ORDER BY. Nobody would think to write the delimiter first. Usually you put the most important argument first, and no one would see the delimiter as the most important one. 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
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On Wed, Aug 4, 2010 at 7:07 PM, Tom Lane t...@sss.pgh.pa.us wrote: Or in other words, any thoughts on: select string_agg(delim, expression); That looks pretty weird to me anyway, with or without use of ORDER BY. Nobody would think to write the delimiter first. Usually you put the most important argument first, and no one would see the delimiter as the most important one. Well, it would match the syntax of things like perl's join(). But I think that's probably not enough reason to go fiddling with it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On Wed, Aug 4, 2010 at 17:07, Tom Lane t...@sss.pgh.pa.us wrote:
Alex Hunsaker bada...@gmail.com writes:
I dunno about anyone else but (a, ',' order by a) just looks weird.
I suppose, but aren't you just focusing on the argument being constant?
Yes.
Or in other words, any thoughts on:
select string_agg(delim, expression);
That looks pretty weird to me anyway, with or without use of ORDER BY.
Nobody would think to write the delimiter first. Usually you put the
most important argument first, and no one would see the delimiter
as the most important one.
No argument about the most important arg first. I think we have a
difference of opinion on what the important argument is :-)
It might just be my perl upbringing talking, for example you join(',',
...) or split(',', ) in perl.
Either way *shrug* Im happy to leave it alone.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Drop one-argument string_agg? (was Re: [BUGS] string_agg delimiter having no effect with order by)
On Wed, Aug 4, 2010 at 6:19 PM, Tom Lane t...@sss.pgh.pa.us wrote: for: tgl, josh, badalex, mmoncure against: rhaas, thom Anybody else want to vote, or change their vote after seeing the patch? If we're not regarding this as beta-forcing, I abstain. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
