Thread: int4 or int32
Which one of these should we use? int4 is a data type, int32 isn't. c.h has DatumGetInt8, but no DatumGetInt64; it also has DatumGetInt32 but no DatumGetInt4. fmgr has PG_GETARG_INT32 et al. Inconsistency everywhere. The C standard has things like int32_t, but technically there's no guarantee that int32 is really 32 bits, you only know sizeof(int32) == 4. -- Peter Eisentraut peter_e@gmx.net http://yi.org/peter-e/
Peter Eisentraut <peter_e@gmx.net> writes:
> Which one of these should we use?
> int4 is a data type, int32 isn't. c.h has DatumGetInt8, but no
> DatumGetInt64; it also has DatumGetInt32 but no DatumGetInt4. fmgr has
> PG_GETARG_INT32 et al. Inconsistency everywhere.
The original convention was to use int4 etc at the SQL level, int32 etc
at the C level. However the typedefs int4 etc have to be visible in
the include/catalog/pg_*.h headers, and so there's been a certain amount
of leakage of those typedefs into the C sources.
I think that int32 etc are better choices at the C level because of
the well-established precedent for naming integer types after numbers
of bits in C code. I don't feel any strong urge to go around and
change the existing misusages, but if you want to, I won't object.
I also have to plead guilty to having changed all the float-datatype
code to use float4 and float8 recently. This was mainly because the
existing typedefs for float32 and float64 had a built-in assumption
that these types would always be pass-by-reference, and I wanted to
abstract the code away from that assumption. We can't touch those
typedefs for a release or three (else we'll break existing user
functions written in C), so switching to the SQL-level names seemed
like the best bet. But it's not real consistent with the integer-type
naming conventions :-(
regards, tom lane
> Peter Eisentraut <peter_e@gmx.net> writes: > > Which one of these should we use? > > int4 is a data type, int32 isn't. c.h has DatumGetInt8, but no > > DatumGetInt64; it also has DatumGetInt32 but no DatumGetInt4. fmgr has > > PG_GETARG_INT32 et al. Inconsistency everywhere. > > The original convention was to use int4 etc at the SQL level, int32 etc > at the C level. However the typedefs int4 etc have to be visible in > the include/catalog/pg_*.h headers, and so there's been a certain amount > of leakage of those typedefs into the C sources. > > I think that int32 etc are better choices at the C level because of > the well-established precedent for naming integer types after numbers > of bits in C code. I don't feel any strong urge to go around and > change the existing misusages, but if you want to, I won't object. > > I also have to plead guilty to having changed all the float-datatype > code to use float4 and float8 recently. This was mainly because the > existing typedefs for float32 and float64 had a built-in assumption > that these types would always be pass-by-reference, and I wanted to > abstract the code away from that assumption. We can't touch those > typedefs for a release or three (else we'll break existing user > functions written in C), so switching to the SQL-level names seemed > like the best bet. But it's not real consistent with the integer-type > naming conventions :-( Tom, I am wondering. If we don't change to int4/int8 internally now, will we ever do it? The function manager change seems like the only good time to do it, if we ever will. Basically, I am asking if we should drop backward C compatibility for 7.1 and bite the bullet on the change? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> I think that int32 etc are better choices at the C level because of
>> the well-established precedent for naming integer types after numbers
>> of bits in C code. I don't feel any strong urge to go around and
>> change the existing misusages, but if you want to, I won't object.
> Tom, I am wondering. If we don't change to int4/int8 internally now,
> will we ever do it?
As I thought I'd just made clear, I'm against standardizing on int4/int8
at the C level. The average C programmer would think that "int8" is
a one-byte type, not an eight-byte type. There's way too much history
behind that for us to swim against the tide. Having different naming
conventions at the C and SQL levels seems a better approach, especially
since it will exist to some extent anyway (int != integer, for
instance).
regards, tom lane
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > >> I think that int32 etc are better choices at the C level because of > >> the well-established precedent for naming integer types after numbers > >> of bits in C code. I don't feel any strong urge to go around and > >> change the existing misusages, but if you want to, I won't object. > > > Tom, I am wondering. If we don't change to int4/int8 internally now, > > will we ever do it? > > As I thought I'd just made clear, I'm against standardizing on int4/int8 > at the C level. The average C programmer would think that "int8" is > a one-byte type, not an eight-byte type. There's way too much history > behind that for us to swim against the tide. Having different naming > conventions at the C and SQL levels seems a better approach, especially > since it will exist to some extent anyway (int != integer, for > instance). OK. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
There were only a few to fix, so I fixed them. > Peter Eisentraut <peter_e@gmx.net> writes: > > Which one of these should we use? > > int4 is a data type, int32 isn't. c.h has DatumGetInt8, but no > > DatumGetInt64; it also has DatumGetInt32 but no DatumGetInt4. fmgr has > > PG_GETARG_INT32 et al. Inconsistency everywhere. > > The original convention was to use int4 etc at the SQL level, int32 etc > at the C level. However the typedefs int4 etc have to be visible in > the include/catalog/pg_*.h headers, and so there's been a certain amount > of leakage of those typedefs into the C sources. > > I think that int32 etc are better choices at the C level because of > the well-established precedent for naming integer types after numbers > of bits in C code. I don't feel any strong urge to go around and > change the existing misusages, but if you want to, I won't object. > > I also have to plead guilty to having changed all the float-datatype > code to use float4 and float8 recently. This was mainly because the > existing typedefs for float32 and float64 had a built-in assumption > that these types would always be pass-by-reference, and I wanted to > abstract the code away from that assumption. We can't touch those > typedefs for a release or three (else we'll break existing user > functions written in C), so switching to the SQL-level names seemed > like the best bet. But it's not real consistent with the integer-type > naming conventions :-( > > regards, tom lane > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 ? config.log ? config.cache ? config.status ? GNUmakefile ? src/Makefile.custom ? src/GNUmakefile ? src/Makefile.global ? src/log ? src/crtags ? src/backend/postgres ? src/backend/catalog/global.bki ? src/backend/catalog/global.description ? src/backend/catalog/template1.bki ? src/backend/catalog/template1.description ? src/backend/port/Makefile ? src/bin/initdb/initdb ? src/bin/initlocation/initlocation ? src/bin/ipcclean/ipcclean ? src/bin/pg_config/pg_config ? src/bin/pg_ctl/pg_ctl ? src/bin/pg_dump/pg_dump ? src/bin/pg_dump/pg_restore ? src/bin/pg_dump/pg_dumpall ? src/bin/pg_id/pg_id ? src/bin/pg_passwd/pg_passwd ? src/bin/pgaccess/pgaccess ? src/bin/pgtclsh/Makefile.tkdefs ? src/bin/pgtclsh/Makefile.tcldefs ? src/bin/pgtclsh/pgtclsh ? src/bin/pgtclsh/pgtksh ? src/bin/psql/psql ? src/bin/scripts/createlang ? src/include/config.h ? src/include/stamp-h ? src/interfaces/ecpg/lib/libecpg.so.3.2.0 ? src/interfaces/ecpg/preproc/ecpg ? src/interfaces/libpgeasy/libpgeasy.so.2.1 ? src/interfaces/libpgtcl/libpgtcl.so.2.1 ? src/interfaces/libpq/libpq.so.2.1 ? src/interfaces/perl5/blib ? src/interfaces/perl5/Makefile ? src/interfaces/perl5/pm_to_blib ? src/interfaces/perl5/Pg.c ? src/interfaces/perl5/Pg.bs ? src/pl/plperl/blib ? src/pl/plperl/Makefile ? src/pl/plperl/pm_to_blib ? src/pl/plperl/SPI.c ? src/pl/plperl/plperl.bs ? src/pl/plpgsql/src/libplpgsql.so.1.0 ? src/pl/tcl/Makefile.tcldefs Index: src/backend/commands/command.c =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/commands/command.c,v retrieving revision 1.116 diff -c -r1.116 command.c *** src/backend/commands/command.c 2001/01/08 03:14:58 1.116 --- src/backend/commands/command.c 2001/01/23 01:45:36 *************** *** 1446,1452 **** { Relation class_rel; HeapTuple tuple; ! int4 newOwnerSysid; Relation idescs[Num_pg_class_indices]; /* --- 1446,1452 ---- { Relation class_rel; HeapTuple tuple; ! int32 newOwnerSysid; Relation idescs[Num_pg_class_indices]; /* Index: src/backend/commands/comment.c =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/commands/comment.c,v retrieving revision 1.24 diff -c -r1.24 comment.c *** src/backend/commands/comment.c 2000/11/16 22:30:18 1.24 --- src/backend/commands/comment.c 2001/01/23 01:45:36 *************** *** 394,400 **** HeapScanDesc scan; Oid oid; bool superuser; ! int4 dba; Oid userid; /*** First find the tuple in pg_database for the database ***/ --- 394,400 ---- HeapScanDesc scan; Oid oid; bool superuser; ! int32 dba; Oid userid; /*** First find the tuple in pg_database for the database ***/ Index: src/include/commands/sequence.h =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/commands/sequence.h,v retrieving revision 1.13 diff -c -r1.13 sequence.h *** src/include/commands/sequence.h 2000/12/28 13:00:28 1.13 --- src/include/commands/sequence.h 2001/01/23 01:45:47 *************** *** 15,26 **** typedef struct FormData_pg_sequence { NameData sequence_name; ! int4 last_value; ! int4 increment_by; ! int4 max_value; ! int4 min_value; ! int4 cache_value; ! int4 log_cnt; char is_cycled; char is_called; } FormData_pg_sequence; --- 15,26 ---- typedef struct FormData_pg_sequence { NameData sequence_name; ! int32 last_value; ! int32 increment_by; ! int32 max_value; ! int32 min_value; ! int32 cache_value; ! int32 log_cnt; char is_cycled; char is_called; } FormData_pg_sequence; Index: src/include/utils/date.h =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/utils/date.h,v retrieving revision 1.7 diff -c -r1.7 date.h *** src/include/utils/date.h 2000/12/03 14:51:11 1.7 --- src/include/utils/date.h 2001/01/23 01:45:48 *************** *** 25,31 **** { double time; /* all time units other than months and * years */ ! int4 zone; /* numeric time zone, in seconds */ } TimeTzADT; /* --- 25,31 ---- { double time; /* all time units other than months and * years */ ! int zone; /* numeric time zone, in seconds */ } TimeTzADT; /* Index: src/include/utils/timestamp.h =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/utils/timestamp.h,v retrieving revision 1.11 diff -c -r1.11 timestamp.h *** src/include/utils/timestamp.h 2000/11/06 16:05:25 1.11 --- src/include/utils/timestamp.h 2001/01/23 01:45:48 *************** *** 36,42 **** typedef struct { double time; /* all time units other than months and years */ ! int4 month; /* months and years, after time for alignment */ } Interval; --- 36,42 ---- typedef struct { double time; /* all time units other than months and years */ ! int month; /* months and years, after time for alignment */ } Interval;
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> There were only a few to fix, so I fixed them.
I don't think it's a good idea to write unspecified-width "int" in
the struct decls for Interval and friends. If the compiler decides
someday that that's int8, things break because the physical size of
Interval etc. is hardwired over in pg_type.h. Use "int32", or
perhaps revert these to int4.
regards, tom lane
Done. > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > There were only a few to fix, so I fixed them. > > I don't think it's a good idea to write unspecified-width "int" in > the struct decls for Interval and friends. If the compiler decides > someday that that's int8, things break because the physical size of > Interval etc. is hardwired over in pg_type.h. Use "int32", or > perhaps revert these to int4. > > regards, tom lane > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 ? config.log ? config.cache ? config.status ? GNUmakefile ? src/Makefile.custom ? src/GNUmakefile ? src/Makefile.global ? src/log ? src/crtags ? src/backend/postgres ? src/backend/catalog/global.description ? src/backend/catalog/global.bki ? src/backend/catalog/template1.bki ? src/backend/catalog/template1.description ? src/backend/port/Makefile ? src/bin/initdb/initdb ? src/bin/initlocation/initlocation ? src/bin/ipcclean/ipcclean ? src/bin/pg_config/pg_config ? src/bin/pg_ctl/pg_ctl ? src/bin/pg_dump/pg_dump ? src/bin/pg_dump/pg_restore ? src/bin/pg_dump/pg_dumpall ? src/bin/pg_id/pg_id ? src/bin/pg_passwd/pg_passwd ? src/bin/pgaccess/pgaccess ? src/bin/pgtclsh/Makefile.tkdefs ? src/bin/pgtclsh/Makefile.tcldefs ? src/bin/pgtclsh/pgtclsh ? src/bin/pgtclsh/pgtksh ? src/bin/psql/psql ? src/bin/scripts/createlang ? src/include/config.h ? src/include/stamp-h ? src/interfaces/ecpg/lib/libecpg.so.3.2.0 ? src/interfaces/ecpg/preproc/ecpg ? src/interfaces/libpgeasy/libpgeasy.so.2.1 ? src/interfaces/libpgtcl/libpgtcl.so.2.1 ? src/interfaces/libpq/libpq.so.2.1 ? src/interfaces/perl5/blib ? src/interfaces/perl5/Makefile ? src/interfaces/perl5/pm_to_blib ? src/interfaces/perl5/Pg.c ? src/interfaces/perl5/Pg.bs ? src/pl/plperl/blib ? src/pl/plperl/Makefile ? src/pl/plperl/pm_to_blib ? src/pl/plperl/SPI.c ? src/pl/plperl/plperl.bs ? src/pl/plpgsql/src/libplpgsql.so.1.0 ? src/pl/tcl/Makefile.tcldefs Index: src/include/utils/date.h =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/utils/date.h,v retrieving revision 1.8 diff -c -r1.8 date.h *** src/include/utils/date.h 2001/01/23 01:48:17 1.8 --- src/include/utils/date.h 2001/01/23 02:23:55 *************** *** 25,31 **** { double time; /* all time units other than months and * years */ ! int zone; /* numeric time zone, in seconds */ } TimeTzADT; /* --- 25,31 ---- { double time; /* all time units other than months and * years */ ! int32 zone; /* numeric time zone, in seconds */ } TimeTzADT; /* Index: src/include/utils/timestamp.h =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/utils/timestamp.h,v retrieving revision 1.12 diff -c -r1.12 timestamp.h *** src/include/utils/timestamp.h 2001/01/23 01:48:17 1.12 --- src/include/utils/timestamp.h 2001/01/23 02:23:55 *************** *** 36,42 **** typedef struct { double time; /* all time units other than months and years */ ! int month; /* months and years, after time for alignment */ } Interval; --- 36,42 ---- typedef struct { double time; /* all time units other than months and years */ ! int32 month; /* months and years, after time for alignment */ } Interval;