Re: pg_dump sometimes misses sequence parameters

From: Alexey Bashtanov <bashtanov(at)imap(dot)cc>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
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 14:06:10
Message-ID: ad692667-fe43-62c5-5e55-e40a8596b621@imap.cc
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Hi Tom,

> That will only fix it on platforms where long is 64 bits.
Oops. I erroneously thought its size could drift the other way.

> I think that converting these strings to integer at all is a dumb
> idea.  It would be
> much better to write the first two tests like
>
>     if (is_ascending && strcmp(minv, "1") == 0)
>         minv = NULL;
>     if (!is_ascending && strcmp(maxv, "-1") == 0)
>         maxv = NULL;
Agree for those two. However, I don't see it as a substantial speedup
or simplification if we anyway do the conversions below.

> It's tempting to try to make the remaining tests look similar, but
> I'm not quite sure how without writing out the exact decimal values
> of the constants, which probably isn't better.  I experimented with
> stuff like "strcmp(minv, CppAsString2(PG_INT32_MIN))" but that
> doesn't quite do what we need.
I didn't manage to find a snprintf-like thing implemented in C
preprocessor language either, and I don't think it would be easy
to write one.

Unwilling to define the decimal representations constants, we have to
convert those strings representations to binary ones
(in case of smallint/integer sequences) or visa versa (bigint).

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. Choosing between atol/atoll vs snprintf+strcmp,
I preferred the former, as the latter is slower and uglier to me.

Should we need any further speedup, I'd go for decimal representation
constants really.

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

Best regards,
  Alexey Bashtanov

Attachment Content-Type Size
pgdump_seq_v2.diff text/x-patch 9.5 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2018-02-20 16:39:20 Re: pg_dump sometimes misses sequence parameters
Previous Message Sudalai 2018-02-20 11:01:04 Re: Segmentation Fault in logical decoding get/peek API