Re: [PATCH] Refactor "if(strspn(str, ...) == strlen(str)" code

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Geoff Winkless <pgsqladmin(at)geoff(dot)dj>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Refactor "if(strspn(str, ...) == strlen(str)" code
Date: 2017-01-11 07:50:13
Message-ID: CAB7nPqSDiNAyv8OkK3DXKkfd2tovSkaj1EhJW1DwRDnR7AypsA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Dec 9, 2016 at 3:23 AM, Aleksander Alekseev
<a(dot)alekseev(at)postgrespro(dot)ru> wrote:
>> You could just change it to
>> if (str[strspn(str, " \t\n\r\f")] == '\0')
>> to mitigate calling strlen. It's safe to do so because strspn will
>> only return values from 0 to strlen(str).
>
>> [...] I have serious doubts that the "optimized" implementation
>> you propose is actually faster than a naive one; it may be slower, and
>> it's certainly longer and harder to understand/test.
>
> I would like to point out that I never said it's optimized. However I
> like the code Geoff proposed. It definitely doesn't make anything worse.
> I decided to keep pg_str_contansonly procedure (i.e. not to inline this
> code) for now. Code with strspn looks OK in a simple example. However in
> a concrete context it looks like a bad Perl code in ROT13 to me:

Looking at this patch, I am not sure that it is worth worrying about.
This is a receipt to make back-patching a bit more complicated, and it
makes the code more complicated to understand. So I would vote for
rejecting it and move on.

By the way, as you are placing this routine in src/common/, you may
want to consider updating the code in src/bin/ that use libpqcommon.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Petr Jelinek 2017-01-11 08:11:35 Re: Logical Replication WIP
Previous Message 高增琦 2017-01-11 07:46:48 Do we support using agg or window functions in delete statement?