int4 or int32

Started by Peter Eisentrautabout 25 years ago10 messages
#1Peter Eisentraut
peter_e@gmx.net

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/

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: int4 or int32

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

#3Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#2)
Re: int4 or int32

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, Pennsylvania 19026
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#3)
Re: int4 or int32

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

#5Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#4)
Re: int4 or int32

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, Pennsylvania 19026
#6Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#2)
1 attachment(s)
Re: int4 or int32

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

Attachments:

/bjm/difftext/plainDownload
? 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;
  
  
#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#6)
Re: int4 or int32

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

#8Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#7)
1 attachment(s)
Re: int4 or int32

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

Attachments:

/bjm/difftext/plainDownload
? 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;
  
  
#9Zeugswetter Andreas SB
ZeugswetterA@wien.spardat.at
In reply to: Bruce Momjian (#8)
AW: int4 or int32

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

Wait a sec !
The patch to timestamp.h and date.h replaces int4 with int instead of int32.
At least the timestamp.h struct is on disk stuff, thus the patch is not so good :-)

Andreas

#10Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Zeugswetter Andreas SB (#9)
Re: AW: int4 or int32

[ Charset ISO-8859-1 unsupported, converting... ]

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

Wait a sec !
The patch to timestamp.h and date.h replaces int4 with int instead of int32.
At least the timestamp.h struct is on disk stuff, thus the patch is not so good :-)

Fixed to int32 now.

-- 
  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