Cockroach: sql: support ALTER TABLE ... ALTER COLUMN ... SET DATA TYPE ...

[knz]

Requested by @jayesh_nambiar_twitter on gitter, suggested by @dt:

you can add the new col, use an UPDATE tbl SET newcol = f(oldcol) to move the data over, then drop the old col
(and then rename newcol to oldcol)
f there could be as simple as a ::newtype cast, or some other builtin function that computes the new values from the old ones

Edit by Richard (March, 19, 2020)
Proposal for alter column type: (Rohan's suggestion)

Suppose a user wants to alter column c from type t to t'

The overall procedure will be similar to primary key changes. In the first txn, we add:

• A new computed column c' computed using c::t'
• For all indexes i that contain c, create a new i' that indexes all columns in i, but replaces c for c'
• Enqueue an "index swap" mutation containing all indexes that need to be rewritten.

When processing the index swap mutation, we'll need to

• change c' to no longer be a computed column, and to be renamed to c.
• Add a column drop mutation for the old column c

It seems like we'll need to handle this case of child jobs / async job cleanup better since there are now multiple things using it.

An annoying case seems to be if the column is indexed in the primary key. In that case, we'll want to just add the new column, and then use the existing alter primary key logcic, with a column drop.

Another thing that will need to be done is make sure everything that references the column is updated, like foreign keys

Summary:

1. Create a new computed column c' using c::t'
2. For all indexes i that contain c, create a new i' that indexes all columns in c, but replaces c for c'
3. Enqueue an "index swap" mutation containing all indexes that need to be rewritten.

The index swap mutation:

1. Change c' to no longer be a computed column, rename to c
2. Add a column drop mutation for the old column c

After Swap:
Change foreign key references

Questions:
Can sequence be altered to another sequence?

• Can altered column type still be a sequence?

[bdarnell]

See also #5950, which asks for a simpler subset of type changes (changing the length limits of string types, needed by the django ORM).

[yznming]

@knz Can I have a try?

[knz]

Hi @yznming yes you can try this, but this is not my area of expertise so I am not sure I can help you.

[yznming]

@knz you have gave me lots of help, thank you so much :)

[jordanlewis]

Based on an informal survey of popular open source Ruby on Rails apps, it seems like this is a very common operation - developers often add migrations that change column types as their apps evolved.

We should make an effort to support this in the not-too-distant future.

[mclark4386]

Yeah, this is a big deal in almost every framework I've ever used... I'm currently trying to implement a workaround for the Buffalo framework for go, but have run into a weird snag if anyone could help that would be at least one platform that could support CockroachDB out of the box with all migrations supported.

[knz]

cc @awoods187 this feature is coming up more and more for compatibility with other tools, perhaps it is time to re-prioritize.

[bobvawter]

Drafting an RFC here https://github.com/cockroachdb/cockroach/pull/24703

[tim-o]

Zendesk ticket #2497 has been linked to this issue.

[awoods187]

top requested feature based on telemetry data

[tim-o]

Comment made from Zendesk by Ricardo Rocha on 2019-07-08 at 14:23:

Closing older ZenDesk forum incidents as per Support meeting on 7/2

[markelog]

Why this was closed?

[jordanlewis]

Good question. It shouldn't have been.

[tim-o]

That was our error, apologies.

[tim-o]

Comment made from Zendesk by Ricardo Rocha on 2019-07-15 at 17:26:

Reopening since bulk update closed incorrectly.

[markelog]

Cool, thanks :), excited about the possibility of this feature btw

[sandstrom]

@RichardJCai In which commit was this fixed? (just curious, I can't find it)

[Luit]

Apparently, not yet fully in 20.2.0-alpha.1:

ERROR:  internal error: unknown conversion for VARCHAR(20) -> VARCHAR(10)

[knz]

cc @jordanlewis for re-triage

[RichardJCai]

Apparently, not yet fully in 20.2.0-alpha.1:

ERROR:  internal error: unknown conversion for VARCHAR(20) -> VARCHAR(10)

This should be fixed in 20.2.0 alpha 2.

[RichardJCai]

@RichardJCai In which commit was this fixed? (just curious, I can't find it)

I think the PR automatically closed the ticket.

Here is the commit that experimental support for ALTER COLUMN TYPE where the column is not part of an index. There is still some work left to be done for cases where the column is part of an index, has a constraint, and it's currently not supported inside a transaction.

https://github.com/cockroachdb/cockroach/pull/46933/commits/dcd242935afbd3b00d59bc279729f4a2bdda6982

[jordanlewis]

Hey @solongordon can we close this in favor of #49329 at this point, or is there still more work to be done that isn't a part of the enable_experimental_alter_column_type_general flag?