Re: pg_dump sometimes misses sequence parameters

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alexey Bashtanov <bashtanov(at)imap(dot)cc>
Cc: pgsql-bugs(at)postgresql(dot)org, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Subject: Re: pg_dump sometimes misses sequence parameters
Date: 2018-02-20 16:39:20
Message-ID: 10612.1519144760@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Alexey Bashtanov <bashtanov(at)imap(dot)cc> writes:
> Current code looks somewhat fragile to me, as it takes some time to assure
> it doesn't pass already nulled out minv or maxv to atoi or strcmp again.
> I've rewritten it in a bit more clear way, so each comparison and
> nulling out happens only once.

Good idea ...

> Choosing between atol/atoll vs snprintf+strcmp,
> I preferred the former, as the latter is slower and uglier to me.

... but I was not thrilled with the way you did that. I don't like
having to back-patch new portability requirements with no time for them
to go through beta testing. What's more, this is randomly unlike the way
we're doing things elsewhere. I call your attention to pg_strtouint64
in src/backend/utils/adt/numutils.c.

What I did for the moment was to implement the comparison using
snprintf+strcmp, since that seemed like a very safely back-patchable
approach. While there's nothing particularly wrong with leaving
it like that, we could imagine creating a src/port module that
contains pg_strtouint64 and a corresponding implementation of
pg_strtoint64, and then fixing pg_dump to use pg_strtoint64 here.
I'd only consider doing that in HEAD though.

> BTW what should I do when submitting a patch if my autoconf adds
> some irrelevant changes?

The easy way is just to omit the configure file from the submitted
patch, noting that the committer must run autoconf. Most committers
probably would do that anyway (I certainly would).

However, if you want to do it right, or if you're someday given a
commit bit and you have to do it right, our project policy is that
we use exactly the GNU-released autoconf of the appropriate version,
so that anyone can reproduce the results, independently of patches
that their distribution might install into their platform's copy of
autoconf. So just pull the right autoconf tarball from any handy
GNU archive mirror, "configure --prefix=/something; make; make install",
prepend /something/bin to your PATH, and away you go. Personally
I keep autoconf N.NN in /usr/local/autoconf-N.NN/ so that I can use
whichever version is appropriate for the PG branch I'm working on.

regards, tom lane

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Lars Vonk 2018-02-20 17:52:35 Re: BUG #15074: psql client never returns when creating index (long running operation)
Previous Message Alexey Bashtanov 2018-02-20 14:06:10 Re: pg_dump sometimes misses sequence parameters