Re: pg_dump -j against standbys

On Tue, May 24, 2016 at 5:39 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Magnus Hagander <magnus(at)hagander(dot)net> writes:
> > I think the cleanest way to do it is to just track if it's a standby in
> the
> > AH struct as written.
>
> > Comments?
>
> This patch will cause pg_dump to fail entirely against pre-9.0 servers.
> You need to execute the test conditionally.
>

Ugh. can I blame coding while too jetlagged?

> Also, in view of that, this test
>
> - if (fout->remoteVersion >= 90000)
> + if (fout->remoteVersion >= 90000 && fout->isStandby)
>
> could be reduced to just "if (fout->isStandby)". And the comment in
> front of it could stand some attention (possibly just replace it with
> the one that's currently within the inner if() ... actually, that
> comment should move to where you moved the test to, no?)
>

True. Will fix.

> Also, why didn't you keep using ExecuteSqlQueryForSingleRow()?
> As coded, you're losing a sanity check that the query gives exactly
> one row back.
>
>
The reason I did that is that ExecuteSqlQueryForSingleRow() is a static
method in pg_dump.c. I was planning to go back and review that, and
consider moving it, but I forgot it :S

I think the clean thing is probably to use that one, and also move it over
to not be a static method in pg_dump.c, but instead sit next to
ExecuteSqlQuery in pg_backup_db.c. Do you agree that's reasonable, and
something that's OK to backpatch?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/