cockroachdb / cockroachdb/cockroach

sql: FETCH from DECLARE CURSOR does not reset statement_timeout

Open
#96,322 6 comments 0 reactions 0 assignees View on GitHub
A-tools-aws-dms C-bug T-sql-foundations
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

Today, after declaring a cursor, `FETCH` is subject to the `statement_timeout`. However, the timeout is from the beginning of `DECLARE CURSOR`, not from the `FETCH`.

See this reproduction:

```
root@127.0.0.1:26257/movr> set statement_timeout = '5s';
SET

Time: 1ms total (execution 1ms / network 0ms)

root@127.0.0.1:26257/movr> CREATE TABLE t (a int primary key);
CREATE TABLE

Time: 6ms total (execution 6ms / network 0ms)

root@127.0.0.1:26257/movr> insert into t values (1), (2);
INSERT 0 2

Time: 7ms total (execution 6ms / network 0ms)

root@127.0.0.1:26257/movr> begin;
BEGIN

Time: 0ms total (execution 0ms / network 0ms)

root@127.0.0.1:26257/movr OPEN> declare c cursor for SELECT * FROM t;
DECLARE CURSOR

Time: 3ms total (execution 3ms / network 0ms)

root@127.0.0.1:26257/movr OPEN> fetch 1 from c;
a
-----
1
(1 row)

Time: 1ms total (execution 1ms / network 0ms)

root@127.0.0.1:26257/movr OPEN> -- wait 5 seconds ......
-> ;
OK

Time: 0ms total (execution 0ms / network 0ms)

root@127.0.0.1:26257/movr OPEN> fetch 1 from c;

ERROR: query execution canceled due to statement timeout
SQLSTATE: 57014
```

quote @rafiss
> ah actually maybe the fix should be to make sure fetch always resets the timer, so that the calculation here doesn't hit the timeout https://github.com/cockroachdb/cockroach/blob/e402d828641007572b0e44ae70a628ce5bf0982c/pkg/sql/conn_executor_exec.go#L530

otan note:
> hmm i'm not sure it's so straightforward to change conn_executor_exec - the abstraction for calling FETCH is all hidden behind the InternalExecutor/connExecutor and pausing and resuming the timeouts on each FETCH is difficult. i think we have to move the statement timeout logic to fetchNode to make this work.
>
> if we still want statement_timeout for fetch, we should set the statement timeout to 0s on the internalexecutor setting up the cursor. if we really want it to work soon & backported, i'd recommend a session variable that overrides statement timeouts for 22.2 for DECLARE CURSOR - true by default for 23.1+, false by default 22.2

Jira issue: CRDB-24063

Epic CRDB-60818

Contributor guide

Open the contributing guide

Research direction

Start with the DECLARE CURSOR/FETCH reproduction in the issue, then read pkg/sql/conn_executor_exec.go around the referenced timeout calculation and inspect fetchNode, which the discussion identifies as a possible location. Determine how statement_timeout is applied across DECLARE and FETCH; done means a later FETCH is timed from its own execution while still respecting the intended timeout behavior, with coverage for the reproduction.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, sql
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.