Subject: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property

From: "cfredri4 (@cfredri4)" <[email-masked]>
Date: Thu, 03 Jul 2025 08:08:32 +0000

This change adds support for setting a default query timeout property

Closes #3704

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 03 Jul 2025 08:59:49 +0000

Could you please add `defaultQueryTimeout` to the test matrix ax well? See https://github.com/pgjdbc/pgjdbc/pull/3632.
For example, keep the default execution (`defaultQueryTimeout` unset) and add an execution with `defaultQueryTimeout=15`.

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 03 Jul 2025 09:01:07 +0000

Please update https://github.com/pgjdbc/pgjdbc/blob/266f0093cd7daa951656b5666454ecaba00807e9/docs/content/documentation/use.md#connection-parameters as well

---

From: "cfredri4 (@cfredri4)" <[email-masked]>
Date: Thu, 03 Jul 2025 10:17:35 +0000

@vlsi done. But I have absolutely no idea what I'm doing with the test matrix so please review thoroughly. 😅

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 03 Jul 2025 10:34:16 +0000

(on docker/postgres-server/scripts/entrypoint.sh)

well, query timeout is a client option, not the server option, so no need to modify entrypoint.sh

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 03 Jul 2025 10:35:03 +0000

Please refer to `testJvmArgs` in `matrix.mjs` (e.g. adaptive_fetch, rewrite_batch_inserts)

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 03 Jul 2025 12:44:11 +0000

(on .github/workflows/matrix.mjs)

A small note: please add `title: x => x == '' ? '' : 'default_query_timeout ' + x,` so the ci job title includes default_query_timeout. Otherwise it is hard to tell what does 15 mean

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 03 Jul 2025 12:46:30 +0000

The change looks good to me. As it adds a new parameter (thus it impacts the documentation), I would be happy for the others to review the change as well.

---

From: "cfredri4 (@cfredri4)" <[email-masked]>
Date: Thu, 03 Jul 2025 14:09:08 +0000

There is a failing test but it seems unrelated?

```
SslTest STANDARD_OUT
    Configuration file C:\projects\pgjdbc\pgjdbc\..\ssltest.local.properties does not exist. Consider adding it to specify test db host and login
FAILURE   0.0sec, org.postgresql.test.ssl.SslTest > initializationError
    org.postgresql.util.PSQLException: The connection attempt failed.
        at app//org.postgresql.core.v3.ConnectionFactoryImpl.openConnectionImpl(ConnectionFactoryImpl.java:385)
        at app//org.postgresql.core.ConnectionFactory.openConnection(ConnectionFactory.java:57)
        at app//org.postgresql.jdbc.PgConnection.<init>(PgConnection.java:286)
        at app//org.postgresql.Driver.makeConnection(Driver.java:448)
        at app//org.postgresql.Driver.connect(Driver.java:298)
        at platform/[email-masked]/java.sql.DriverManager.getConnection(DriverManager.java:683)
        at platform/[email-masked]/java.sql.DriverManager.getConnection(DriverManager.java:191)
        at app//org.postgresql.test.TestUtil.openDB(TestUtil.java:358)
        at app//org.postgresql.test.TestUtil.openPrivilegedDB(TestUtil.java:327)
        at app//org.postgresql.test.TestUtil.openPrivilegedDB(TestUtil.java:318)
        at app//org.postgresql.test.ssl.SslTest.dropRoles(SslTest.java:288)
        Suppressed: org.opentest4j.TestAbortedException: Assumption failed: assumption is not true
            at app//org.postgresql.test.TestUtil.assumeSslTestsEnabled(TestUtil.java:266)
            at app//org.postgresql.test.ssl.SslTest.createRoles(SslTest.java:268)
        Suppressed: org.opentest4j.TestAbortedException: Assumption failed: assumption is not true
            at app//org.postgresql.test.TestUtil.assumeSslTestsEnabled(TestUtil.java:266)
            at app//org.postgresql.test.ssl.SslTest.createRoles(SslTest.java:268)
        Caused by: java.net.SocketTimeoutException: Read timed out
            at java.base/sun.nio.ch.NioSocketImpl.timedRead(NioSocketImpl.java:278)
            at java.base/sun.nio.ch.NioSocketImpl.implRead(NioSocketImpl.java:304)
            at java.base/sun.nio.ch.NioSocketImpl.read(NioSocketImpl.java:346)
            at java.base/sun.nio.ch.NioSocketImpl$1.read(NioSocketImpl.java:796)
            at java.base/java.net.Socket$SocketInputStream.read(Socket.java:1099)
            at org.postgresql.core.VisibleBufferedInputStream.readMore(VisibleBufferedInputStream.java:192)
            at org.postgresql.core.VisibleBufferedInputStream.ensureBytes(VisibleBufferedInputStream.java:159)
            at org.postgresql.core.VisibleBufferedInputStream.ensureBytes(VisibleBufferedInputStream.java:144)
            at org.postgresql.core.VisibleBufferedInputStream.read(VisibleBufferedInputStream.java:76)
            at org.postgresql.core.PGStream.receiveChar(PGStream.java:477)
            at org.postgresql.core.v3.ConnectionFactoryImpl.enableSSL(ConnectionFactoryImpl.java:627)
            at org.postgresql.core.v3.ConnectionFactoryImpl.tryConnect(ConnectionFactoryImpl.java:207)
            at org.postgresql.core.v3.ConnectionFactoryImpl.openConnectionImpl(ConnectionFactoryImpl.java:289)
            ... 10 more
```

