Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plpgsql: properly handle COMMIT/ROLLBACK in nested procedures #122266

Open
DrewKimball opened this issue Apr 12, 2024 · 1 comment
Open

plpgsql: properly handle COMMIT/ROLLBACK in nested procedures #122266

DrewKimball opened this issue Apr 12, 2024 · 1 comment
Labels
A-sql-routine UDFs and Stored Procedures C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. docs-known-limitation O-qa T-sql-queries SQL Queries Team

Comments

@DrewKimball
Copy link
Collaborator

DrewKimball commented Apr 12, 2024

Postgres allows using COMMIT/ROLLBACK in nested routines under the following conditions (from the docs):

Transaction control is only possible in CALL or DO invocations from the top level or
nested CALL or DO invocations without any other intervening command. For example,
if the call stack is CALL proc1() → CALL proc2() → CALL proc3(), then the second and
third procedures can perform transaction control actions. But if the call stack is
CALL proc1() → SELECT func2() → CALL proc3(), then the last procedure cannot do
transaction control, because of the SELECT in between.

In the long term, we should allow COMMIT/ROLLBACK in this case, and check for intervening statements to produce the correct error. For 24.1, we should just disallow a nested routine from using COMMIT/ROLLBACK entirely.

Jira issue: CRDB-37782

@DrewKimball DrewKimball added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-qa T-sql-queries SQL Queries Team A-sql-routine UDFs and Stored Procedures labels Apr 12, 2024
@DrewKimball
Copy link
Collaborator Author

Right now, this can lead to unexpected behavior since the deferred CALL execution isn't set up to handle nested routines:

CREATE PROCEDURE test_proc7c(x int, INOUT a int, INOUT b numeric)
LANGUAGE plpgsql
AS $$
BEGIN
  a := x * 10;
  b := x * 2;
  COMMIT;
END;
$$;

CREATE PROCEDURE test_proc7cc(_x int)
LANGUAGE plpgsql
AS $$
DECLARE _a int; _b numeric;
BEGIN
  CALL test_proc7c(_x, _a, _b);
  RAISE NOTICE '_x: %,_a: %, _b: %', _x, _a, _b;
END
$$;

CALL test_proc7cc(10);

Result:

NOTICE: _x: 10,_a: <NULL>, _b: <NULL>
CALL
ERROR: number of field descriptions must equal number of destinations, got 1 and 0
SQLSTATE: XX000
HINT: You have encountered an unexpected error.

Please check the public issue tracker to check whether this problem is
already tracked. If you cannot find it there, please report the error
with details by creating a new issue.

If you would rather not post publicly, please contact us directly
using the support form.

We appreciate your feedback.

DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Apr 18, 2024
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 cockroachdb#122266

Release note: None
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Apr 23, 2024
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 cockroachdb#122266

Release note: None
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Apr 24, 2024
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 cockroachdb#122266

Release note: None
craig bot pushed a commit that referenced this issue Apr 24, 2024
122657: plpgsql: disallow txn control in udfs and nested procedures r=DrewKimball a=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

Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Apr 24, 2024
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 cockroachdb#122266

Release note: None
DrewKimball added a commit that referenced this issue Apr 25, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-routine UDFs and Stored Procedures C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. docs-known-limitation O-qa T-sql-queries SQL Queries Team
Projects
Status: 24.2 Release
Development

No branches or pull requests

1 participant