Re: [PATCH] Split varlena.c into varlena.c and bytea.c

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