pgwire: fix JDBC compatibility for SP with COMMIT

[ZhouXing19]

Fixes #158771

Previously, when using the extended protocol (Parse/Bind/Describe/Execute/Sync) to call a PL/pgSQL procedure containing COMMIT statements, CockroachDB would send extra RowDescription messages after the COMMIT, causing the JDBC driver to throw NoSuchElementException due to unexpected message sequences.

This change fixes the message flow to match JDBC driver expectations when procedures execute COMMIT statements internally. The fix ensures that the proper sequence of PostgreSQL wire protocol messages is sent, preventing the driver from entering an inconsistent state.

For reference, PG's code for describe portal message: https://github.com/postgres/postgres/blob/451c43974f8e199097d97624a4952ad0973cea61/src/backend/tcop/postgres.c#L2731

Release Notes (Bug Fix): Fixed compatibility issue with JDBC driver when calling PL/pgSQL procedures containing COMMIT statements via prepared statements. The driver would previously throw NoSuchElementException due to unexpected PostgreSQL wire protocol message sequences.

[cockroach-teamcity]

This change is 

[yuzefovich]

Nice! I have some nits and a question to consider.

@yuzefovich reviewed 16 files and all commit messages, and made 6 comments.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss and @ZhouXing19).

---

pkg/sql/conn_executor_prepare.go line 679 at r1 (raw file):

			res.SetNoDataRowDescription()
		} else {
			skipRowDescription := false

Hm, feels like this part might not be needed - if I comment it out, the existing tests still work ...

---

pkg/sql/pgwire/testdata/pgtest/pgjdbc line 290 at r1 (raw file):

{"Type":"CommandComplete","CommandTag":"CALL"}
{"Type":"ReadyForQuery","TxStatus":"I"}

nit: should we verify the final state of the table?

---

pkg/sql/conn_executor.go line 1716 at r1 (raw file):

			resumeStmt statements.Statement[tree.Statement]

			// callRowDescSent tracks whether RowDescription has already been sent for a CALL statement

nit: wrap comments at 80 characters.

---

pkg/sql/conn_executor.go line 4398 at r1 (raw file):

		ast.StatementReturnType() == tree.Rows {
		// Only write RowDescription message if the procedure has output parameters.
		skipWriteRowDesc := false

nit: let's unify this variable name together with parameter names in the interface methods - how about skipRowDesc everywhere? (Currently we use skipWriteRowDesc in one spot and skipRowDescription in the other.)

---

pkg/sql/conn_executor.go line 4399 at r1 (raw file):

		// Only write RowDescription message if the procedure has output parameters.
		skipWriteRowDesc := false
		if _, isCallStmt := ast.(*tree.Call); isCallStmt {

nit: should we compare against tree.CallStmtTag for consistency?

[ZhouXing19]

@ZhouXing19 made 5 comments and resolved 3 discussions.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss and @yuzefovich).

---

pkg/sql/conn_executor.go line 1716 at r1 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

nit: wrap comments at 80 characters.

Done.

---

pkg/sql/conn_executor.go line 4398 at r1 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

nit: let's unify this variable name together with parameter names in the interface methods - how about skipRowDesc everywhere? (Currently we use skipWriteRowDesc in one spot and skipRowDescription in the other.)

Unified to skipRowDescription.

---

pkg/sql/conn_executor.go line 4399 at r1 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

nit: should we compare against tree.CallStmtTag for consistency?

Done.

---

pkg/sql/conn_executor_prepare.go line 679 at r1 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

Hm, feels like this part might not be needed - if I comment it out, the existing tests still work ...

Good catch -- we don't need to check if callRowDescSent since it only matters when for the actual stmt execution. Here we only need to check if the stmt returns any data or not. Removed.

---

pkg/sql/pgwire/testdata/pgtest/pgjdbc line 290 at r1 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

nit: should we verify the final state of the table?

Done.

[yuzefovich]

Feels like this should be backported, so I added a couple of labels.

@yuzefovich reviewed 7 files and all commit messages, made 2 comments, and resolved 2 discussions.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @rafiss and @ZhouXing19).

---

pkg/sql/conn_executor_prepare.go line 668 at r2 (raw file):

			// Sending a nil formatCodes is equivalent to sending all text format
			// codes.
			res.SetPortalOutput(ctx, cursor.Rows.Types(), nil /* formatCodes */)

nit: let's restore the inlined comment.

[ZhouXing19]

TFTR!
bors r+

[rafiss]

I think this comment should be updated to clarify that this was the old behavior. This PR fixes it so the statement in the comment is no longer true.

[ZhouXing19]

Done.

[yuzefovich]

bors r-

To update the comment based on Rafi's review

[craig]

Canceled.

[ZhouXing19]

Done updating the comment in the test.
TFTRs!
bors r+

[craig]

Build succeeded:

• local_roachtest_fips
• docker_image_amd64
• local_roachtest
• check_generated_code
• examples_orms
• acceptance
• lint
• linux_amd64_build
• linux_amd64_fips_build
• unit_tests

[blathers-crl]

Based on the specified backports for this PR, I applied new labels to the following linked issue(s). Please adjust the labels as needed to match the branches actually affected by the issue(s), including adding any known older branches.

---

Issue #158771: branch-release-25.4, branch-release-26.1.

---

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[blathers-crl]

Encountered an error creating backports. Some common things that can go wrong:

1. The backport branch might have already existed.
2. There was a merge conflict.
3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

---

💡 Consider backporting to the fork repo instead of the main repo. See instructions for more details.

error creating merge commit from f1d26c6 to blathers/backport-release-25.4-160377: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 25.4.x failed. See errors above.

---

💡 Consider backporting to the fork repo instead of the main repo. See instructions for more details.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}