Hi Peter, > >> The proposed patch extracts the code dealing with bytea from varlena.c > >> into a separate file, as proposed previously [1]. It merely changes > >> the location of the existing functions. There are no other changes. > > > > Rebased. > > I think this is reasonable. varlena.c is pretty big, and bytea is a > reasonable subset to take out. We already have a corresponding header > file bytea.h, so the naming fits. > > I wonder if makeStringAggState() is still useful. This was factored out > so that it could be shared between bytea_string_agg_transfn() and > string_agg_transfn(), but now that these are in separate files, it seems > to me that it might be easier now to just write the required code inline.
Agree. I assume you meant only bytea.c. Since varlena.c uses makeStringAggState() in several places I choose to keep it there. > Here are some refinements to the includes: > > --- a/src/backend/utils/adt/bytea.c > +++ b/src/backend/utils/adt/bytea.c > @@ -15,13 +15,20 @@ > #include "postgres.h" > > #include "access/detoast.h" > -#include "catalog/pg_collation.h" > +#include "catalog/pg_collation_d.h" > +#include "catalog/pg_type_d.h" > #include "common/int.h" > -#include "funcapi.h" > +#include "fmgr.h" > #include "libpq/pqformat.h" > +#include "port/pg_bitutils.h" > #include "utils/builtins.h" > #include "utils/bytea.h" > +#include "utils/fmgroids.h" > +#include "utils/fmgrprotos.h" > +#include "utils/memutils.h" > +#include "utils/sortsupport.h" > #include "utils/varlena.h" > +#include "varatt.h" > > Especially funcapi.h was apparently not used. The other additions are > required includes that came previously via indirect includes. Thanks, fixed. clangd complained that utils/fmgroids.h is not going to be used, as it complained about funcapi.h before. So I choose not to include fmgroids.h. PFA patch v3. -- Best regards, Aleksander Alekseev
v3-0001-Split-varlena.c-into-varlena.c-and-bytea.c.patch
Description: Binary data
