sql: deprecate TableDescriptor.GCMutations

[stevendanna]

This appears unused. While the schema changer adds entries that the gc
job subsequently removes, the only other code that made use of this
field (outside of tests) was FindIndexByID. FindIndexByID appears to
use it to return a special error that no one looks for.

Release note: None

[cockroach-teamcity]

This change is 

[postamar]

Let's see what CI finds to complain about but this looks OK to me so far. I can already tell you that you'll have to remove the "GCMutations" entry from the validationMap in validate_test.go in the tabledesc package.

[postamar]

I believe you could get away with outright removing this field (well, replacing it with a reserved 33 to prevent reuse of this tag number in this proto) and then remove updateDescriptorGCMutations outright. Sure, any descriptors might still have a populated GCMutations around, but:

• it's a handful of bytes at worst,
• if it's never read, it doesn't matter what's in it,
• plus it will disappear the next time that the table descriptor is updated.

This shouldn't cause any problems in a mixed-version cluster either. Worst case the GC job gets picked up by a node with the old version, which then edits this slice when it's done. It kind of expects the GC'd index to be in the slice, but doesn't complain if it's not.

[stevendanna]

Happy to go either way here. What would SQL prefer?

[postamar]

There are CI failures but they should be trivial to fix. Nice! I look forward to approving this.

[ajwerner]

mod CI too

Reviewed 9 of 10 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner, @postamar, and @stevendanna)

---

pkg/sql/gcjob/descriptor_utils.go, line 26 at r1 (raw file):

)

// updateDescriptorGCMutations removes the GCMutation for tie given

s/tie/the/

[postamar]

Reviewed 10 of 10 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @stevendanna)

---

pkg/sql/catalog/descpb/structured.proto, line 1124 at r1 (raw file):

Previously, stevendanna (Steven Danna) wrote…

Happy to go either way here. What would SQL prefer?

The best PR is the PR that gets merged quickly, so please just go ahead and merge this one and we'll create ourselves a followup issue to track the removal in the next release.

[stevendanna]

bors r=postamar,ajwerner

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build succeeded:

• GitHub CI (Cockroach)