migrations: add target column to system.pts_records #74281
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
|
First commit is from #74211. |
adityamaru
force-pushed
the
add-migration-to-pts-table
branch
from
b963f58 to
a5057c6
Compare
This was referenced Dec 28, 2021
adityamaru
force-pushed
the
add-migration-to-pts-table
branch
from
a5057c6 to
91e7bad
Compare
adityamaru
requested review from
a team and
shermanCRL
and removed request for
a team and
shermanCRL
adityamaru
force-pushed
the
add-migration-to-pts-table
branch
from
91e7bad to
ce46759
Compare
|
@arulajmani @irfansharif rebased and RFAL! |
ajwerner
approved these changes
Jan 4, 2022
pkg/migration/migrations/alter_table_protected_timestamp_records.go
Outdated
Show resolved
Hide resolved
irfansharif
approved these changes
Jan 4, 2022
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @adityamaru and @arulajmani)
pkg/migration/migrations/alter_table_protected_timestamp_records_test.go, line 64 at r1 (raw file):
// Inject the old copy of the descriptor. migrations.InjectLegacyTable(ctx, t, s, systemschema.ProtectedTimestampsRecordsTable, getDeprecatedProtectedTimestampRecordsDescriptor)
[nit] Line wrap.
pkg/migration/migrations/alter_table_protected_timestamp_records_test.go, line 66 at r1 (raw file):
migrations.InjectLegacyTable(ctx, t, s, systemschema.ProtectedTimestampsRecordsTable, getDeprecatedProtectedTimestampRecordsDescriptor) // Validate that the protected timestamp records table has the old schema. migrations.ValidateSchemaExists(
[nit] Wrap this in a tiny helper func:
validateSchemaExists := func (exp bool) { ...}
// ...
validateSchemaExists(false)
migrations.Migrate(...)
validateSchemaExists(true)
pkg/migration/migrations/alter_table_protected_timestamp_records_test.go, line 99 at r1 (raw file):
} // getDeprecatedTableStatisticsDescriptor returns the system.table_statistics
Comment needs to be updated.
adityamaru
force-pushed
the
add-migration-to-pts-table
branch
from
ce46759 to
9c2e0c8
Compare
adityamaru
commented
Jan 5, 2022
There was a problem hiding this comment.
Reviewable status:
pkg/migration/migrations/alter_table_protected_timestamp_records_test.go, line 64 at r1 (raw file):
Previously, irfansharif (irfan sharif) wrote…
[nit] Line wrap.
Done.
pkg/migration/migrations/alter_table_protected_timestamp_records_test.go, line 66 at r1 (raw file):
Previously, irfansharif (irfan sharif) wrote…
[nit] Wrap this in a tiny helper func:
validateSchemaExists := func (exp bool) { ...} // ... validateSchemaExists(false) migrations.Migrate(...) validateSchemaExists(true)
Done.
pkg/migration/migrations/alter_table_protected_timestamp_records_test.go, line 99 at r1 (raw file):
Previously, irfansharif (irfan sharif) wrote…
Comment needs to be updated.
Done.
This change adds a `BYTES` column to the system.pts_records table, that will store an encoded protocol buffer representing the `Target` on a protected timestamp record. We do not populate the column in this PR, that will be done in a followup. Additionally, we add a tenant migration to `ALTER TABLE ... ADD COLUMN` to run on the upgrade path for older clusters. Informs: cockroachdb#73727 Release note (sql change): `system.protected_timestamp_records` table now has an additional `target` column that will store an encoded protobuf that represents the target a record protects. This target can either be the entire cluster, tenants, or schema objects (databases/tables).
adityamaru
force-pushed
the
add-migration-to-pts-table
branch
from
9c2e0c8 to
cdb829c
Compare
|
TFTR! bors r=ajwerner,irfansharif |
craig bot
pushed a commit
that referenced
this pull request
Jan 5, 2022
74281: migrations: add target column to system.pts_records r=ajwerner,irfansharif a=adityamaru This change adds a `BYTES` column to the system.pts_records table, that will store an encoded protocol buffer representing the `Target` on a protected timestamp record. We do not populate the column in this PR, that will be done in a followup. Additionally, we add a tenant migration to `ALTER TABLE ... ADD COLUMN` to run on the upgrade path for older clusters. Informs: #73727 Release note (sql change): `system.protected_timestamp_records` table now has an additional `target` column that will store an encoded protobuf that represents the target a record protects. This target can either be the entire cluster, tenants, or schema objects (databases/tables). Co-authored-by: Aditya Maru <[email protected]>
|
Build failed: |
|
TestComposeGSS flake bors r=ajwerner,irfansharif |
craig bot
pushed a commit
that referenced
this pull request
Jan 5, 2022
74281: migrations: add target column to system.pts_records r=ajwerner,irfansharif a=adityamaru This change adds a `BYTES` column to the system.pts_records table, that will store an encoded protocol buffer representing the `Target` on a protected timestamp record. We do not populate the column in this PR, that will be done in a followup. Additionally, we add a tenant migration to `ALTER TABLE ... ADD COLUMN` to run on the upgrade path for older clusters. Informs: #73727 Release note (sql change): `system.protected_timestamp_records` table now has an additional `target` column that will store an encoded protobuf that represents the target a record protects. This target can either be the entire cluster, tenants, or schema objects (databases/tables). 74418: authors: add sherman-grewal to authors r=sherman-grewal a=sherman-grewal Release note: None Co-authored-by: Aditya Maru <[email protected]> Co-authored-by: Sherman Grewal <[email protected]>
|
Build failed (retrying...): |
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change adds a
BYTEScolumn to the system.pts_recordstable, that will store an encoded protocol buffer
representing the
Targeton a protected timestamprecord. We do not populate the column in this PR, that will
be done in a followup.
Additionally, we add a tenant migration to
ALTER TABLE ... ADD COLUMNto run on the upgrade path for older clusters.
Informs: #73727
Release note (sql change):
system.protected_timestamp_recordstablenow has an additional
targetcolumn that will store an encodedprotobuf that represents the target a record protects. This target
can either be the entire cluster, tenants, or schema objects
(databases/tables).