plpgsql: disallow txn control in udfs and nested procedures

[DrewKimball]

This patch adds validation for transaction control statements, so that attempting to use them from within a UDF results in an error. In addition, attempting to use a transaction control statement from a nested stored procedure call will now result in an "unimplemented" error.

Informs #122266

Release note: None

[cockroach-teamcity]

This change is 

[yuzefovich]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball and @mgartner)

---

pkg/sql/opt/optbuilder/builder.go line 155 at r1 (raw file):

	// insideDataSource is true when we are processing a data source.
	insideDataSource bool
  

nit: dangling spaces.

---

pkg/sql/opt/optbuilder/create_function.go line 381 at r1 (raw file):

		plpgsqltree.Walk(&tc, stmt.AST)
		if tc.foundTxnControlStatement {
			if !cf.IsProcedure {

nit: should we only perform the walk for functions? I.e. for procedures the walk seems redundant.

---

pkg/sql/opt/optbuilder/plpgsql.go line 290 at r1 (raw file):

			if b.ob.insideNestedPLpgSQLCall {
				// Disallow transaction control statements in nested routines for now.
				// TODO(#12345): once we support this, make sure to validate that

nit: is this the right issue number?

---

pkg/ccl/logictestccl/testdata/logic_test/plpgsql_txn line 995 at r1 (raw file):

statement error pgcode 2D000 pq: invalid transaction termination
CREATE FUNCTION f() RETURNS INT LANGUAGE PLpgSQL AS $$ BEGIN COMMIT; END $$;

nit: did you mean to use ROLLBACK in this case?

[DrewKimball]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @mgartner and @yuzefovich)

---

pkg/sql/opt/optbuilder/builder.go line 155 at r1 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

nit: dangling spaces.

Done.

---

pkg/sql/opt/optbuilder/create_function.go line 381 at r1 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

nit: should we only perform the walk for functions? I.e. for procedures the walk seems redundant.

Good idea, Done.

---

pkg/sql/opt/optbuilder/plpgsql.go line 290 at r1 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

nit: is this the right issue number?

Hmmm... Done.

---

pkg/ccl/logictestccl/testdata/logic_test/plpgsql_txn line 995 at r1 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

nit: did you mean to use ROLLBACK in this case?

Yes, good catch.

[yuzefovich]

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mgartner)

[DrewKimball]

TFYR!

bors r+

[craig]

Build succeeded:

• linux_amd64_build
• local_roachtest
• docker_image_amd64
• unit_tests
• lint
• linux_amd64_fips_build
• check_generated_code
• macos_arm64_build
• linux_arm64_build
• acceptance
• macos_amd64_build
• local_roachtest_fips
• examples_orms
• windows_build

[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.

---

error creating merge commit from e354328 to blathers/backport-release-24.1-122657: 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 24.1.x failed. See errors above.

---

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