Subject: [pgjdbc/pgjdbc] PR #2976: feat: fetch size for RefCursor

From: "davecramer (@davecramer)" <[email-masked]>
Date: Wed, 01 Nov 2023 15:40:01 +0000

This replaces the original PR #925 originally written by @rhavermans It was easier to pull in the changes from his PR instead of rebasing the original. This is entirely his code.

We now support setting the fetch size on a RefCursor.

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing](https://github.com/pgjdbc/pgjdbc/blob/master/CONTRIBUTING.md) document?
* [ ] Have you checked to ensure there aren't other open [Pull Requests](../../pulls) for the same update/change?

<!-- You can erase any parts of this template not applicable to your Pull Request. -->

### New Feature Submissions:

1. [ ] Does your submission pass tests?
2. [ ] Does `./gradlew autostyleCheck checkstyleAll` pass ?
3. [ ] Have you added your new test classes to an existing test suite in alphabetical order?

### Changes to Existing Features:

* [ ] Does this break existing behaviour? If so please explain.
* [ ] Have you added an explanation of what your changes do and why you'd like us to include them?
* [ ] Have you written new tests for your core changes, as applicable?
* [ ] Have you successfully run tests with your changes locally?

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Tue, 07 Nov 2023 04:51:43 +0000

(on pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSet.java:2188)

Could you please clarify why ignore the new fetch size?
I guess users might want to adjust fetchSize as they fetch, so it might be unexpected to ignore the new values.

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Tue, 07 Nov 2023 04:59:10 +0000

(on pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSet.java)

It looks weird that `closeRefCursor` is called only when `fetchSize` exceeds 0.
Why can't it close unconditionally?
I might be missing something, but as I read the code, `close()` should probably free up the resources, and conditional release looks like a leak to me

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Tue, 07 Nov 2023 05:10:59 +0000

(on pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSet.java:2269)

I wonder if we can reuse the existing "extended protocol" fetch to fetch the data rather than build "fetch forward" commands dynamically

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Tue, 07 Nov 2023 08:49:40 +0000

If you set the fetchSize to 0 here we will stop fetching more data.

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Tue, 07 Nov 2023 08:51:33 +0000

I'll have a look at this. Thanks

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Wed, 15 Nov 2023 18:02:13 +0000

@vlsi I want to include this in the release. Thoughts ?

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 16 Nov 2023 08:50:47 +0000

(on pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSet.java:2263)

I am afraid this change does not fix the issue.

It uses `fetchRows` from the base `ResultSet` rather than `fetchRows` of the `ResultSet` that is associated with `refcursor` in question.

In other words:

Imagine we call `{? = call test_blob(?)}`.
`CallableStatement.execute` would gather call results, and it would execute `getObject` at https://github.com/pgjdbc/pgjdbc/blob/ca25d1053199da96a63838f7476a14137dc9d9be/pgjdbc/src/main/java/org/postgresql/jdbc/PgCallableStatement.java#L131

Then it would delegate to `PgResultSet.getObject -> internalGetObject -> getRefCursor`, and it would effectively use `fetchSize` from a temporary resultset associated with `{? = call test_blob(?)}`.

I believe we should add a test to ensure `fetch forward` is triggered at all.

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Thu, 16 Nov 2023 11:57:30 +0000

Here are the logs from running

```
public void testFetchSize() throws SQLException {
    checkAndRunResult(3);
  }
```

```
2023-11-16 06:53:04.080 EST [84077] LOG:  duration: 0.136 ms  execute <unnamed>/C_1: select * from testspg__getRefcursor ($1)  as result
2023-11-16 06:53:04.080 EST [84077] DETAIL:  parameters: $1 = NULL
2023-11-16 06:53:04.080 EST [84077] LOG:  duration: 0.008 ms  parse <unnamed>: FETCH FORWARD 3 IN "<unnamed portal 1>"
2023-11-16 06:53:04.080 EST [84077] LOG:  duration: 0.002 ms  bind <unnamed>: FETCH FORWARD 3 IN "<unnamed portal 1>"
2023-11-16 06:53:04.080 EST [84077] LOG:  duration: 0.039 ms  execute <unnamed>: FETCH FORWARD 3 IN "<unnamed portal 1>"
2023-11-16 06:53:04.080 EST [84077] LOG:  duration: 0.002 ms  parse <unnamed>: FETCH FORWARD 3 IN "<unnamed portal 1>"
2023-11-16 06:53:04.080 EST [84077] LOG:  duration: 0.001 ms  bind <unnamed>: FETCH FORWARD 3 IN "<unnamed portal 1>"
2023-11-16 06:53:04.080 EST [84077] LOG:  duration: 0.003 ms  execute <unnamed>: FETCH FORWARD 3 IN "<unnamed portal 1>"
2023-11-16 06:53:04.081 EST [84077] LOG:  duration: 0.002 ms  parse <unnamed>: FETCH FORWARD 3 IN "<unnamed portal 1>"
2023-11-16 06:53:04.081 EST [84077] LOG:  duration: 0.002 ms  bind <unnamed>: FETCH FORWARD 3 IN "<unnamed portal 1>"
2023-11-16 06:53:04.081 EST [84077] LOG:  duration: 0.001 ms  execute <unnamed>: FETCH FORWARD 3 IN "<unnamed portal 1>"
2023-11-16 06:53:04.081 EST [84077] LOG:  duration: 0.006 ms  parse <unnamed>: CLOSE "<unnamed portal 1>"
2023-11-16 06:53:04.081 EST [84077] LOG:  duration: 0.001 ms  bind <unnamed>: CLOSE "<unnamed portal 1>"
2023-11-16 06:53:04.081 EST [84077] LOG:  duration: 0.002 ms  execute <unnamed>: CLOSE "<unnamed portal 1>"
2023-11-16 06:53:04.081 EST [84077] LOG:  duration: 0.005 ms  parse S_2: COMMIT
2023-11-16 06:53:04.081 EST [84077] LOG:  duration: 0.002 ms  bind S_2: COMMIT
2023-11-16 06:53:04.081 EST [84077] LOG:  duration: 0.006 ms  execute S_2: COMMIT
```

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 16 Nov 2023 12:22:06 +0000

(on pgjdbc/src/test/java/org/postgresql/test/jdbc2/RefCursorTest.java:185)

Frankly speaking, I would not expect `call.setFetchSize` to be inherited when fetching the refcursor.

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Thu, 16 Nov 2023 12:48:14 +0000

I'm not sure why not ?

Statement.setFetchSize() docs say 
```
Gives the JDBC driver a hint as to the number of rows that should be fetched from the database when more rows are needed for ResultSet objects generated by this Statement. If the value specified is zero, then the hint is ignored. The default value is zero.
Params:
rows – the number of rows to fetch
Throws:
SQLException – if a database access error occurs, this method is called on a closed Statement or the condition rows >= 0 is not satisfied.
```

How else would they be used ?

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 16 Nov 2023 12:59:04 +0000

ResultSet.setFetchSize would be ignored though

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Thu, 16 Nov 2023 13:03:57 +0000

Yes, however it is allowed. 
```
Gives the JDBC driver a hint as to the number of rows that should be fetched from the database when more rows are needed for this ResultSet object. If the fetch size specified is zero, the JDBC driver ignores the value and is free to make its own best guess as to what the fetch size should be. The default value is set by the Statement object that created the result set. The fetch size may be changed at any time.
```

I don't see a way to change it once we have the statement. Does that currently work with portals ?

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 16 Nov 2023 13:38:22 +0000

We should treat `fetchSize` like a hint rather than an "exact value to use for fetching". Then `0` would not cause "0 rows to fetch"

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 16 Nov 2023 13:38:53 +0000

The initial value for `fetchSize` is `0`, so ignoring `setFetchSize(0)` sounds arbitrary

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Thu, 16 Nov 2023 13:40:41 +0000

fetch size of zero is specifically called out as special `If the fetch size specified is zero, the JDBC driver ignores the value and is free to make its own best guess as to what the fetch size should be.`

---

From: "jbaris (@jbaris)" <[email-masked]>
Date: Wed, 26 Mar 2025 12:33:25 +0000

@davecramer any news on this? We are still facing this issue :(