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: nested blocks are unaware of each other's cursors #121078

Closed
DrewKimball opened this issue Mar 25, 2024 · 2 comments · Fixed by #122321
Closed

plpgsql: nested blocks are unaware of each other's cursors #121078

DrewKimball opened this issue Mar 25, 2024 · 2 comments · Fixed by #122321
Assignees
Labels
branch-master Failures and bugs on the master branch. branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. GA-blocker T-sql-queries SQL Queries Team

Comments

@DrewKimball
Copy link
Collaborator

DrewKimball commented Mar 25, 2024

A PL/pgSQL block has to be aware of all cursors created within its scope, including those created within nested blocks. Currently, this is not the case, as demonstrated in the following logic test:

statement ok
CREATE FUNCTION f(n INT) RETURNS INT AS $$
  DECLARE
    x REFCURSOR;
    y REFCURSOR;
  BEGIN
    OPEN x FOR SELECT 100;
    BEGIN
      OPEN y FOR SELECT 200;
      IF n = 0 THEN
        RETURN 1 // 0;
      END IF;
    EXCEPTION
      WHEN division_by_zero THEN
        RETURN (SELECT count(*) FROM pg_cursors);
    END;
    RETURN 1 // 0;
  EXCEPTION
    WHEN division_by_zero THEN
      RETURN (SELECT count(*) FROM pg_cursors);
  END
$$ LANGUAGE PLpgSQL;

query I
SELECT f(0);
----
1

query I
SELECT f(1);
----
1

Both cursors in the f(1) case should have been closed due to the exception, and yet one remains open.

Jira issue: CRDB-37056

@DrewKimball DrewKimball added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. GA-blocker T-sql-queries SQL Queries Team 24.1 labels Mar 25, 2024
@DrewKimball DrewKimball self-assigned this Mar 25, 2024
Copy link

blathers-crl bot commented Mar 25, 2024

Hi @DrewKimball, please add branch-* labels to identify which branch(es) this GA-blocker affects.

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

@github-project-automation github-project-automation bot moved this to Triage in SQL Queries Mar 25, 2024
@DrewKimball DrewKimball added the branch-master Failures and bugs on the master branch. label Mar 25, 2024
@DrewKimball DrewKimball moved this from Triage to Active in SQL Queries Mar 25, 2024
@michae2 michae2 added the branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 label Apr 9, 2024
@michae2
Copy link
Collaborator

michae2 commented Apr 9, 2024

[triage] consequence would be a cursor staying open that should be closed. @DrewKimball to either remove GA-blocker or fix

craig bot pushed a commit that referenced this issue Apr 16, 2024
122321: plpgsql: fix exception handling for nested blocks r=DrewKimball a=DrewKimball

#### plpgsql: prevent inlining for block-exit continuation

This commit prevents inlining for the continuation that transitions out
of a PL/pgSQL block with an exception handler. This is necessary to
ensure that the statements following the nested block are considered
part of the parent block, not the nested block. Otherwise, an error thrown
after the nested block might still be caught by the nested block's
exception handler, which is incorrect behavior.

#### plpgsql: keep track of the subroutine that begins a PL/pgSQL block

This commit adds logic to keep track of the PL/pgSQL sub-routine that
logically transitions into a PL/pgSQL block with an exception handler.
This is necessary to ensure that the state shared between sub-routines
within the same block is correctly initialized. Previously, the block
state was only initialized once, but this is incorrect for loops, which
need to re-initialize the state on each iteration.

#### plpgsql: roll back all cursors within a block that handles an exception

This commit fixes handling for cursors opened within the scope of a
block with an exception handler that has nested blocks or nested routine
calls. Previously, if a PL/pgSQL block caught an exception, only the cursors
opened directly by that block would be rolled back. Any cursors opened by
a nested block or routine call would remain open. Now, the block state
tracks the timestamp when the block's execution began. Once an exception
is caught, all cursors with a timestamp later than the block's start are
rolled back.

Fixes #122278
Fixes #121078

Release note: None

Co-authored-by: Drew Kimball <[email protected]>
@craig craig bot closed this as completed in e729242 Apr 16, 2024
@github-project-automation github-project-automation bot moved this from Active to Done in SQL Queries Apr 16, 2024
blathers-crl bot pushed a commit that referenced this issue Apr 16, 2024
This commit fixes handling for cursors opened within the scope of a
block with an exception handler that has nested blocks or nested routine
calls. Previously, if a PL/pgSQL block caught an exception, only the cursors
opened directly by that block would be rolled back. Any cursors opened by
a nested block or routine call would remain open. Now, the block state
tracks the timestamp when the block's execution began. Once an exception
is caught, all cursors with a timestamp later than the block's start are
rolled back.

Fixes #121078

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-master Failures and bugs on the master branch. branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. GA-blocker T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants