pgjdbc/pgjdbc GitHub issues and pull requests (mirror)
help / color / mirror / Atom feed[pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
18+ messages / 5 participants
[nested] [flat]
* [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-06-02 15:29 "vlsi (@vlsi)" <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: vlsi (@vlsi) @ 2023-06-02 15:29 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
**Describe the issue**
Currently `defaultRowFetchSize` is `0`, so it fetches all the rows in-memory which might cause `OutOfMemoryConditions`
I suggest using a finite default to reduce the number of application failures.
We should align the change with adaptive fetch.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-06-02 15:34 ` "davecramer (@davecramer)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: davecramer (@davecramer) @ 2023-06-02 15:34 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
+1 from me
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-12 08:16 ` "zhurs (@zhurs)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: zhurs (@zhurs) @ 2023-07-12 08:16 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
It seems that it is an incompatible change - for insert..returning statements, if the size of the result is more than fetchSize method executeUpdate returns -1, here is a small example:
```java
conn.prepareStatement("DROP TABLE IF EXISTS test_table").execute();
conn.prepareStatement("CREATE TABLE IF NOT EXISTS test_table (id SERIAL PRIMARY KEY, cnt INT NOT NULL)").execute();
for (var fetchSize : List.of(0, 1, 2, 3)) {
System.out.println("FetchSize=" + fetchSize);
try (var stmt = conn.prepareStatement("INSERT INTO test_table (cnt) VALUES (1), (2) RETURNING id", RETURN_GENERATED_KEYS)) {
stmt.setFetchSize(fetchSize);
var ret = stmt.executeUpdate();
System.out.println("executeUpdate result: " + ret);
var rs = stmt.getGeneratedKeys();
System.out.print("ids: ");
while (rs.next()) {
System.out.print(rs.getInt(1) + " ");
}
System.out.print("\n\n");
}
}
```
Output:
```
FetchSize=0
executeUpdate result: 2
ids: 1 2
FetchSize=1
executeUpdate result: -1
ids: 3 4
FetchSize=2
executeUpdate result: -1
ids: 5 6
FetchSize=3
executeUpdate result: 2
ids: 7 8
```
Also, -1 is not documented and contradicts with jdbc specification:
> executeUpdate Returns:
> either (1) the row count for SQL Data Manipulation Language (DML) statements or (2) 0 for SQL statements that return nothing
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-12 12:20 ` "davecramer (@davecramer)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: davecramer (@davecramer) @ 2023-07-12 12:20 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
looking at this, thanks for the repro
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-12 17:25 ` "lure (@lure)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: lure (@lure) @ 2023-07-12 17:25 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
`conn.setAutoCommit(false)` is required for the code above to be reproducible
in 68ba613810784b375ab855cf1fa639bc5b254ecd/postgresql-42.6.0-sources.jar!/org/postgresql/jdbc/PgStatement.java:509
```
if (wantsGeneratedKeysOnce || wantsGeneratedKeysAlways) {
generatedKeys = currentResult;
result = castNonNull(currentResult, "handler.getResults()").getNext();
```
The currentResult returns null for `getNext()`, hovewer, the `currentResult.getResultSet().next()` returns `true` and the generated values are available.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-12 17:30 ` "davecramer (@davecramer)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: davecramer (@davecramer) @ 2023-07-12 17:30 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
So the problem is that if you set the fetch size to a non-zero value we use a cursor.
executeUpdate executes the statement but returns a cursor. While the backend knows how many rows have been updated, that information is not conveyed. Seems like a bug in the server.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-12 17:36 ` "lure (@lure)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: lure (@lure) @ 2023-07-12 17:36 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
```
case 'C': { // Command Status (end of Execute)
```
Looks like this is what not happening if autocommit is set to false. The rowCount is obtained from the end of the execution, which seems does not happen until the transaction is committed
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-12 17:37 ` "lure (@lure)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: lure (@lure) @ 2023-07-12 17:37 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
@davecramer ~~not sure I see a cursor there...~~
May be, I am not sure, as I skipped the that moment during the debug, I would appreciate if you point out where exactly it happens for the comprehensiveness of my knowledge.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-12 17:48 ` "davecramer (@davecramer)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: davecramer (@davecramer) @ 2023-07-12 17:48 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
https://github.com/pgjdbc/pgjdbc/blob/258cff34e32f8b5fe5d71f7553f7ac5f30690f85/pgjdbc/src/main/java/... is where it decides to use a CURSOR which in reality is a portal
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-12 19:03 ` "lure (@lure)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: lure (@lure) @ 2023-07-12 19:03 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
> that information is not conveyed. Seems like a bug in the server.
After some consideration and reading, it might not be a bug. The intermediate operations "see" only the rows passed to them. the generatedKeys result set holds the reference to the cursor, which allows it to read remaining records from the server. Initial set contains just the FetchSize number of rows.
So, unless there are changes to /capabilities of PG protocol that could be used to request the affected row count after the generated keys are parsed, we are probably stuck
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-12 19:53 ` "davecramer (@davecramer)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: davecramer (@davecramer) @ 2023-07-12 19:53 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Yeah, unfortunately it seems to be implemented that way. One could argue that the insert is completed however before the cursor is read. I'll take a stab at this on -hackers, but I'm likely not going to get anywhere.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-13 10:41 ` "davecramer (@davecramer)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: davecramer (@davecramer) @ 2023-07-13 10:41 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Also, -1 is not documented and contradicts with jdbc specification:
Well if we return 0 that isn't correct and we can't return the correct amount so what would you suggest ?
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-13 11:01 ` "vlsi (@vlsi)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: vlsi (@vlsi) @ 2023-07-13 11:01 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
I think `-1` for `executeLargeUpdate` and `executeUpdate` is a bug. For DMLs we should have server-side information on "the number of modified rows", and we should just return it in `executeUpdate`.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-13 11:09 ` "davecramer (@davecramer)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: davecramer (@davecramer) @ 2023-07-13 11:09 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
> I think `-1` for `executeLargeUpdate` and `executeUpdate` is a bug. For DMLs we should have server-side information on "the number of modified rows", and we should just return it in `executeUpdate`.
Well we don't get server side information if fetchsize is set to a non zero positive number and we are returning a result set. It has been said that if you are returning a resultset then executeUpdate is the wrong function to call. Either way there's little the driver can do.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-13 11:29 ` "zhurs (@zhurs)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: zhurs (@zhurs) @ 2023-07-13 11:29 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Dave, thanks a lot for your investigation!
The question what to do is quite complicated, but I'll try to voice some thoughts:
- specially for `INSERT` we can force fetchSize=0 at statement level - all rows to insert we already have in memory, so we can read all results at once
- we can make api more consistent - return always -1 for such cases `executeUpdate` with returning and add this to docs, but it will break a lot of code and third-party libraries, such as Exposed
- as more light version of previous one we can write about such situation to log and suggest to use `.execute`
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2023-07-13 11:33 ` "davecramer (@davecramer)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: davecramer (@davecramer) @ 2023-07-13 11:33 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
>
> The question what to do is quite complicated, but I'll try to voice some thoughts:
>
> * specially for `INSERT` we can force fetchSize=0 at statement level - all rows to insert we already have in memory, so we can read all results at once
Not sure I'm a fan of this. You eventually find out how many rows it's just not from executeUpdate
> * we can make api more consistent - return always -1 for such cases `executeUpdate` with returning and add this to docs, but it will break a lot of code and third-party libraries, such as Exposed
Well if fetchSize is 0 it works properly
> * as more light version of previous one we can write about such situation to log and suggest to use `.execute`
Documenting the behaviour is my vote.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2025-09-04 12:19 ` "freak2fast4u (@freak2fast4u)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: freak2fast4u (@freak2fast4u) @ 2025-09-04 12:19 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
I'd rather have a "safer" value in the ballpark of 128, and if that's too low for you, 1000 max, but nowhere above that. Currently pgJDBC eats RAM like it's unlimited. I would like that just as much as you do, but it isn't ;)
Also, why not honor the fetchSize on every SELECT statement regardless of auto-commit ?
Auto-commit should only affect writing statements (INSERT/UPDATE/DELETE), not read (SELECT) statements, right ? Why is auto-commit even relevant to honoring the fetchSize in the first place ? I'm confused.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability
@ 2025-09-04 15:03 ` "davecramer (@davecramer)" <[email protected]>
16 siblings, 0 replies; 18+ messages in thread
From: davecramer (@davecramer) @ 2025-09-04 15:03 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
> I'd rather have a "safer" value in the ballpark of 128, and if that's too low for you, 1000 max, but nowhere above that. Currently pgJDBC eats RAM like it's unlimited. I would like that just as much as you do, but it isn't ;)
>
> Also, why not honor the fetchSize on every SELECT statement regardless of auto-commit ?
>
> Auto-commit should only affect writing statements (INSERT/UPDATE/DELETE), not read (SELECT) statements, right ? Why is auto-commit even relevant to honoring the fetchSize in the first place ? I'm confused.
This is an artifact of MVCC. You need to be in a transaction to create a snapshot to make sure that the data is consistent between fetches
^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2025-09-04 15:03 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-06-02 15:29 [pgjdbc/pgjdbc] issue #2918: Set defaultRowFetchSize=10000 by default to improve app stability "vlsi (@vlsi)" <[email protected]>
2023-06-02 15:34 ` "davecramer (@davecramer)" <[email protected]>
2023-07-12 08:16 ` "zhurs (@zhurs)" <[email protected]>
2023-07-12 12:20 ` "davecramer (@davecramer)" <[email protected]>
2023-07-12 17:25 ` "lure (@lure)" <[email protected]>
2023-07-12 17:30 ` "davecramer (@davecramer)" <[email protected]>
2023-07-12 17:36 ` "lure (@lure)" <[email protected]>
2023-07-12 17:37 ` "lure (@lure)" <[email protected]>
2023-07-12 17:48 ` "davecramer (@davecramer)" <[email protected]>
2023-07-12 19:03 ` "lure (@lure)" <[email protected]>
2023-07-12 19:53 ` "davecramer (@davecramer)" <[email protected]>
2023-07-13 10:41 ` "davecramer (@davecramer)" <[email protected]>
2023-07-13 11:01 ` "vlsi (@vlsi)" <[email protected]>
2023-07-13 11:09 ` "davecramer (@davecramer)" <[email protected]>
2023-07-13 11:29 ` "zhurs (@zhurs)" <[email protected]>
2023-07-13 11:33 ` "davecramer (@davecramer)" <[email protected]>
2025-09-04 12:19 ` "freak2fast4u (@freak2fast4u)" <[email protected]>
2025-09-04 15:03 ` "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