-
Notifications
You must be signed in to change notification settings - Fork 4.1k
sql: cannot perform schema change in transaction after write #54477
Description
Describe the problem
Somewhat surprisingly, this long-standing issue has not been represented on its own but rather has been lumped into #26508. Cockroach today broadcasts information about changes to descriptors, zone configs, cluster settings, and tenants via a special commit trigger on the SystemConfigSpan. This trigger, set by calling SetSystemConfigTrigger necessitates that the transaction's key be a special key (the start of the SystemConfigSpan). The EndTransaction request then ensures that the trigger is run at application time. The trigger scans the entire span and publishes it to gossip. This gossip used to be used for three purposes but is now used for just two.
- Notifying KV of the system config to power the split queue, merge queue, and zone configs
- Notifying nodes of cluster settings
- (no longer true as of 20.2) notifying nodes of changes to descriptor versions.
The downside of this architecture is that one cannot change a transactions key. A transaction's key is set upon the first write. This leads to the below limitation that is surprising and a problem for compatibility.
root@127.0.0.1:40165/movr> CREATE TABLE foo (i INT PRIMARY KEY);
CREATE TABLE
root@127.0.0.1:40165/movr> BEGIN;
BEGIN
root@127.0.0.1:40165/movr OPEN> INSERT INTO foo VALUES (1);
INSERT 1
root@127.0.0.1:40165/movr OPEN> ALTER TABLE foo ADD COLUMN j INT;
ERROR: unimplemented: schema change statement cannot follow a statement that has written in the same transaction
SQLSTATE: 0A000
HINT: You have attempted to use a feature that is not yet implemented.
See: https://go.crdb.dev/issue-v/26508/v20.2
Expected behavior
Ideally we'd support arbitrary mixes of statements in transactions.
Proposed solution
There are two classes of solution as far as I'm concerned:
Rework the zone-configuration and cluster setting infrastructure
In this set of solution I'd like to see us move away from the monolithic, gossipped system config. There are a variety of problems with it which will be discussed in a forthcoming RFC. The current setup really does not work in the context of separate tenants.
My ideal solution would involve rangefeeds and would enable reasonably trivial incremental updates to be broadcasted and cached in a coherent way on all nodes.
Rework the trigger to occur at intent resolution time rather than commit time
This approach is almost certainly more focused and pragmatic. There would be some concerns about ensuring that the trigger continues to run in O(txns) rather than O(intents). Furthermore there's questions about whether this can be feasibly achieved without a change to the protocol. Perhaps a small change to the protocol would be acceptable.
Epic: CRDB-10489