Skip site navigation (1) Skip section navigation (2)

Re: [PATCH] Make pg_basebackup configure and start standby [Review]

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Boszormenyi Zoltan <zb(at)cybertec(dot)at>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Make pg_basebackup configure and start standby [Review]
Date: 2013-01-02 09:12:44
Message-ID: (view raw, whole thread or download thread mbox)
Lists: pgsql-hackers
On Wed, Jan 2, 2013 at 9:59 AM, Boszormenyi Zoltan <zb(at)cybertec(dot)at> wrote:

> 2013-01-02 01:24 keltezéssel, Tom Lane írta:
>  Boszormenyi Zoltan <zb(at)cybertec(dot)at> writes:
>>> 2013-01-01 17:18 keltezéssel, Magnus Hagander írta:
>>>> That way we can get around the whole need for changing memory
>>>> allocation across all the
>>>> frontends, no? Like the attached.
>>> Sure it's simpler but then the consistent look of the code is lost.
>>> What about the other patch to unify pg_malloc and friends?
>>> Basically all client code boils down to
>>>       fprintf(stderr, ...)
>>> in different disguise in their error reporting, so that patch can
>>> also be simplified but it seems that the atexit() - either explicitly
>>> or hidden behind InitPostgresFrontend() - cannot be avoided.
>> Meh.  I find it seriously wrongheaded that something as minor as an
>> escape_quotes() function should get to dictate both malloc wrappers
>> and error recovery handling throughout every program that might use it.
> Actually, the unification of pg_malloc and friends wasn't dictated
> by this little code, it was just that pg_basebackup doesn't provide
> a pg_malloc implementation (only pg_malloc0) that is used by
> initdb's escape_quotes() function. Then I noticed how wide these
> almost identical functions have spread into client apps already.
> I would say this unification patch is completely orthogonal to
> the patch in $SUBJECT. I will post it in a different thread if it's
> wanted at all. The extra atexit() handler is not needed if a simple
> fprintf(stderr, ...) error reporting is enough in all clients.
> As far as I saw, all clients do exactly this but some of them hide
> this behind #define's.

Please do keep that one separate - let's avoid unnecessary feature-creep,
whether it's good or bad features.

I like Magnus' version a lot better than that idea.
> OK, I will post the core patch building on his code.


 A bigger issue that I notice with this code is that it's only correct in
>> backend-safe encodings, as the comment mentions.  If we're going to be
>> putting it into frontend programs, how safe is that going to be?
>>                         regards, tom lane
> The question in a different form is: does PostgreSQL support
> non-ASCII-safe encodings at all (or on the client side)? Forgive
> my ignorance and enlighten me: how many such encodings
> exist besides EBCDIC? Is UTF-16 non-ASCII-safe?
We do. See

There are quite a few far-eastern encodings that aren't available as server
encondings, and I believe it's all for this reason.

That said, do we need to care *in this specific case*? We use it in initdb
to parse config strings, I believe. And we'd use it to parse a conninfo
string in pg_basebackup, correct? Perhaps all we need to do is to clearly
comment that it doesn't work with non-ascii safe encodings, or rename it to
indicate that it's limited in this?

 Magnus Hagander

In response to


pgsql-hackers by date

Next:From: Boszormenyi ZoltanDate: 2013-01-02 09:37:21
Subject: Re: [PATCH] Make pg_basebackup configure and start standby [Review]
Previous:From: Magnus HaganderDate: 2013-01-02 09:06:14
Subject: Re: pgsql: Unify some tar functionality across different parts

Privacy Policy | About PostgreSQL
Copyright © 1996-2017 The PostgreSQL Global Development Group