Re: Replace some cstring_to_text to cstring_to_text_with_len

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: John Naylor <john(dot)naylor(at)enterprisedb(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Replace some cstring_to_text to cstring_to_text_with_len
Date: 2023-08-31 15:28:57
Message-ID: 2bc4561f-cf71-bb65-a6a1-0eac8be8e238@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 31.08.23 16:10, Ranier Vilela wrote:
> Em qui., 31 de ago. de 2023 às 09:51, Andrew Dunstan
> <andrew(at)dunslane(dot)net <mailto:andrew(at)dunslane(dot)net>> escreveu:
>
>
> On 2023-08-31 Th 07:41, John Naylor wrote:
>>
>> On Thu, Aug 31, 2023 at 6:07 PM Ranier Vilela <ranier(dot)vf(at)gmail(dot)com
>> <mailto:ranier(dot)vf(at)gmail(dot)com>> wrote:
>> >
>> > Em qui., 31 de ago. de 2023 às 00:22, Michael Paquier
>> <michael(at)paquier(dot)xyz <mailto:michael(at)paquier(dot)xyz>> escreveu:
>> >>
>> >> On Wed, Aug 30, 2023 at 03:00:13PM -0300, Ranier Vilela wrote:
>> >> > cstring_to_text has a small overhead, because call strlen for
>> >> > pointer to char parameter.
>> >> >
>> >> > Is it worth the effort to avoid this, where do we know the
>> size of the
>> >> > parameter?
>> >>
>> >> Are there workloads where this matters?
>> >
>> > None, but note this change has the same spirit of 8b26769bc.
>>
>> - return cstring_to_text("");
>> + return cstring_to_text_with_len("", 0);
>>
>> This looks worse, so we'd better be getting something in return.
>
>
> I agree this is a bit ugly. I wonder if we'd be better off creating
> a function that returned an empty text value, so we'd just avoid
> converting the empty cstring altogether and say:
>
>   return empty_text();
>
> Hi,
> Thanks for the suggestion,  I agreed.
>
> New patch is attached.

I think these patches make the code uniformly uglier and harder to
understand.

If a performance benefit could be demonstrated, then making
cstring_to_text() an inline function could be sensible. But I wouldn't
go beyond that.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Mat Arye 2023-08-31 15:32:36 Re: UUID v7
Previous Message Robert Haas 2023-08-31 15:25:33 sandboxing untrusted code