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

v22.1: Cannot ALTER add a column and create partial index on that column in the same transaction #83593

Open
dimpavloff opened this issue Jun 29, 2022 · 4 comments
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) X-blathers-triaged blathers was able to find an owner

Comments

@dimpavloff
Copy link

dimpavloff commented Jun 29, 2022

Describe the problem

If a column is added to an existing table in a transaction and is then the subject of the WHERE clause of a partial index also part of the same transaction, you get back

ERROR: cannot create partial index on column "active" (3) which is not public
SQLSTATE: 0A000

This seems like a regression in 22.1 as it works in 21.2.10

To Reproduce

CREATE TABLE myusers(id UUID PRIMARY KEY, username TEXT); 
BEGIN; 
ALTER TABLE myusers ADD COLUMN active BOOL; 
CREATE UNIQUE INDEX myusers_only_one_active ON myusers (username) WHERE active = true; 
COMMIT;

ERROR: cannot create partial index on column "active" (3) which is not public
SQLSTATE: 0A000

Splitting the partial index creation into a separate transaction works around this issue.

The following also work as expected:

BEGIN; 
CREATE TABLE myusers(id UUID PRIMARY KEY, username TEXT, active BOOL); 
CREATE UNIQUE INDEX myusers_only_one_active ON myusers (username) WHERE active = true; 
COMMIT;
CREATE TABLE myusers(id UUID PRIMARY KEY, username TEXT); 
BEGIN; 
ALTER TABLE myusers ADD COLUMN active BOOL; 
CREATE UNIQUE INDEX myusers_only_one_active ON myusers (active) WHERE username like 'blah%'; 
COMMIT;

Expected behavior
The transaction completes successfully.

Environment:

  • CockroachDB version 22.1.2
  • Server OS: Linux
  • Client app cockroach sql

Additional context
Schema migrations that used to work no longer do which is preventing us from adopting 22.1 unless we rewrite the schemas which carries complications.

@dimpavloff dimpavloff added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jun 29, 2022
@blathers-crl
Copy link

blathers-crl bot commented Jun 29, 2022

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I have CC'd a few people who may be able to assist you:

  • @cockroachdb/sql-schema (found keywords: ALTER TABLE)

If we have not gotten back to your issue within a few business days, you can try the following:

  • Join our community slack channel and ask on #cockroachdb.
  • Try find someone from here if you know they worked closely on the area and CC them.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-triaged blathers was able to find an owner labels Jun 29, 2022
@postamar postamar added this to Triage in SQL Foundations via automation Jun 29, 2022
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Jun 29, 2022
@ajwerner
Copy link
Contributor

ajwerner commented Jul 5, 2022

It turned out that this was buggy and always caused incorrect results in previous versions. We backported changes to disallow this behavior given it never worked:
#82670
#82668

We do ultimately want to support this sort of change, and we hope to be able to bring support for it with our rewrite of schema changes that is currently underway.

@ajwerner ajwerner moved this from Triage to Declarative Schema Changer Graveyard in SQL Foundations Jul 5, 2022
@dimpavloff
Copy link
Author

Hey @ajwerner , thanks for the explanation, glad to hear this was already spotted and I'm (already) looking forward to the rewritten schema changes behaviour :)

Just one piece of feedback about backporting such a fix: Whilst I can see from the PR description that DML queries would fail whilst the DDL txn is being executed and fixing it by preventing the scenario makes sense, from another point of view this patch in 21.2 is a breaking change -- functionally, a DDL transaction which used to work in 21.2 no longer does and I would have preferred risking DMLs temporarily failing (in our case there wouldn't have been any on a fresh install environment) rather than having to squash a list of migrations and patch this in our previous software releases (and go through a QA & release process) so that these versions can be fresh-installed against the latest 21.2.x CRDB version.

I appreciate backwards compatibility, especially wrt bugfixes, can sometimes be a ton of effort and a grey area, but I think it's worth saying the above nevertheless.

@ajwerner
Copy link
Contributor

ajwerner commented Jul 5, 2022

x-ref: #79613

@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) X-blathers-triaged blathers was able to find an owner
Projects
SQL Foundations
  
Declarative Schema Changer Will Fix T...
Development

No branches or pull requests

2 participants