sql: add NotVisible to index descriptor

[wenyihu6]

This commit adds a new field NotVisible to the struct IndexDescriptor. Since
primary indexes cannot be not visible, it also includes another check in
pkg/sql/catalog/tabledesc/validate.go. Since the invisible index feature has
not been introduced yet, all indexes created should be visible.

See also: #84776

Assists: #72576

Release note: none

[cockroach-teamcity]

This change is 

[michae2]

message: ThisPR {
  optional bool lgtm = 1 [(gogoproto.nullable) = false];
}

return &ThisPR{lgtm: true}

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

[mgartner]

Very nicely done! Let's get someone on SQL Schema to stamp this before merging.

Reviewed 24 of 24 files at r1, all commit messages.
Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @postamar and @wenyihu6)

---

-- commits line 2 at r1:
nit: we prefer to use the imperative mood for commit titles, like sql: add NotVisible to index descriptor

I think we typically use the present tense in the commit body (e.g. "This PR adds ..." rather than "This PR added ..."), but I don't see that as an official guideline in the doc linked above, so that's up to you if you want to change it.

---

-- commits line 4 at r1:
super nit: PR => commit

---

pkg/sql/catalog/catformat/index.go line 179 at r1 (raw file):

	if index.NotVisible {
		f.WriteString(" NOT VISIBLE")

Reminder to add tests for this with SHOW CREATE TABLE once not visible indexes can be added.

---

pkg/sql/opt/indexrec/hypothetical_index.go line 52 at r1 (raw file):

	// notVisible indicates if an index is not visible to the optimizer.
	notVisible bool

I don't think we need this field, we can just always return false in IsNotVisible below. I don't see a case where we'd make a recommendation to the user to create an invisible index.

You could add a TODO here to handle cases where an invisible index exists that should be made non-visible to improve the query plan.

---

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_index.go line 46 at r1 (raw file):

		IsInverted:     n.Inverted,
		IsConcurrently: n.Concurrently,
		IsNotVisible:   false, // TODO(wenyihu6): populate hidden property after CREATE

nit: hidden => IsNotVisible

[ajwerner]

LGTM

[wenyihu6]

pkg/sql/opt/indexrec/hypothetical_index.go line 52 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

I don't think we need this field, we can just always return false in IsNotVisible below. I don't see a case where we'd make a recommendation to the user to create an invisible index.

You could add a TODO here to handle cases where an invisible index exists that should be made non-visible to improve the query plan.

Done.

[wenyihu6]

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_index.go line 46 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

nit: hidden => IsNotVisible

Done.

[wenyihu6]

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)