---

From: "cfredri4 (@cfredri4)" <[email-masked]>
Date: Thu, 07 Aug 2025 04:58:36 +0000

@vlsi is this ok to merge, or could you ping others for additional review?

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Tue, 12 Aug 2025 07:58:33 +0000

I've made a second review pass, and I wondered whether we should support millisecond resolution for the default query timeout or second resolution.

JDBC supports second-resolution only via `java.sql.Statement#setQueryTimeout(int seconds)`.
At the same time, `java.sql.Connection#setNetworkTimeout(Executor, int millis)` uses millis.

pgjdbc has `org.postgresql.jdbc.PgStatement#setQueryTimeoutMs(long millis)` which uses millis.

However, the existing `loginTimeout`, `connectTimeout`, `socketTimeout`, and `cancelSignalTimeout` connection properties use seconds. So it sounds fine to use seconds unit for the newly added `defaultQueryTimeout` property.

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Tue, 12 Aug 2025 08:09:40 +0000

(on docs/content/documentation/use.md)

We'd probably rephrase it so it mentions `Statement#setQueryTimeout` so the users don't confuse it with PostgreSQL queries. We should probably clarify the value of `0` means going with no limit

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Tue, 12 Aug 2025 10:46:58 +0000

where is defaultQueryTimeout actually used ?

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Tue, 12 Aug 2025 13:48:11 +0000

It is a new connection property so the users could configure "global query timeout" for their connection pools. It is somewhat related to `socketTimeout`.

At the same time, when it comes to naming, it looks like we don't use `default...` prefix often (the only property is `defaultRowFetchSize`).
It might be a good idea to go for `queryTimeout` naming, so it becomes the same property at different levels:
* connection pool level via "`queryTimeout` connection property"
* `java.sql.Connection` level via `PGConnection#setQueryTimeout(int)` (I'm not sure it exists now)
* `java.sql.Statement` level via `Statement#setQueryTimeout(int)`

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Tue, 12 Aug 2025 13:55:05 +0000

@cfredri4 , @davecramer , @bokken , WDYT of `defaultQueryTimeout` (🎉) vs `queryTimeout` (🚀) naming vs whatever (👀 ) ?

---

From: "cfredri4 (@cfredri4)" <[email-masked]>
Date: Tue, 09 Sep 2025 10:53:46 +0000

Gentle reminder @vlsi @davecramer @bokken

---

From: "cfredri4 (@cfredri4)" <[email-masked]>
Date: Mon, 29 Sep 2025 08:29:28 +0000

I changed to `queryTimeout` instead of `defaultQueryTimeout` as suggested above.

@vlsi @davecramer @bokken Can this please be merged now? There was a pgjdbc release last week and I'm sad this was not included; it has been pending feedback or merging for almost 4 months now. 🙁

---

From: "cfredri4 (@cfredri4)" <[email-masked]>
Date: Wed, 08 Oct 2025 05:28:31 +0000

Or maybe @jorsol @praiskup @ringerc @sehrope can give this some :eyes:?

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Wed, 08 Oct 2025 10:45:05 +0000

LGTM. I'll merge this if nobody objects? @vlsi ?

I am rather curious why we aren't using postgres statement timeout, but I'm not about to change it now