pgjdbc/pgjdbc GitHub issues and pull requests (mirror)
help / color / mirror / Atom feed[pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
19+ messages / 3 participants
[nested] [flat]
* [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-07-03 08:08 "cfredri4 (@cfredri4)" <[email protected]>
0 siblings, 0 replies; 19+ messages in thread
From: cfredri4 (@cfredri4) @ 2025-07-03 08:08 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
This change adds support for setting a default query timeout property
Closes #3704
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-07-03 08:59 "vlsi (@vlsi)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: vlsi (@vlsi) @ 2025-07-03 08:59 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
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`.
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-07-03 09:01 "vlsi (@vlsi)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: vlsi (@vlsi) @ 2025-07-03 09:01 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Please update https://github.com/pgjdbc/pgjdbc/blob/266f0093cd7daa951656b5666454ecaba00807e9/docs/content/document... as well
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-07-03 10:17 "cfredri4 (@cfredri4)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: cfredri4 (@cfredri4) @ 2025-07-03 10:17 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
@vlsi done. But I have absolutely no idea what I'm doing with the test matrix so please review thoroughly. 😅
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-07-03 10:34 "vlsi (@vlsi)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: vlsi (@vlsi) @ 2025-07-03 10:34 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(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
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-07-03 10:35 "vlsi (@vlsi)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: vlsi (@vlsi) @ 2025-07-03 10:35 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Please refer to `testJvmArgs` in `matrix.mjs` (e.g. adaptive_fetch, rewrite_batch_inserts)
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-07-03 12:44 "vlsi (@vlsi)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: vlsi (@vlsi) @ 2025-07-03 12:44 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(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
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-07-03 12:46 "vlsi (@vlsi)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: vlsi (@vlsi) @ 2025-07-03 12:46 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
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.
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-07-03 14:09 "cfredri4 (@cfredri4)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: cfredri4 (@cfredri4) @ 2025-07-03 14:09 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
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 protected]/java.sql.DriverManager.getConnection(DriverManager.java:683)
at platform/[email protected]/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
```
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-08-07 04:58 "cfredri4 (@cfredri4)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: cfredri4 (@cfredri4) @ 2025-08-07 04:58 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
@vlsi is this ok to merge, or could you ping others for additional review?
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-08-12 07:58 "vlsi (@vlsi)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: vlsi (@vlsi) @ 2025-08-12 07:58 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
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.
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-08-12 08:09 "vlsi (@vlsi)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: vlsi (@vlsi) @ 2025-08-12 08:09 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(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
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-08-12 10:46 "davecramer (@davecramer)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: davecramer (@davecramer) @ 2025-08-12 10:46 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
where is defaultQueryTimeout actually used ?
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-08-12 13:48 "vlsi (@vlsi)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: vlsi (@vlsi) @ 2025-08-12 13:48 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
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)`
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-08-12 13:55 "vlsi (@vlsi)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: vlsi (@vlsi) @ 2025-08-12 13:55 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
@cfredri4 , @davecramer , @bokken , WDYT of `defaultQueryTimeout` (🎉) vs `queryTimeout` (🚀) naming vs whatever (👀 ) ?
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-09-09 10:53 "cfredri4 (@cfredri4)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: cfredri4 (@cfredri4) @ 2025-09-09 10:53 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Gentle reminder @vlsi @davecramer @bokken
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-09-29 08:29 "cfredri4 (@cfredri4)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: cfredri4 (@cfredri4) @ 2025-09-29 08:29 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
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. 🙁
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-10-08 05:28 "cfredri4 (@cfredri4)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: cfredri4 (@cfredri4) @ 2025-10-08 05:28 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Or maybe @jorsol @praiskup @ringerc @sehrope can give this some :eyes:?
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property
@ 2025-10-08 10:45 "davecramer (@davecramer)" <[email protected]>
17 siblings, 0 replies; 19+ messages in thread
From: davecramer (@davecramer) @ 2025-10-08 10:45 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
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
^ permalink raw reply [nested|flat] 19+ messages in thread
end of thread, other threads:[~2025-10-08 10:45 UTC | newest]
Thread overview: 19+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-07-03 08:08 [pgjdbc/pgjdbc] PR #3705: feat: default query timeout property "cfredri4 (@cfredri4)" <[email protected]>
2025-07-03 08:59 ` "vlsi (@vlsi)" <[email protected]>
2025-07-03 09:01 ` "vlsi (@vlsi)" <[email protected]>
2025-07-03 10:17 ` "cfredri4 (@cfredri4)" <[email protected]>
2025-07-03 10:34 ` "vlsi (@vlsi)" <[email protected]>
2025-07-03 10:35 ` "vlsi (@vlsi)" <[email protected]>
2025-07-03 12:44 ` "vlsi (@vlsi)" <[email protected]>
2025-07-03 12:46 ` "vlsi (@vlsi)" <[email protected]>
2025-07-03 14:09 ` "cfredri4 (@cfredri4)" <[email protected]>
2025-08-07 04:58 ` "cfredri4 (@cfredri4)" <[email protected]>
2025-08-12 07:58 ` "vlsi (@vlsi)" <[email protected]>
2025-08-12 08:09 ` "vlsi (@vlsi)" <[email protected]>
2025-08-12 10:46 ` "davecramer (@davecramer)" <[email protected]>
2025-08-12 13:48 ` "vlsi (@vlsi)" <[email protected]>
2025-08-12 13:55 ` "vlsi (@vlsi)" <[email protected]>
2025-09-09 10:53 ` "cfredri4 (@cfredri4)" <[email protected]>
2025-09-29 08:29 ` "cfredri4 (@cfredri4)" <[email protected]>
2025-10-08 05:28 ` "cfredri4 (@cfredri4)" <[email protected]>
2025-10-08 10:45 ` "davecramer (@davecramer)" <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox