sql/schemachanger: Preserving column family order for complex column type changes #133845
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 1 of 14 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @spilchen)
-- commits line 5 at r1:
Trying to understand the rationale for why we need this? From a SQL viewpoint nothing changes based on the order (maybe the DDL from SHOW CREATE only). I think a hazard is that rows could become unreadable if we aren't careful about how this is ordered
There was a problem hiding this comment.
Reviewable status:
Previously, fqazi (Faizan Qazi) wrote…
Trying to understand the rationale for why we need this? From a SQL viewpoint nothing changes based on the order (maybe the DDL from SHOW CREATE only). I think a hazard is that rows could become unreadable if we aren't careful about how this is ordered
The reasons were:
- To ensure consistent output in commands like
SHOW CREATEafter the alteration. - The legacy schema changer also preserved the order.
I admit these aren’t strong reasons, so I’m okay with deferring this work.
There was a problem hiding this comment.
Reviewed 13 of 14 files at r1.
Reviewable status:
Previously, spilchen wrote…
The reasons were:
- To ensure consistent output in commands like
SHOW CREATEafter the alteration.- The legacy schema changer also preserved the order.
I admit these aren’t strong reasons, so I’m okay with deferring this work.
Just as sanity lets confirm this with a DML injection test to confirm that everything stays readable. The changes themselves look fine, but it would just add piece of mind.
…type changes When altering a column’s type in a way that requires a backfill, we drop the old column and add a new one. Previously, the new column was always added to the end of the column family. This change ensures the column family order is preserved. In the DSC, the column family is updated when a new ColumnType element is added. I introduced a new field to this element to control the order, which requires specifying the column ID that the new column should follow. Epic: CRDB-25314 Closes cockroachdb#133040 Release note: none
spilchen
force-pushed
the
issue-133040/preserve-order
branch
from
53de17e to
5059f89
Compare
There was a problem hiding this comment.
Reviewable status:
Previously, fqazi (Faizan Qazi) wrote…
Just as sanity lets confirm this with a DML injection test to confirm that everything stays readable. The changes themselves look fine, but it would just add piece of mind.
I added a DML injection test. But there is a known problem where the column we are altering can become invisible for a couple of stages. I have issue #133996 opened to investigate that.
|
TFTR @fqazi bors r+ |
When altering a column’s type in a way that requires a backfill, we drop the old column and add a new one. Previously, the new column was always added to the end of the column family. This change ensures the column family order is preserved.
In the DSC, the column family is updated when a new ColumnType element is added. I introduced a new field to this element to control the order, which requires specifying the column ID that the new column should follow.
Epic: CRDB-25314
Closes #133040
Release note: none