Re: Patch to improve a few appendStringInfo* calls

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch to improve a few appendStringInfo* calls
Date: 2015-07-02 09:59:15
Message-ID: 55950B73.2040406@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 06/15/2015 03:56 AM, David Rowley wrote:
> On 29 May 2015 at 12:51, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
>
>> On 5/12/15 4:33 AM, David Rowley wrote:
>>> Shortly after I sent the previous patch I did a few more searches and
>>> also found some more things that are not quite right.
>>> Most of these are to use the binary append method when the length of the
>>> string is already known.
>>
>> For these cases it might be better to invent additional functions such
>> as stringinfo_to_text() and appendStringInfoStringInfo() instead of
>> repeating the pattern of referring to data and length separately.
>
> You're probably right. It would be nicer to see some sort of wrapper
> functions that cleaned these up a bit.
>
> I really think that's something for another patch though, this patch just
> intends to put things the way they're meant to be in the least invasive way
> possible. What you're proposing is changing the way it's meant to work,
> which will cause much more code churn.
>
> I've attached a re-based patch.

Applied the straightforward parts. I left out the changes like

> - appendStringInfoString(&collist, buf.data);
> + appendBinaryStringInfo(&collist, buf.data, buf.len);

because they're not an improvement in readablity, IMHO, and they were
not in performance-critical paths.

I also noticed that the space after "CREATE EVENT TRIGGER <name>" in
pg_dump was actually spurious, and just added a space before newline. I
removed that space altogether,

- Heikki

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2015-07-02 10:08:23 Re: Patch to improve a few appendStringInfo* calls
Previous Message Kouhei Kaigai 2015-07-02 09:31:35 Re: Foreign join pushdown vs EvalPlanQual