Fix string literal length for compressed_data_info

[mkindahl]

The function compressed_data_info returns a record containing name
but copies the data from string literal. Since heap_form_tuple expects a
name value as source as well, it will copy too much data from the
source, leading to ASAN complaining about copying outside the allocated
range of data.

The problem is fixed by NameData for each
source string, ensuring that each of the string literals has sufficient
data for being copied as names.

[svenklemm]

cstring seems like an unusual return type choice, why not make it text?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.81%. Comparing base (59f50f2) to head (b6b51e6).
Report is 288 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7187      +/-   ##
==========================================
+ Coverage   80.06%   81.81%   +1.74%     
==========================================
  Files         190      203      +13     
  Lines       37181    38048     +867     
  Branches     9450     9858     +408     
==========================================
+ Hits        29770    31128    +1358     
+ Misses       2997     2960      -37     
+ Partials     4414     3960     -454     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mkindahl]

cstring seems like an unusual return type choice, why not make it text?

It can be made text, but internally it is a string literal and I see no advantage to make it a text type. Since the convention seems to be for most other build-in PostgreSQL functions that return string literals to use cstring return type, it seems like the most natural choice.

For example:

mats=# select proname, proargtypes::regtype[],prorettype::regtype from pg_proc where prorettype = 'cstring'::regtype limit 10;
    proname    |    proargtypes     | prorettype 
---------------+--------------------+------------
 boolout       | [0:0]={boolean}    | cstring
 byteaout      | [0:0]={bytea}      | cstring
 charout       | [0:0]={"\"char\""} | cstring
 nameout       | [0:0]={name}       | cstring
 int2out       | [0:0]={smallint}   | cstring
 int2vectorout | [0:0]={int2vector} | cstring
 int4out       | [0:0]={integer}    | cstring
 regprocout    | [0:0]={regproc}    | cstring
 textout       | [0:0]={text}       | cstring
 tidout        | [0:0]={tid}        | cstring
(10 rows)

If you feel strongly about it, I can use a text type.

[erimatnor]

cstring seems like an unusual return type choice, why not make it text?

It can be made text, but internally it is a string literal and I see no advantage to make it a text type. Since the convention seems to be for most other build-in PostgreSQL functions that return string literals to use cstring return type, it seems like the most natural choice.

For example:

mats=# select proname, proargtypes::regtype[],prorettype::regtype from pg_proc where prorettype = 'cstring'::regtype limit 10;
    proname    |    proargtypes     | prorettype 
---------------+--------------------+------------
 boolout       | [0:0]={boolean}    | cstring
 byteaout      | [0:0]={bytea}      | cstring
 charout       | [0:0]={"\"char\""} | cstring
 nameout       | [0:0]={name}       | cstring
 int2out       | [0:0]={smallint}   | cstring
 int2vectorout | [0:0]={int2vector} | cstring
 int4out       | [0:0]={integer}    | cstring
 regprocout    | [0:0]={regproc}    | cstring
 textout       | [0:0]={text}       | cstring
 tidout        | [0:0]={tid}        | cstring
(10 rows)

If you feel strongly about it, I can use a text type.

Or, we could just make sure we return a name type since that is the definition from the start.

[svenklemm]

cstring is part of the interface of out functions but outside of that it is not used, for those text is used. eg pg_catalog.version which returns a static string returns text

sven@test[434551]=# SELECT oid::regprocedure from pg_proc where prorettype = 'cstring'::regtype and proname not like '%out' and proname not like 'cstring_%';
 oid 
-----
(0 rows)

[erimatnor]

cstring is part of the interface of out functions but outside of that it is not used, for those text is used. eg pg_catalog.version which returns a static string returns text

sven@test[434551]=# SELECT oid::regprocedure from pg_proc where prorettype = 'cstring'::regtype and proname not like '%out' and proname not like 'cstring_%';
 oid 
-----
(0 rows)

FWIW, this is kind of like an out function for compressed_data.

[erimatnor]

LGTM, don't care too much about the return type. This is just an internal function used mostly for debugging so cstring is fine I guess. Still, I would probably just have fixed the code to construct a NAME object instead of changing the function definition.

[mkindahl]

cstring is part of the interface of out functions but outside of that it is not used, for those text is used. eg pg_catalog.version which returns a static string returns text

sven@test[434551]=# SELECT oid::regprocedure from pg_proc where prorettype = 'cstring'::regtype and proname not like '%out' and proname not like 'cstring_%';
 oid 
-----
(0 rows)

Switched to using name and just ensuring that the source buffer was big enough.