Re: [HACKERS] patch: bytea_agg
Peter Eisentraut writes: > On lör, 2012-04-07 at 10:38 -0400, Tom Lane wrote: >> Hm. So are you now suggesting we should get rid of one-argument >> bytea_agg and replace it with two-argument string_agg(bytea,bytea)? >> I could support that, since we've not released bytea_agg yet. > Yes, that looks like the best solution. Here is a patch for that. Looks sane in a quick once-over, except for the documentation entry. I'm not really thrilled with "text, text or bytea, bytea" because it seems easy to misparse. Moreover, as written the entry claims that the return type is text either way, which is wrong. You could fix the latter by writing "same as argument data type", but I wonder whether it'd be better to make separate table entries for the two forms. 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] patch: bytea_agg
On lör, 2012-04-07 at 10:38 -0400, Tom Lane wrote:
> > Nevertheless, the problem would now be that adding string_agg(bytea)
> > would effectively forbid adding string_agg(bytea, delim) in the
> future.
> > So making a two-argument string_agg(bytea, bytea) now seems like the
> > best solution anyway. (This applies independently of the function
> > renaming, actually.)
>
> Hm. So are you now suggesting we should get rid of one-argument
> bytea_agg and replace it with two-argument string_agg(bytea,bytea)?
> I could support that, since we've not released bytea_agg yet.
Yes, that looks like the best solution. Here is a patch for that.
diff --git i/doc/src/sgml/func.sgml w/doc/src/sgml/func.sgml
index ae22ee5..131384c 100644
--- i/doc/src/sgml/func.sgml
+++ w/doc/src/sgml/func.sgml
@@ -10965,24 +10965,6 @@ SELECT NULLIF(value, '(none)') ...
-bytea_agg
-
-
- bytea_agg(expression)
-
-
-
- bytea
-
-
- bytea
-
- input values concatenated into a bytea
-
-
-
-
-
count
count(*)
@@ -11061,7 +11043,7 @@ SELECT NULLIF(value, '(none)') ...
- text, text
+ text, text or bytea, bytea
text
diff --git i/src/backend/utils/adt/varlena.c w/src/backend/utils/adt/varlena.c
index 65e9af8..a5592d5 100644
--- i/src/backend/utils/adt/varlena.c
+++ w/src/backend/utils/adt/varlena.c
@@ -397,7 +397,7 @@ byteasend(PG_FUNCTION_ARGS)
}
Datum
-bytea_agg_transfn(PG_FUNCTION_ARGS)
+bytea_string_agg_transfn(PG_FUNCTION_ARGS)
{
StringInfo state;
@@ -408,21 +408,28 @@ bytea_agg_transfn(PG_FUNCTION_ARGS)
{
bytea *value = PG_GETARG_BYTEA_PP(1);
+ /* On the first time through, we ignore the delimiter. */
if (state == NULL)
state = makeStringAggState(fcinfo);
+ else if (!PG_ARGISNULL(2))
+ {
+ bytea *delim = PG_GETARG_BYTEA_PP(2);
+
+ appendBinaryStringInfo(state, VARDATA_ANY(delim), VARSIZE_ANY_EXHDR(delim));
+ }
appendBinaryStringInfo(state, VARDATA_ANY(value), VARSIZE_ANY_EXHDR(value));
}
/*
- * The transition type for bytea_agg() is declared to be "internal",
+ * The transition type for string_agg() is declared to be "internal",
* which is a pass-by-value type the same size as a pointer.
*/
PG_RETURN_POINTER(state);
}
Datum
-bytea_agg_finalfn(PG_FUNCTION_ARGS)
+bytea_string_agg_finalfn(PG_FUNCTION_ARGS)
{
StringInfo state;
diff --git i/src/include/catalog/pg_aggregate.h w/src/include/catalog/pg_aggregate.h
index adda07c..461772c 100644
--- i/src/include/catalog/pg_aggregate.h
+++ w/src/include/catalog/pg_aggregate.h
@@ -229,7 +229,7 @@ DATA(insert ( 2335 array_agg_transfn array_agg_finalfn 0 2281 _null_ ));
DATA(insert ( 3538 string_agg_transfn string_agg_finalfn 0 2281 _null_ ));
/* bytea */
-DATA(insert ( 3545 bytea_agg_transfn bytea_agg_finalfn 0 2281 _null_ ));
+DATA(insert ( 3545 bytea_string_agg_transfn bytea_string_agg_finalfn 0 2281 _null_ ));
/*
* prototypes for functions in pg_aggregate.c
diff --git i/src/include/catalog/pg_proc.h w/src/include/catalog/pg_proc.h
index 6414b33..aa4d350 100644
--- i/src/include/catalog/pg_proc.h
+++ w/src/include/catalog/pg_proc.h
@@ -2433,11 +2433,11 @@ DATA(insert OID = 3536 ( string_agg_finalfn PGNSP PGUID 12 1 0 0 0 f f f f f f
DESCR("aggregate final function");
DATA(insert OID = 3538 ( string_aggPGNSP PGUID 12 1 0 0 0 t f f f f f i 2 0 25 "25 25" _null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ ));
DESCR("concatenate aggregate input into a string");
-DATA(insert OID = 3543 ( bytea_agg_transfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2281 "2281 17" _null_ _null_ _null_ _null_ bytea_agg_transfn _null_ _null_ _null_ ));
+DATA(insert OID = 3543 ( bytea_string_agg_transfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 3 0 2281 "2281 17 17" _null_ _null_ _null_ _null_ bytea_string_agg_transfn _null_ _null_ _null_ ));
DESCR("aggregate transition function");
-DATA(insert OID = 3544 ( bytea_agg_finalfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 1 0 17 "2281" _null_ _null_ _null_ _null_ bytea_agg_finalfn _null_ _null_ _null_ ));
+DATA(insert OID = 3544 ( bytea_string_agg_finalfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 1 0 17 "2281" _null_ _null_ _null_ _null_ bytea_string_agg_finalfn _null_ _null_ _null_ ));
DESCR("aggregate final function");
-DATA(insert OID = 3545 ( bytea_aggPGNSP PGUID 12 1 0 0 0 t f f f f f i 1 0 17 "17" _null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ ));
+DATA(insert OID = 3545 ( string_aggPGNSP PGUID 12 1 0 0 0 t f f f f f i 2 0 17 "17 17" _null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ ));
DESCR("concatenate aggregate input into a bytea");
/* To ASCII conversion */
diff --git i/src/include/utils/builtins.h w/src/include/utils/builtins.h
index 9fda7ad..201b23e 100644
--- i/src/include/utils/builtins.h
+++ w/src/include/utils/built
Re: [HACKERS] patch: bytea_agg
Peter Eisentraut writes: > On ons, 2012-04-04 at 18:59 -0400, Tom Lane wrote: >> Uh, no. That test is there for good and sufficient reasons, as per its >> comment: > I had reviewed that thread very carefully, but I'm not sure it applies. > The issue was that we don't want aggregates with optional second > argument, because "novices" could confuse > agg(a, b ORDER BY c) > with > agg(a ORDER BY b, c) -- wrong > without the error being flagged. > But that doesn't apply if the first argument has different types. > Erroneously calling agg(textdatum ORDER BY textdatum, sthelse) will not > result in a call to agg(bytea). The point of the policy is to be sure that we will throw a specific error message with a suitable hint when a malformed call is made. If there are alternative aggregates in the catalogs that have the potential to "capture" such a call, then we risk the wrong thing happening. It appears that the lack of any implicit cast from bytea to text would prevent that today, but I still think this proposal boxes us in to an unreasonable degree compared to the benefit. For example, this would greatly restrict our ability to throw "ambiguous aggregate" messages. > Nevertheless, the problem would now be that adding string_agg(bytea) > would effectively forbid adding string_agg(bytea, delim) in the future. > So making a two-argument string_agg(bytea, bytea) now seems like the > best solution anyway. (This applies independently of the function > renaming, actually.) Hm. So are you now suggesting we should get rid of one-argument bytea_agg and replace it with two-argument string_agg(bytea,bytea)? I could support that, since we've not released bytea_agg yet. 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] patch: bytea_agg
On ons, 2012-04-04 at 18:59 -0400, Tom Lane wrote: > > >> Why not call it string_agg? > > > Here is a patch to do the renaming. As it stands, it fails the > > opr_sanity regression test, because that complains that there are now > > two aggregate functions string_agg with different number of arguments. > > It seems to me that that test should really only complain if the common > > argument types of the two aggregates are the same, correct? > > Uh, no. That test is there for good and sufficient reasons, as per its > comment: > > -- Check that there are not aggregates with the same name and different > -- numbers of arguments. While not technically wrong, we have a project > policy > -- to avoid this because it opens the door for confusion in connection with > -- ORDER BY: novices frequently put the ORDER BY in the wrong place. > -- See the fate of the single-argument form of string_agg() for history. > > The renaming you propose would only be acceptable to those who have > forgotten that history. I haven't. I had reviewed that thread very carefully, but I'm not sure it applies. The issue was that we don't want aggregates with optional second argument, because "novices" could confuse agg(a, b ORDER BY c) with agg(a ORDER BY b, c) -- wrong without the error being flagged. But that doesn't apply if the first argument has different types. Erroneously calling agg(textdatum ORDER BY textdatum, sthelse) will not result in a call to agg(bytea). (Unless the textdatum is really an unknown literal, but that would be silly.) Nevertheless, the problem would now be that adding string_agg(bytea) would effectively forbid adding string_agg(bytea, delim) in the future. So making a two-argument string_agg(bytea, bytea) now seems like the best solution anyway. (This applies independently of the function renaming, actually.) -- 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] patch: bytea_agg
On Wed, Apr 4, 2012 at 11:59 PM, Tom Lane wrote: > The renaming you propose would only be acceptable to those who have > forgotten that history. I haven't. I had. I looked it up http://archives.postgresql.org/pgsql-bugs/2010-08/msg00044.php That was quite a thread. -- greg -- 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] patch: bytea_agg
Peter Eisentraut writes: > On fre, 2011-12-23 at 19:51 +0200, Peter Eisentraut wrote: >> On ons, 2011-12-21 at 11:04 +0100, Pavel Stehule wrote: >>> this patch adds a bytea_agg aggregation. >> Why not call it string_agg? > Here is a patch to do the renaming. As it stands, it fails the > opr_sanity regression test, because that complains that there are now > two aggregate functions string_agg with different number of arguments. > It seems to me that that test should really only complain if the common > argument types of the two aggregates are the same, correct? Uh, no. That test is there for good and sufficient reasons, as per its comment: -- Check that there are not aggregates with the same name and different -- numbers of arguments. While not technically wrong, we have a project policy -- to avoid this because it opens the door for confusion in connection with -- ORDER BY: novices frequently put the ORDER BY in the wrong place. -- See the fate of the single-argument form of string_agg() for history. The renaming you propose would only be acceptable to those who have forgotten that history. I haven't. 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] patch: bytea_agg
On fre, 2011-12-23 at 19:51 +0200, Peter Eisentraut wrote:
> On ons, 2011-12-21 at 11:04 +0100, Pavel Stehule wrote:
> > this patch adds a bytea_agg aggregation.
> >
> > It allow fast bytea concatetation.
>
> Why not call it string_agg? All the function names are the same between
> text and bytea (e.g., ||, substr, position, length). It would be nice
> not to introduce arbitrary differences.
Here is a patch to do the renaming. As it stands, it fails the
opr_sanity regression test, because that complains that there are now
two aggregate functions string_agg with different number of arguments.
It seems to me that that test should really only complain if the common
argument types of the two aggregates are the same, correct?
diff --git i/doc/src/sgml/func.sgml w/doc/src/sgml/func.sgml
index 34fea16..393cfcd 100644
--- i/doc/src/sgml/func.sgml
+++ w/doc/src/sgml/func.sgml
@@ -10963,10 +10963,10 @@ SELECT NULLIF(value, '(none)') ...
-bytea_agg
+string_agg
- bytea_agg(expression)
+ string_agg(expression)
diff --git i/src/backend/utils/adt/varlena.c w/src/backend/utils/adt/varlena.c
index 65e9af8..c6b351e 100644
--- i/src/backend/utils/adt/varlena.c
+++ w/src/backend/utils/adt/varlena.c
@@ -397,7 +397,7 @@ byteasend(PG_FUNCTION_ARGS)
}
Datum
-bytea_agg_transfn(PG_FUNCTION_ARGS)
+bytea_string_agg_transfn(PG_FUNCTION_ARGS)
{
StringInfo state;
@@ -415,14 +415,14 @@ bytea_agg_transfn(PG_FUNCTION_ARGS)
}
/*
- * The transition type for bytea_agg() is declared to be "internal",
+ * The transition type for string_agg() is declared to be "internal",
* which is a pass-by-value type the same size as a pointer.
*/
PG_RETURN_POINTER(state);
}
Datum
-bytea_agg_finalfn(PG_FUNCTION_ARGS)
+bytea_string_agg_finalfn(PG_FUNCTION_ARGS)
{
StringInfo state;
diff --git i/src/include/catalog/pg_aggregate.h w/src/include/catalog/pg_aggregate.h
index adda07c..461772c 100644
--- i/src/include/catalog/pg_aggregate.h
+++ w/src/include/catalog/pg_aggregate.h
@@ -229,7 +229,7 @@ DATA(insert ( 2335 array_agg_transfn array_agg_finalfn 0 2281 _null_ ));
DATA(insert ( 3538 string_agg_transfn string_agg_finalfn 0 2281 _null_ ));
/* bytea */
-DATA(insert ( 3545 bytea_agg_transfn bytea_agg_finalfn 0 2281 _null_ ));
+DATA(insert ( 3545 bytea_string_agg_transfn bytea_string_agg_finalfn 0 2281 _null_ ));
/*
* prototypes for functions in pg_aggregate.c
diff --git i/src/include/catalog/pg_proc.h w/src/include/catalog/pg_proc.h
index 49b0754..e1962fe 100644
--- i/src/include/catalog/pg_proc.h
+++ w/src/include/catalog/pg_proc.h
@@ -2433,11 +2433,11 @@ DATA(insert OID = 3536 ( string_agg_finalfn PGNSP PGUID 12 1 0 0 0 f f f f f f
DESCR("aggregate final function");
DATA(insert OID = 3538 ( string_aggPGNSP PGUID 12 1 0 0 0 t f f f f f i 2 0 25 "25 25" _null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ ));
DESCR("concatenate aggregate input into a string");
-DATA(insert OID = 3543 ( bytea_agg_transfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2281 "2281 17" _null_ _null_ _null_ _null_ bytea_agg_transfn _null_ _null_ _null_ ));
+DATA(insert OID = 3543 ( bytea_string_agg_transfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 2 0 2281 "2281 17" _null_ _null_ _null_ _null_ bytea_string_agg_transfn _null_ _null_ _null_ ));
DESCR("aggregate transition function");
-DATA(insert OID = 3544 ( bytea_agg_finalfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 1 0 17 "2281" _null_ _null_ _null_ _null_ bytea_agg_finalfn _null_ _null_ _null_ ));
+DATA(insert OID = 3544 ( bytea_string_agg_finalfn PGNSP PGUID 12 1 0 0 0 f f f f f f i 1 0 17 "2281" _null_ _null_ _null_ _null_ bytea_string_agg_finalfn _null_ _null_ _null_ ));
DESCR("aggregate final function");
-DATA(insert OID = 3545 ( bytea_aggPGNSP PGUID 12 1 0 0 0 t f f f f f i 1 0 17 "17" _null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ ));
+DATA(insert OID = 3545 ( string_aggPGNSP PGUID 12 1 0 0 0 t f f f f f i 1 0 17 "17" _null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ ));
DESCR("concatenate aggregate input into a bytea");
/* To ASCII conversion */
diff --git i/src/include/utils/builtins.h w/src/include/utils/builtins.h
index 9fda7ad..201b23e 100644
--- i/src/include/utils/builtins.h
+++ w/src/include/utils/builtins.h
@@ -771,8 +771,8 @@ extern Datum unknownsend(PG_FUNCTION_ARGS);
extern Datum pg_column_size(PG_FUNCTION_ARGS);
-extern Datum bytea_agg_transfn(PG_FUNCTION_ARGS);
-extern Datum bytea_agg_finalfn(PG_FUNCTION_ARGS);
+extern Datum bytea_string_agg_transfn(PG_FUNCTION_ARGS);
+extern Datum bytea_string_agg_finalfn(PG_FUNCTION_ARGS);
extern Datum string_agg_transfn(PG_FUNCTION_ARGS);
extern Datum string_agg_finalfn(PG_FUNCTION_ARGS);
diff --git i/src/test/regress/expected/aggregates.out w/src/test/regress/expected/aggregates.out
index 2ec4eec..99ea5ef 100644
--- i/src/test/regress/expected/aggregates.out
Re: [HACKERS] patch: bytea_agg
On Fri, Dec 23, 2011 at 12:30 PM, Robert Haas wrote: > Well, because it doesn't operate on strings. > > I argued when we added string_agg that it ought to be called > concat_agg, or something like that, but I got shouted down. So now > here we are. +1. Using the input type names to name the function is a mistake and should be stopped...enough. It's verbose and unnecessary...everything else in sql is heavily overloaded (we don't have int_max() and float_max()). merlin -- 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] patch: bytea_agg
Excerpts from Pavel Stehule's message of vie dic 23 18:36:11 -0300 2011: > Hello > > 2011/12/23 Tom Lane : > > I generally agree with Peter: string_agg makes sense here. The only > > real argument against it is Pavel's point that he didn't include a > > delimiter parameter, but that just begs the question why not. It > > seems at least plausible that there would be use-cases for it. > > I don't know a real usage for bytea delimiter. Probably there is, but > I expect so most often use case will be without delimiter. Well, sometimes bytea is used to store character strings when the encoding information is to be handled by the app instead of having Postgres know it, for various reasons. I haven't seen any of those cases yet that would use string_add(bytea) but I wouldn't foreclose that possibility. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] patch: bytea_agg
Pavel Stehule wrote: > maybe we can introduce a synonym type for bytea - like "binary > string" or "bstring". The standard mentions these names for binary strings: BINARY, BINARY VARYING, or BINARY LARGE OBJECT which have a certain symmetry with: CHARACTER, CHARACTER VARYING, and CHARACTER LARGE OBJECT -Kevin -- 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] patch: bytea_agg
Hello 2011/12/23 Tom Lane : > Robert Haas writes: >> On Fri, Dec 23, 2011 at 2:16 PM, Peter Eisentraut wrote: >>> On fre, 2011-12-23 at 13:30 -0500, Robert Haas wrote: Well, because it doesn't operate on strings. > >>> Sure, binary strings. Both the SQL standard and the PostgreSQL >>> documentation use that term. > >> I'm unimpressed by that argument, but let's see what other people think. > > I generally agree with Peter: string_agg makes sense here. The only > real argument against it is Pavel's point that he didn't include a > delimiter parameter, but that just begs the question why not. It > seems at least plausible that there would be use-cases for it. I don't know a real usage for bytea delimiter. Probably there is, but I expect so most often use case will be without delimiter. And when it is necessary, then || should be used. I see two ways: a) use it bytea_agg as it now b) use a string_agg with delimiter, that will be usually empty. Using a string_agg for bytea is not too intuitive (but has sense) - maybe we can introduce a synonym type for bytea - like "binary string" or "bstring". Regards Pavel > > So I think we should try to make this as much like the text case as > possible. > > 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] patch: bytea_agg
Robert Haas writes: > On Fri, Dec 23, 2011 at 2:16 PM, Peter Eisentraut wrote: >> On fre, 2011-12-23 at 13:30 -0500, Robert Haas wrote: >>> Well, because it doesn't operate on strings. >> Sure, binary strings. Both the SQL standard and the PostgreSQL >> documentation use that term. > I'm unimpressed by that argument, but let's see what other people think. I generally agree with Peter: string_agg makes sense here. The only real argument against it is Pavel's point that he didn't include a delimiter parameter, but that just begs the question why not. It seems at least plausible that there would be use-cases for it. So I think we should try to make this as much like the text case as possible. 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] patch: bytea_agg
Robert Haas wrote: > Peter Eisentraut wrote: >> Robert Haas wrote: >>> Peter Eisentraut wrote: On ons, 2011-12-21 at 11:04 +0100, Pavel Stehule wrote: > this patch adds a bytea_agg aggregation. > > It allow fast bytea concatetation. Why not call it string_agg? All the function names are the same between text and bytea (e.g., ||, substr, position, length). It would be nice not to introduce arbitrary differences. >>> >>> Well, because it doesn't operate on strings. >> >> Sure, binary strings. Both the SQL standard and the PostgreSQL >> documentation use that term. > > I'm unimpressed by that argument, but let's see what other people > think. I, for one, try to be consistent about saying "character strings" when that is what I mean. Since at least the SQL-92 standard there have been both "character strings" and "bit strings", with a certain amount of symmetry in how they are handled. I don't remember when binary strings were introduced, but that is the standard terminology. There is, for example, a standard substring function for binary strings. -Kevin -- 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] patch: bytea_agg
On Fri, Dec 23, 2011 at 2:16 PM, Peter Eisentraut wrote: > On fre, 2011-12-23 at 13:30 -0500, Robert Haas wrote: >> On Fri, Dec 23, 2011 at 12:51 PM, Peter Eisentraut wrote: >> > On ons, 2011-12-21 at 11:04 +0100, Pavel Stehule wrote: >> >> this patch adds a bytea_agg aggregation. >> >> >> >> It allow fast bytea concatetation. >> > >> > Why not call it string_agg? All the function names are the same between >> > text and bytea (e.g., ||, substr, position, length). It would be nice >> > not to introduce arbitrary differences. >> >> Well, because it doesn't operate on strings. > > Sure, binary strings. Both the SQL standard and the PostgreSQL > documentation use that term. I'm unimpressed by that argument, but let's see what other people think. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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] patch: bytea_agg
On fre, 2011-12-23 at 13:30 -0500, Robert Haas wrote: > On Fri, Dec 23, 2011 at 12:51 PM, Peter Eisentraut wrote: > > On ons, 2011-12-21 at 11:04 +0100, Pavel Stehule wrote: > >> this patch adds a bytea_agg aggregation. > >> > >> It allow fast bytea concatetation. > > > > Why not call it string_agg? All the function names are the same between > > text and bytea (e.g., ||, substr, position, length). It would be nice > > not to introduce arbitrary differences. > > Well, because it doesn't operate on strings. Sure, binary strings. Both the SQL standard and the PostgreSQL documentation use that term. -- 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] patch: bytea_agg
On Fri, Dec 23, 2011 at 12:51 PM, Peter Eisentraut wrote: > On ons, 2011-12-21 at 11:04 +0100, Pavel Stehule wrote: >> this patch adds a bytea_agg aggregation. >> >> It allow fast bytea concatetation. > > Why not call it string_agg? All the function names are the same between > text and bytea (e.g., ||, substr, position, length). It would be nice > not to introduce arbitrary differences. Well, because it doesn't operate on strings. I argued when we added string_agg that it ought to be called concat_agg, or something like that, but I got shouted down. So now here we are. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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] patch: bytea_agg
Hello 2011/12/23 Peter Eisentraut : > On ons, 2011-12-21 at 11:04 +0100, Pavel Stehule wrote: >> this patch adds a bytea_agg aggregation. >> >> It allow fast bytea concatetation. > > Why not call it string_agg? All the function names are the same between > text and bytea (e.g., ||, substr, position, length). It would be nice > not to introduce arbitrary differences. My opinion is not strong. I don't think so using string_agg is good name (- as minimal (and only one) reason is different API - there is no support for delimiter. If I remember well discussion about string_agg, where delimiter is not optimal, there is request for immutable interface for aggregates - there was a issue with ORDER clause. So bytea_agg is good name. Regards Pavel > -- 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] patch: bytea_agg
Hello 2011/12/23 Peter Eisentraut : > On ons, 2011-12-21 at 11:04 +0100, Pavel Stehule wrote: >> this patch adds a bytea_agg aggregation. >> >> It allow fast bytea concatetation. > > Why not call it string_agg? All the function names are the same between > text and bytea (e.g., ||, substr, position, length). It would be nice > not to introduce arbitrary differences. My opinion is not too strong. I don't think so using string_agg is good name (for bytea_agg) - as minimal (and only one) reason is different API - there is no support for delimiter. If I remember well discussion about string_agg, where delimiter is not optimal, there is request for immutable interface for aggregates - there was a issue with ORDER clause. So bytea_agg is good name. Regards Pavel > -- 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] patch: bytea_agg
On ons, 2011-12-21 at 11:04 +0100, Pavel Stehule wrote: > this patch adds a bytea_agg aggregation. > > It allow fast bytea concatetation. Why not call it string_agg? All the function names are the same between text and bytea (e.g., ||, substr, position, length). It would be nice not to introduce arbitrary differences. -- 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] patch: bytea_agg
On Thu, Dec 22, 2011 at 11:49 AM, Robert Haas wrote: > On Wed, Dec 21, 2011 at 5:04 AM, Pavel Stehule > wrote: >> this patch adds a bytea_agg aggregation. >> >> It allow fast bytea concatetation. > > Looks fine to me. I'll commit this, barring objections. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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] patch: bytea_agg
On Wed, Dec 21, 2011 at 5:04 AM, Pavel Stehule wrote: > this patch adds a bytea_agg aggregation. > > It allow fast bytea concatetation. Looks fine to me. I'll commit this, barring objections. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] patch: bytea_agg
Hello
this patch adds a bytea_agg aggregation.
It allow fast bytea concatetation.
Regards
Pavel Stehule
*** ./doc/src/sgml/func.sgml.orig 2011-12-07 11:04:33.0 +0100
--- ./doc/src/sgml/func.sgml 2011-12-21 11:00:18.255753111 +0100
***
*** 10911,10916
--- 10911,10934
+ bytea_agg
+
+
+ bytea_agg(expression)
+
+
+
+bytea
+
+
+bytea
+
+ input values concatenated into a bytea
+
+
+
+
+
count
count(*)
*** ./src/backend/utils/adt/varlena.c.orig 2011-12-21 08:21:10.0 +0100
--- ./src/backend/utils/adt/varlena.c 2011-12-21 10:46:33.344807038 +0100
***
*** 396,401
--- 396,448
PG_RETURN_BYTEA_P(vlena);
}
+ Datum
+ bytea_agg_transfn(PG_FUNCTION_ARGS)
+ {
+ StringInfo state;
+
+ state = PG_ARGISNULL(0) ? NULL : (StringInfo) PG_GETARG_POINTER(0);
+
+ /* Append the value unless null. */
+ if (!PG_ARGISNULL(1))
+ {
+ bytea *value = PG_GETARG_BYTEA_PP(1);
+
+ if (state == NULL)
+ state = makeStringAggState(fcinfo);
+
+ appendBinaryStringInfo(state, VARDATA_ANY(value), VARSIZE_ANY_EXHDR(value));
+ }
+
+ /*
+ * The transition type for bytea_agg() is declared to be "internal",
+ * which is a pass-by-value type the same size as a pointer.
+ */
+ PG_RETURN_POINTER(state);
+ }
+
+ Datum
+ bytea_agg_finalfn(PG_FUNCTION_ARGS)
+ {
+ StringInfo state;
+
+ /* cannot be called directly because of internal-type argument */
+ Assert(AggCheckCallContext(fcinfo, NULL));
+
+ state = PG_ARGISNULL(0) ? NULL : (StringInfo) PG_GETARG_POINTER(0);
+
+ if (state != NULL)
+ {
+ bytea *result;
+
+ result = (bytea *) palloc(state->len + VARHDRSZ);
+ SET_VARSIZE(result, state->len + VARHDRSZ);
+ memcpy(VARDATA(result), state->data, state->len);
+ PG_RETURN_BYTEA_P(result);
+ }
+ else
+ PG_RETURN_NULL();
+ }
/*
* textin - converts "..." to internal representation
*** ./src/include/catalog/pg_aggregate.h.orig 2011-12-07 11:04:33.0 +0100
--- ./src/include/catalog/pg_aggregate.h 2011-12-21 10:28:37.016877356 +0100
***
*** 226,231
--- 226,234
/* text */
DATA(insert ( 3538 string_agg_transfn string_agg_finalfn 0 2281 _null_ ));
+ /* bytea */
+ DATA(insert ( 3545 bytea_agg_transfn bytea_agg_finalfn 0 2281 _null_ ));
+
/*
* prototypes for functions in pg_aggregate.c
*/
*** ./src/include/catalog/pg_proc.h.orig 2011-12-21 08:21:10.0 +0100
--- ./src/include/catalog/pg_proc.h 2011-12-21 10:25:29.533889614 +0100
***
*** 2403,2414
--- 2403,2421
DESCR("aggregate final function");
DATA(insert OID = 2817 ( float8_corrPGNSP PGUID 12 1 0 0 0 f f f t f i 1 0 701 "1022" _null_ _null_ _null_ _null_ float8_corr _null_ _null_ _null_ ));
DESCR("aggregate final function");
+
DATA(insert OID = 3535 ( string_agg_transfn PGNSP PGUID 12 1 0 0 0 f f f f f i 3 0 2281 "2281 25 25" _null_ _null_ _null_ _null_ string_agg_transfn _null_ _null_ _null_ ));
DESCR("aggregate transition function");
DATA(insert OID = 3536 ( string_agg_finalfn PGNSP PGUID 12 1 0 0 0 f f f f f i 1 0 25 "2281" _null_ _null_ _null_ _null_ string_agg_finalfn _null_ _null_ _null_ ));
DESCR("aggregate final function");
DATA(insert OID = 3538 ( string_aggPGNSP PGUID 12 1 0 0 0 t f f f f i 2 0 25 "25 25" _null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ ));
DESCR("concatenate aggregate input into a string");
+ DATA(insert OID = 3543 ( bytea_agg_transfn PGNSP PGUID 12 1 0 0 0 f f f f f i 2 0 2281 "2281 17" _null_ _null_ _null_ _null_ bytea_agg_transfn _null_ _null_ _null_ ));
+ DESCR("aggregate transition function");
+ DATA(insert OID = 3544 ( bytea_agg_finalfn PGNSP PGUID 12 1 0 0 0 f f f f f i 1 0 17 "2281" _null_ _null_ _null_ _null_ bytea_agg_finalfn _null_ _null_ _null_ ));
+ DESCR("aggregate final function");
+ DATA(insert OID = 3545 ( bytea_aggPGNSP PGUID 12 1 0 0 0 t f f f f i 1 0 17 "17" _null_ _null_ _null_ _null_ aggregate_dummy _null_ _null_ _null_ ));
+ DESCR("concatenate aggregate input into a bytea");
/* To ASCII conversion */
DATA(insert OID = 1845 ( to_ascii PGNSP PGUID 12 1 0 0 0 f f f t f i 1 0 25 "25" _null_ _null_ _null_ _null_ to_ascii_default _null_ _null_ _null_ ));
*** ./src/include/utils/builtins.h.orig 2011-12-21 08:21:10.0 +0100
--- ./src/include/utils/builtins.h 2011-12-21 10:16:10.521926024 +0100
***
*** 769,774
--- 769,776
extern Datum pg_column_size(PG_FUNCTION_ARGS);
+ extern Datum bytea_agg_transfn(PG_FUNCTION_ARGS);
+ extern Datum bytea_agg_finalfn(PG_FUNCTION_ARGS);
extern Datum string_agg_transfn(PG_FUNCTION_ARGS);
extern Datum string_agg_finalfn(PG_FUNCTION_ARGS);
*** ./src/test/regress/expected/aggregates.out.orig 2011-12-07 11:04:33.0 +0100
--- ./src/test/regress/expecte
