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

Hi,

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.

[1]: http://postgr.es/m/Zy2UqcZS2XAXBibq%40paquier.xyz

-- 
Best regards,
Aleksander Alekseev

> 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.


--
Best regards,
Aleksander Alekseev

On 31.03.25 17:29, Aleksander Alekseev wrote:
>> 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.

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.

Hi Michael,

> /* text_name()
>  * Converts a text type to a Name type.
>  */
>
> Not related to this patch, sorry for the regression, just noticed
> a nit while looking at the diffs of what you have here..  This one, as
> well as name_text(), uses a comment block that is inconsistent with
> the format we have in the tree.  It's a bit surprising that pgindent
> is not picking up that.

Good observation!

I experimented with pgindent a bit. This is an effect of the -nfc1
flag i.e. format_col1_comments=OFF. Unfortunately just removing it has
undesired side effects so we will have to modify pg_bsd_indent a bit,
apparently somewhere here:

pr_commnet.c:
```
    if (ps.col_1 && !format_col1_comments) {    /* if comment starts in column
                         * 1 it should not be touched */
```

I will start a new thread and propose an appropriate patch.

-- 
Best regards,
Aleksander Alekseev