Обсуждение: Use PointerGetDatum(cstring_to_text_with_len()) instead of CStringGetTextDatum() to avoid duplicate strlen

Hi

I found some code like the following:

> StringInfoData s;
> ...
> values[6] = CStringGetTextDatum(s.data);

The length of string can be found in ' StringInfoData.len',
but the macro CStringGetTextDatum will use strlen to count the length again.
I think we can use PointerGetDatum(cstring_to_text_with_len(s.data, s.len)) to improve.

> #define CStringGetTextDatum(s) PointerGetDatum(cstring_to_text(s))
> text *
> cstring_to_text(const char *s)
> {
>     return cstring_to_text_with_len(s, strlen(s));
> }


There may have more places that can get the length of string in advance,
But that may need new variable to store it ,So I just find all StringInfoData cases.

Best regards,
houzj

On 19/10/2020 09:32, Hou, Zhijie wrote:
> Hi
> 
> I found some code like the following:
> 
>> StringInfoData s;
>> ...
>> values[6] = CStringGetTextDatum(s.data);
> 
> The length of string can be found in ' StringInfoData.len',
> but the macro CStringGetTextDatum will use strlen to count the length again.
> I think we can use PointerGetDatum(cstring_to_text_with_len(s.data, s.len)) to improve.
> 
>> #define CStringGetTextDatum(s) PointerGetDatum(cstring_to_text(s))
>> text *
>> cstring_to_text(const char *s)
>> {
>>     return cstring_to_text_with_len(s, strlen(s));
>> }
> 
> 
> There may have more places that can get the length of string in advance,
> But that may need new variable to store it ,So I just find all StringInfoData cases.

None of these calls are performance-critical, so it hardly matters. I 
would rather keep them short and simple.

It might make sense to create a new macro or function for this, though. 
Something like:

static inline text *
StringInfoGetTextDatum(StringInfo s)
{
     return cstring_to_text_with_len(s->data, s->len);
}

That would perhaps make existing code a bit shorter and nicer to read.

- Heikki