sql: support DDL in read committed transactions

[michae2]

We currently do not allow DDL statements like CREATE TABLE, CREATE SCHEMA, CREATE INDEX, etc. within explicit read committed transactions when autocommit_before_ddl is off. Postgres allows these statements within read committed transactions.

A demonstration is something like:

SET autocommit_before_ddl = off;
SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true;
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
SELECT 1;
CREATE TABLE a (a INT PRIMARY KEY);
ROLLBACK;

In CRDB this fails with explicit transaction involving a schema change needs to be SERIALIZABLE.

Jira issue: CRDB-33677

[rafiss]

https://github.com/cockroachdb/cockroach/issues/87236 might be an interesting way to resolve this - it would match the behavior of other systems like MySQL.

[michae2]

Also making this about implicit transactions (which currently are auto-upgraded to Serializable to run the DDL).

[rmloveland]

@michae2 and @rafiss I'm seeing the following behavior on v24.2.0-beta.2 which differs from the issue description, but could def be holding it wrong

summary:

in v24.2.0 we do appear to allow DDL in read committed transactions as long as autocommit_before_ddl is on (default is off) (see below section 'manual testing' for details of statements etc) therefore do we still consider this a known limitation? my sense is it is not, or at least the limitation has changed, and the limitation is now that you must turn on DDL autocommit if you want to do DDL in READ COMMITTED transactions do you agree with this assessment? I'm working on a docs PR for the v24.2 release next week and intend to make a docs PR listing this on the known limitations page

manual testing:

If DDL auto commit is off, this happens

SET autocommit_before_ddl = off;
SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true;
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
SELECT 1;
CREATE TABLE a (a INT PRIMARY KEY);
ROLLBACK;

ERROR: to use multi-statement transactions involving a schema change under weak isolation levels, enable the autocommit_before_ddl setting
SQLSTATE: 0A000

and a does not get created (as expected from error message)

show create table a;

ERROR: relation "a" does not exist
SQLSTATE: 42P01

however it does appear to work (AFAICT?) with DDL autocommit turned on

SET autocommit_before_ddl = on;
SET CLUSTER SETTING sql.txn.read_committed_isolation.enabled = true;
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
SELECT 1;
CREATE TABLE b (a INT PRIMARY KEY);
ROLLBACK;

NOTICE: auto-committing transaction before processing DDL due to autocommit_before_ddl setting

and b does end up getting created

show create table b;

  table_name |             create_statement
-------------+--------------------------------------------
  b          | CREATE TABLE public.b (
             |     a INT8 NOT NULL,
             |     CONSTRAINT b_pkey PRIMARY KEY (a ASC)
             | )
(1 row)

[rmloveland]

cc @taroface apparently we already have this listed in the READ COMMITTED known limitations for v24.2 here, but the text may need updating based on the findings above

Schema changes (e.g., CREATE TABLE, CREATE SCHEMA, CREATE INDEX) cannot be performed within explicit READ COMMITTED transactions, and will cause transactions to abort. As a workaround, set the transaction's isolation level to SERIALIZABLE. #114778

given we already doc this on the KLs page, I am going to add the docs-done label to this issue since (from the "we documented a known limitation" POV) this is already done however we may need to update the text

[bratwurzt]

Why is sql.txn.read_committed_isolation.enabled moved to enterprise license in version 24.1.3? It's a very basic functionality that just about every database supports out of the box.

[michae2]

in v24.2.0 we do appear to allow DDL in read committed transactions as long as autocommit_before_ddl is on (default is off) (see below section 'manual testing' for details of statements etc) therefore do we still consider this a known limitation? my sense is it is not, or at least the limitation has changed, and the limitation is now that you must turn on DDL autocommit if you want to do DDL in READ COMMITTED transactions do you agree with this assessment?

@rmloveland, yes, I think your assessment is correct (thank you for testing it out!). I will change the description to mention autocommit_before_ddl.

[taroface]

I've amended the 24.2 known limitation accordingly: https://github.com/cockroachdb/docs/pull/18800/commits/c28002b81c3d2d5a4dba9dfd40b079782e5e59f5