On 11/6/25 12:46 PM, David Rowley wrote:
I've attached a couple of patches to get the ball rolling.
This inspired me to write my own patch to chip away at the use of long.
The target for the attached patch is TimestampDifference() and
feTimestampDifference() which return the timestamp difference in seconds
and microseconds as a long and an int and replace it with in64 and
int32. The return values of these functions are used a lot for printing
durations to the log and sometimes to populate a struct timeval (in that
case returning a timeval would make some sense but time_t is sadly 32
bits on some platforms).
The patch also cleans up code a bit in check_log_duration() which was
unnecessarily complicated.
But writing the mail made me wonder if a cleaner solution wouldn't be to
just make TimestampDifference() return a TimeOffset and then write
helper functions to e.g. convert a TimeOffset to a timeval or to
milliseconds.
Andreas
From cb2cdcaf37c33a5df9f3750fd8777d905f09ad16 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email protected]>
Date: Mon, 10 Nov 2025 08:44:35 +0100
Subject: [PATCH] Stop using long TimestampDifference()
Since long can vary between different platforms while a TimestamptTz is
always an int64 returning the difference in seconds as a long could
cause confusion and potentially bugs. Instead we return the diff as an
int64 with the number of seconds and an int32 with the number of
microseconds.
We also do exactly the same change to feTimestampDifference().
---
src/backend/access/heap/vacuumlazy.c | 4 +--
src/backend/postmaster/autovacuum.c | 4 +--
src/backend/postmaster/startup.c | 6 ++--
src/backend/replication/slot.c | 8 ++---
src/backend/storage/ipc/standby.c | 10 +++----
src/backend/storage/lmgr/proc.c | 16 +++++-----
src/backend/tcop/postgres.c | 31 ++++++++------------
src/backend/utils/activity/backend_status.c | 4 +--
src/backend/utils/activity/pgstat_database.c | 4 +--
src/backend/utils/adt/timestamp.c | 10 +++----
src/backend/utils/misc/timeout.c | 4 +--
src/bin/pg_basebackup/pg_recvlogical.c | 4 +--
src/bin/pg_basebackup/receivelog.c | 22 +++++++-------
src/bin/pg_basebackup/streamutil.c | 6 ++--
src/bin/pg_basebackup/streamutil.h | 2 +-
src/include/postmaster/startup.h | 4 +--
src/include/utils/timestamp.h | 2 +-
17 files changed, 67 insertions(+), 74 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index deb9a3dc0d1..7d9b6551623 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -963,8 +963,8 @@ heap_vacuum_rel(Relation rel, const VacuumParams params,
TimestampDifferenceExceeds(starttime, endtime,
params.log_vacuum_min_duration))
{
- long secs_dur;
- int usecs_dur;
+ int64 secs_dur;
+ int32 usecs_dur;
WalUsage walusage;
BufferUsage bufferusage;
StringInfoData buf;
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index ed19c74bb19..b6d95a3b5c1 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -834,8 +834,8 @@ launcher_determine_sleep(bool canlaunch, bool recursing, struct timeval *nap)
TimestampTz current_time = GetCurrentTimestamp();
TimestampTz next_wakeup;
avl_dbase *avdb;
- long secs;
- int usecs;
+ int64 secs;
+ int32 usecs;
avdb = dlist_tail_element(avl_dbase, adl_node, &DatabaseList);
diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c
index 27e86cf393f..0068638f380 100644
--- a/src/backend/postmaster/startup.c
+++ b/src/backend/postmaster/startup.c
@@ -356,10 +356,10 @@ begin_startup_progress_phase(void)
* otherwise return false.
*/
bool
-has_startup_progress_timeout_expired(long *secs, int *usecs)
+has_startup_progress_timeout_expired(int64 *secs, int32 *usecs)
{
- long seconds;
- int useconds;
+ int64 seconds;
+ int32 useconds;
TimestampTz now;
/* No timeout has occurred. */
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 1ec1e997b27..aff65b8f729 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1652,7 +1652,7 @@ ReportSlotInvalidation(ReplicationSlotInvalidationCause cause,
XLogRecPtr restart_lsn,
XLogRecPtr oldestLSN,
TransactionId snapshotConflictHorizon,
- long slot_idle_seconds)
+ int64 slot_idle_seconds)
{
StringInfoData err_detail;
StringInfoData err_hint;
@@ -1689,7 +1689,7 @@ ReportSlotInvalidation(ReplicationSlotInvalidationCause cause,
case RS_INVAL_IDLE_TIMEOUT:
{
/* translator: %s is a GUC variable name */
- appendStringInfo(&err_detail, _("The slot's idle time of %lds exceeds the configured \"%s\" duration of %ds."),
+ appendStringInfo(&err_detail, _("The slot's idle time of %" PRId64 "s exceeds the configured \"%s\" duration of %ds."),
slot_idle_seconds, "idle_replication_slot_timeout",
idle_replication_slot_timeout_secs);
/* translator: %s is a GUC variable name */
@@ -1851,7 +1851,7 @@ InvalidatePossiblyObsoleteSlot(uint32 possible_causes,
int active_pid = 0;
ReplicationSlotInvalidationCause invalidation_cause = RS_INVAL_NONE;
TimestampTz now = 0;
- long slot_idle_secs = 0;
+ int64 slot_idle_secs = 0;
Assert(LWLockHeldByMeInMode(ReplicationSlotControlLock, LW_SHARED));
@@ -1941,7 +1941,7 @@ InvalidatePossiblyObsoleteSlot(uint32 possible_causes,
*/
if (invalidation_cause == RS_INVAL_IDLE_TIMEOUT)
{
- int slot_idle_usecs;
+ int32 slot_idle_usecs;
TimestampDifference(inactive_since, now, &slot_idle_secs,
&slot_idle_usecs);
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 4222bdab078..a533c096a99 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -275,9 +275,9 @@ LogRecoveryConflict(ProcSignalReason reason, TimestampTz wait_start,
TimestampTz now, VirtualTransactionId *wait_list,
bool still_waiting)
{
- long secs;
- int usecs;
- long msecs;
+ int64 secs;
+ int32 usecs;
+ int64 msecs;
StringInfoData buf;
int nprocs = 0;
@@ -327,7 +327,7 @@ LogRecoveryConflict(ProcSignalReason reason, TimestampTz wait_start,
if (still_waiting)
{
ereport(LOG,
- errmsg("recovery still waiting after %ld.%03d ms: %s",
+ errmsg("recovery still waiting after %" PRId64 ".%03" PRId32 " ms: %s",
msecs, usecs, get_recovery_conflict_desc(reason)),
nprocs > 0 ? errdetail_log_plural("Conflicting process: %s.",
"Conflicting processes: %s.",
@@ -336,7 +336,7 @@ LogRecoveryConflict(ProcSignalReason reason, TimestampTz wait_start,
else
{
ereport(LOG,
- errmsg("recovery finished waiting after %ld.%03d ms: %s",
+ errmsg("recovery finished waiting after %" PRId64 ".%03" PRId32 " ms: %s",
msecs, usecs, get_recovery_conflict_desc(reason)));
}
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 1504fafe6d8..f6d1771499f 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -1572,9 +1572,9 @@ ProcSleep(LOCALLOCK *locallock)
lock_waiters_sbuf,
lock_holders_sbuf;
const char *modename;
- long secs;
- int usecs;
- long msecs;
+ int64 secs;
+ int32 usecs;
+ int64 msecs;
int lockHoldersNum = 0;
initStringInfo(&buf);
@@ -1598,7 +1598,7 @@ ProcSleep(LOCALLOCK *locallock)
if (deadlock_state == DS_SOFT_DEADLOCK)
ereport(LOG,
- (errmsg("process %d avoided deadlock for %s on %s by rearranging queue order after %ld.%03d ms",
+ (errmsg("process %d avoided deadlock for %s on %s by rearranging queue order after %" PRId64 ".%03" PRId32 " ms",
MyProcPid, modename, buf.data, msecs, usecs),
(errdetail_log_plural("Process holding the lock: %s. Wait queue: %s.",
"Processes holding the lock: %s. Wait queue: %s.",
@@ -1613,7 +1613,7 @@ ProcSleep(LOCALLOCK *locallock)
* events get logged.
*/
ereport(LOG,
- (errmsg("process %d detected deadlock while waiting for %s on %s after %ld.%03d ms",
+ (errmsg("process %d detected deadlock while waiting for %s on %s after %" PRId64 ".%03" PRId32 " ms",
MyProcPid, modename, buf.data, msecs, usecs),
(errdetail_log_plural("Process holding the lock: %s. Wait queue: %s.",
"Processes holding the lock: %s. Wait queue: %s.",
@@ -1622,14 +1622,14 @@ ProcSleep(LOCALLOCK *locallock)
if (myWaitStatus == PROC_WAIT_STATUS_WAITING)
ereport(LOG,
- (errmsg("process %d still waiting for %s on %s after %ld.%03d ms",
+ (errmsg("process %d still waiting for %s on %s after %" PRId64 ".%03" PRId32 " ms",
MyProcPid, modename, buf.data, msecs, usecs),
(errdetail_log_plural("Process holding the lock: %s. Wait queue: %s.",
"Processes holding the lock: %s. Wait queue: %s.",
lockHoldersNum, lock_holders_sbuf.data, lock_waiters_sbuf.data))));
else if (myWaitStatus == PROC_WAIT_STATUS_OK)
ereport(LOG,
- (errmsg("process %d acquired %s on %s after %ld.%03d ms",
+ (errmsg("process %d acquired %s on %s after %" PRId64 ".%03" PRId32 " ms",
MyProcPid, modename, buf.data, msecs, usecs)));
else
{
@@ -1646,7 +1646,7 @@ ProcSleep(LOCALLOCK *locallock)
*/
if (deadlock_state != DS_HARD_DEADLOCK)
ereport(LOG,
- (errmsg("process %d failed to acquire %s on %s after %ld.%03d ms",
+ (errmsg("process %d failed to acquire %s on %s after %" PRId64 ".%03" PRId32 " ms",
MyProcPid, modename, buf.data, msecs, usecs),
(errdetail_log_plural("Process holding the lock: %s. Wait queue: %s.",
"Processes holding the lock: %s. Wait queue: %s.",
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 2bd89102686..d67e0f15ae3 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2441,9 +2441,9 @@ check_log_duration(char *msec_str, bool was_logged)
if (log_duration || log_min_duration_sample >= 0 ||
log_min_duration_statement >= 0 || xact_is_sampled)
{
- long secs;
- int usecs;
- int msecs;
+ int64 secs;
+ int32 usecs;
+ int64 msecs;
bool exceeded_duration;
bool exceeded_sample_duration;
bool in_sample = false;
@@ -2451,22 +2451,16 @@ check_log_duration(char *msec_str, bool was_logged)
TimestampDifference(GetCurrentStatementStartTimestamp(),
GetCurrentTimestamp(),
&secs, &usecs);
- msecs = usecs / 1000;
+ msecs = secs * 1000 + usecs / 1000;
+ usecs = usecs % 1000;
- /*
- * This odd-looking test for log_min_duration_* being exceeded is
- * designed to avoid integer overflow with very long durations: don't
- * compute secs * 1000 until we've verified it will fit in int.
- */
exceeded_duration = (log_min_duration_statement == 0 ||
(log_min_duration_statement > 0 &&
- (secs > log_min_duration_statement / 1000 ||
- secs * 1000 + msecs >= log_min_duration_statement)));
+ msecs >= log_min_duration_statement));
exceeded_sample_duration = (log_min_duration_sample == 0 ||
(log_min_duration_sample > 0 &&
- (secs > log_min_duration_sample / 1000 ||
- secs * 1000 + msecs >= log_min_duration_sample)));
+ msecs >= log_min_duration_sample));
/*
* Do not log if log_statement_sample_rate = 0. Log a sample if
@@ -2480,8 +2474,7 @@ check_log_duration(char *msec_str, bool was_logged)
if (exceeded_duration || in_sample || log_duration || xact_is_sampled)
{
- snprintf(msec_str, 32, "%ld.%03d",
- secs * 1000 + msecs, usecs % 1000);
+ snprintf(msec_str, 32, "%" PRId64 ".%03" PRId32 , msecs, usecs);
if ((exceeded_duration || in_sample || xact_is_sampled) && !was_logged)
return 2;
else
@@ -5187,12 +5180,12 @@ static void
log_disconnections(int code, Datum arg)
{
Port *port = MyProcPort;
- long secs;
- int usecs;
- int msecs;
+ int64 secs;
+ int32 usecs;
int hours,
minutes,
- seconds;
+ seconds,
+ msecs;
TimestampDifference(MyStartTimestamp,
GetCurrentTimestamp(),
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index a290cc4c975..2308cfa5b78 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -633,8 +633,8 @@ pgstat_report_activity(BackendState state, const char *cmd_str)
beentry->st_state == STATE_IDLEINTRANSACTION_ABORTED) &&
state != beentry->st_state)
{
- long secs;
- int usecs;
+ int64 secs;
+ int32 usecs;
TimestampDifference(beentry->st_state_start_timestamp,
current_timestamp,
diff --git a/src/backend/utils/activity/pgstat_database.c b/src/backend/utils/activity/pgstat_database.c
index b31f20d41bc..5a643c366ed 100644
--- a/src/backend/utils/activity/pgstat_database.c
+++ b/src/backend/utils/activity/pgstat_database.c
@@ -344,8 +344,8 @@ pgstat_update_dbstats(TimestampTz ts)
if (pgstat_should_report_connstat())
{
- long secs;
- int usecs;
+ int64 secs;
+ int32 usecs;
/*
* pgLastSessionReportTime is initialized to MyStartTimestamp by
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 156a4830ffd..eb63f54e095 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -1719,7 +1719,7 @@ timeofday(PG_FUNCTION_ARGS)
*/
void
TimestampDifference(TimestampTz start_time, TimestampTz stop_time,
- long *secs, int *microsecs)
+ int64 *secs, int32 *microsecs)
{
TimestampTz diff = stop_time - start_time;
@@ -1730,8 +1730,8 @@ TimestampDifference(TimestampTz start_time, TimestampTz stop_time,
}
else
{
- *secs = (long) (diff / USECS_PER_SEC);
- *microsecs = (int) (diff % USECS_PER_SEC);
+ *secs = diff / USECS_PER_SEC;
+ *microsecs = (int32) (diff % USECS_PER_SEC);
}
}
@@ -1796,8 +1796,8 @@ TimestampDifferenceExceedsSeconds(TimestampTz start_time,
TimestampTz stop_time,
int threshold_sec)
{
- long secs;
- int usecs;
+ int64 secs;
+ int32 usecs;
/* Calculate the difference in seconds */
TimestampDifference(start_time, stop_time, &secs, &usecs);
diff --git a/src/backend/utils/misc/timeout.c b/src/backend/utils/misc/timeout.c
index d92efa12550..aad7d23a1fa 100644
--- a/src/backend/utils/misc/timeout.c
+++ b/src/backend/utils/misc/timeout.c
@@ -213,8 +213,8 @@ schedule_alarm(TimestampTz now)
{
struct itimerval timeval;
TimestampTz nearest_timeout;
- long secs;
- int usecs;
+ int64 secs;
+ int32 usecs;
MemSet(&timeval, 0, sizeof(struct itimerval));
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 14ad1504678..1081f9c35ce 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -396,8 +396,8 @@ StreamLogicalLog(void)
if (message_target > 0 || fsync_target > 0)
{
TimestampTz targettime;
- long secs;
- int usecs;
+ int64 secs;
+ int32 usecs;
targettime = message_target;
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 25b13c7f55c..129bf675297 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -34,8 +34,8 @@ static bool still_sending = true; /* feedback still needs to be sent? */
static PGresult *HandleCopyStream(PGconn *conn, StreamCtl *stream,
XLogRecPtr *stoppos);
-static int CopyStreamPoll(PGconn *conn, long timeout_ms, pgsocket stop_socket);
-static int CopyStreamReceive(PGconn *conn, long timeout, pgsocket stop_socket,
+static int CopyStreamPoll(PGconn *conn, int64 timeout_ms, pgsocket stop_socket);
+static int CopyStreamReceive(PGconn *conn, int64 timeout, pgsocket stop_socket,
char **buffer);
static bool ProcessKeepaliveMsg(PGconn *conn, StreamCtl *stream, char *copybuf,
int len, XLogRecPtr blockpos, TimestampTz *last_status);
@@ -44,7 +44,7 @@ static bool ProcessWALDataMsg(PGconn *conn, StreamCtl *stream, char *copybuf, in
static PGresult *HandleEndOfCopyStream(PGconn *conn, StreamCtl *stream, char *copybuf,
XLogRecPtr blockpos, XLogRecPtr *stoppos);
static bool CheckCopyStreamStop(PGconn *conn, StreamCtl *stream, XLogRecPtr blockpos);
-static long CalculateCopyStreamSleeptime(TimestampTz now, int standby_message_timeout,
+static int64 CalculateCopyStreamSleeptime(TimestampTz now, int standby_message_timeout,
TimestampTz last_status);
static bool ReadEndOfStreamingResult(PGresult *res, XLogRecPtr *startpos,
@@ -755,7 +755,7 @@ HandleCopyStream(PGconn *conn, StreamCtl *stream,
{
int r;
TimestampTz now;
- long sleeptime;
+ int64 sleeptime;
/*
* Check if we should continue streaming, or abort at this point.
@@ -875,7 +875,7 @@ error:
* or interrupted by signal or stop_socket input, and -1 on an error.
*/
static int
-CopyStreamPoll(PGconn *conn, long timeout_ms, pgsocket stop_socket)
+CopyStreamPoll(PGconn *conn, int64 timeout_ms, pgsocket stop_socket)
{
int ret;
fd_set input_mask;
@@ -904,8 +904,8 @@ CopyStreamPoll(PGconn *conn, long timeout_ms, pgsocket stop_socket)
timeoutptr = NULL;
else
{
- timeout.tv_sec = timeout_ms / 1000L;
- timeout.tv_usec = (timeout_ms % 1000L) * 1000L;
+ timeout.tv_sec = timeout_ms / INT64CONST(1000);
+ timeout.tv_usec = (timeout_ms % INT64CONST(1000)) * INT64CONST(1000);
timeoutptr = &timeout;
}
@@ -937,7 +937,7 @@ CopyStreamPoll(PGconn *conn, long timeout_ms, pgsocket stop_socket)
* -1 on error. -2 if the server ended the COPY.
*/
static int
-CopyStreamReceive(PGconn *conn, long timeout, pgsocket stop_socket,
+CopyStreamReceive(PGconn *conn, int64 timeout, pgsocket stop_socket,
char **buffer)
{
char *copybuf = NULL;
@@ -1239,7 +1239,7 @@ CheckCopyStreamStop(PGconn *conn, StreamCtl *stream, XLogRecPtr blockpos)
/*
* Calculate how long send/receive loops should sleep
*/
-static long
+static int64
CalculateCopyStreamSleeptime(TimestampTz now, int standby_message_timeout,
TimestampTz last_status)
{
@@ -1252,8 +1252,8 @@ CalculateCopyStreamSleeptime(TimestampTz now, int standby_message_timeout,
if (status_targettime > 0)
{
- long secs;
- int usecs;
+ int64 secs;
+ int32 usecs;
feTimestampDifference(now,
status_targettime,
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index e5a7cb6e5b1..2057a81cacd 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -820,7 +820,7 @@ feGetCurrentTimestamp(void)
*/
void
feTimestampDifference(TimestampTz start_time, TimestampTz stop_time,
- long *secs, int *microsecs)
+ int64 *secs, int32 *microsecs)
{
TimestampTz diff = stop_time - start_time;
@@ -831,8 +831,8 @@ feTimestampDifference(TimestampTz start_time, TimestampTz stop_time,
}
else
{
- *secs = (long) (diff / USECS_PER_SEC);
- *microsecs = (int) (diff % USECS_PER_SEC);
+ *secs = diff / USECS_PER_SEC;
+ *microsecs = (int32) (diff % USECS_PER_SEC);
}
}
diff --git a/src/bin/pg_basebackup/streamutil.h b/src/bin/pg_basebackup/streamutil.h
index 017b227303c..aaeb29392aa 100644
--- a/src/bin/pg_basebackup/streamutil.h
+++ b/src/bin/pg_basebackup/streamutil.h
@@ -59,7 +59,7 @@ extern bool GetSlotInformation(PGconn *conn, const char *slot_name,
extern bool RetrieveWalSegSize(PGconn *conn);
extern TimestampTz feGetCurrentTimestamp(void);
extern void feTimestampDifference(TimestampTz start_time, TimestampTz stop_time,
- long *secs, int *microsecs);
+ int64 *secs, int32 *microsecs);
extern bool feTimestampDifferenceExceeds(TimestampTz start_time, TimestampTz stop_time,
int msec);
diff --git a/src/include/postmaster/startup.h b/src/include/postmaster/startup.h
index ec2c8d3bff5..d519cf38ab2 100644
--- a/src/include/postmaster/startup.h
+++ b/src/include/postmaster/startup.h
@@ -17,8 +17,8 @@
*/
#define ereport_startup_progress(msg, ...) \
do { \
- long secs; \
- int usecs; \
+ int64 secs; \
+ int32 usecs; \
if (has_startup_progress_timeout_expired(&secs, &usecs)) \
ereport(LOG, errmsg(msg, secs, (usecs / 10000), __VA_ARGS__ )); \
} while(0)
diff --git a/src/include/utils/timestamp.h b/src/include/utils/timestamp.h
index 93531732b08..37f4fe22d1e 100644
--- a/src/include/utils/timestamp.h
+++ b/src/include/utils/timestamp.h
@@ -110,7 +110,7 @@ extern TimestampTz GetCurrentTimestamp(void);
extern TimestampTz GetSQLCurrentTimestamp(int32 typmod);
extern Timestamp GetSQLLocalTimestamp(int32 typmod);
extern void TimestampDifference(TimestampTz start_time, TimestampTz stop_time,
- long *secs, int *microsecs);
+ int64 *secs, int32 *microsecs);
extern long TimestampDifferenceMilliseconds(TimestampTz start_time,
TimestampTz stop_time);
extern bool TimestampDifferenceExceeds(TimestampTz start_time,
--
2.47.3
