Thread: pgsql: Centralize definition of integer limits.

Centralize definition of integer limits.

Several submitted and even committed patches have run into the problem
that C89, our baseline, does not provide minimum/maximum values for
various integer datatypes. C99's stdint.h does, but we can't rely on
it.

Several parts of the code defined limits locally, so instead centralize
the definitions to c.h.

This patch also changes the more obvious usages of literal limit values;
there's more places that could be changed, but it's less clear whether
it's beneficial to change those.

Author: Andrew Gierth
Discussion: 87619tc5wc.fsf@news-spur.riddles.org.uk

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/83ff1618bc9d4e530d3ef2a668a71326784a753c

Modified Files
--------------
contrib/btree_gist/btree_ts.c             |    4 +--
contrib/intarray/_int_gist.c              |    4 ++-
contrib/pgbench/pgbench.c                 |    6 +----
src/backend/access/transam/xlog.c         |    2 +-
src/backend/tsearch/wparser_def.c         |    6 +++--
src/backend/utils/adt/int8.c              |    2 +-
src/backend/utils/adt/numutils.c          |    2 +-
src/backend/utils/adt/timestamp.c         |    8 ------
src/backend/utils/adt/tsrank.c            |    3 ++-
src/backend/utils/adt/txid.c              |    2 +-
src/include/c.h                           |   41 +++++++++++++++++++++++++++++
src/include/datatype/timestamp.h          |    4 +--
src/include/executor/instrument.h         |    2 +-
src/include/nodes/parsenodes.h            |    2 +-
src/include/pg_config_manual.h            |    4 +--
src/include/port/atomics.h                |    4 +--
src/include/storage/predicate_internals.h |    2 +-
src/include/utils/date.h                  |    6 ++---
18 files changed, 68 insertions(+), 36 deletions(-)

Hi,

On 2015-03-30 14:01:25 +0900, Michael Paquier wrote:
> On Thu, Mar 26, 2015 at 6:49 AM, Andres Freund <andres@anarazel.de> wrote:
> > Centralize definition of integer limits.
> >
> > Several submitted and even committed patches have run into the problem
> > that C89, our baseline, does not provide minimum/maximum values for
> > various integer datatypes. C99's stdint.h does, but we can't rely on
> > it.
> >
> > Several parts of the code defined limits locally, so instead centralize
> > the definitions to c.h.
> >
> > This patch also changes the more obvious usages of literal limit values;
> > there's more places that could be changed, but it's less clear whether
> > it's beneficial to change those.
>
> My OSX dev box is generating a couple of warnings since this commit:
> pg_dump.c:14548:45: warning: format specifies type 'long' but the argument
> has type 'long long' [-Wformat]
>         snprintf(bufm, sizeof(bufm), INT64_FORMAT, SEQ_MINVALUE);
>         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
> ../../../src/include/pg_config_manual.h:52:22: note: expanded from macro
> 'SEQ_MINVALUE'
> #define SEQ_MINVALUE    (-SEQ_MAXVALUE)
>                         ^
> /usr/include/secure/_stdio.h:56:62: note: expanded from macro 'snprintf'
>   __builtin___snprintf_chk (str, len, 0, __darwin_obsz(str), __VA_ARGS__)
>                                                              ^
> pg_dump.c:14549:45: warning: format specifies type 'long' but the argument
> has type 'long long' [-Wformat]
>         snprintf(bufx, sizeof(bufx), INT64_FORMAT, SEQ_MAXVALUE);
>         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
> ../../../src/include/pg_config_manual.h:51:22: note: expanded from macro
> 'SEQ_MAXVALUE'
> #define SEQ_MAXVALUE    INT64_MAX
>                         ^~~~~~~~~
> /usr/include/stdint.h:122:26: note: expanded from macro 'INT64_MAX'
> #define INT64_MAX        9223372036854775807LL
>                          ^~~~~~~~~~~~~~~~~~~~~
> /usr/include/secure/_stdio.h:56:62: note: expanded from macro 'snprintf'
>   __builtin___snprintf_chk (str, len, 0, __darwin_obsz(str), __VA_ARGS__)

Hm. Can you send config.log and the generated pg_config.h?

Greetings,

Andres Freund

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On 2015-03-30 14:27:20 +0900, Michael Paquier wrote:
> INT64_MODIFIER is "l" on OSX, causing this warning. Perhaps there is
> something better to do instead of casting blindly to int64. Thoughts?
> --
> Michael

> diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
> index 7da5c41..a15c875 100644
> --- a/src/bin/pg_dump/pg_dump.c
> +++ b/src/bin/pg_dump/pg_dump.c
> @@ -14545,8 +14545,8 @@ dumpSequence(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
>      /* Make sure we are in proper schema */
>      selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name);
>
> -    snprintf(bufm, sizeof(bufm), INT64_FORMAT, SEQ_MINVALUE);
> -    snprintf(bufx, sizeof(bufx), INT64_FORMAT, SEQ_MAXVALUE);
> +    snprintf(bufm, sizeof(bufm), INT64_FORMAT, (int64) SEQ_MINVALUE);
> +    snprintf(bufx, sizeof(bufx), INT64_FORMAT, (int64) SEQ_MAXVALUE);
>

I think that's entirely the wrong approach. ISTM that, independent from
this patch, INT64CONST/HAVE_LL_CONSTANTS are wrong for your
environment. The HAVE_LL_CONSTANTS test is probably too simplistic.

Greetings,

Andres Freund

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services