sql: introduce index_recommendations_enabled session variable

[nehageorge]

This commit introduces a session variable to control whether or not index
recommendations are shown at the bottom of the EXPLAIN plan. If
index_recommendations_enabled is set to true, index recommendations
are shown. Otherwise, they are not shown. This variable defaults to true.

Release note (sql change): Index recommendations can be omitted from
the EXPLAIN plan if the index_recommendations_enabled session variable
is set to false.

NOTE: This PR only concerns the final commit. Other commits will be reviewed and merged separately.

[cockroach-teamcity]

This change is 

[rytaft]

Reviewed 6 of 6 files at r1, 4 of 4 files at r2, 25 of 57 files at r3, 10 of 10 files at r4, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @nehageorge and @rytaft)

---

-- commits, line 46 at r4:
nit: feels like it would be more natural (and more consistent with other settings) to call this index_recommendations_enabled and have it default to true. (I see we have a couple of settings that do start with disable, but I think that was a mistake)

[nehageorge]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rytaft)

---

-- commits, line 46 at r4:

Previously, rytaft (Rebecca Taft) wrote…

nit: feels like it would be more natural (and more consistent with other settings) to call this index_recommendations_enabled and have it default to true. (I see we have a couple of settings that do start with disable, but I think that was a mistake)

Makes sense. Done.

[rytaft]

last commit

Reviewed 10 of 10 files at r5, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @rytaft)

[mgartner]

Reviewed 5 of 10 files at r5, 65 of 65 files at r6, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @nehageorge)

[mgartner]

I wonder if we want a cluster setting so users can turn it off for all sessions.

[rytaft]

I wonder if we want a cluster setting so users can turn it off for all sessions.

I think we're moving away from the sql.defaults cluster settings due to issues with supporting them for multi-tenancy. @RaduBerinde can probably give more context.

[nehageorge]

Good point @mgartner. Do you think it would make sense to have both a cluster setting and a session setting?

[nehageorge]

I wonder if we want a cluster setting so users can turn it off for all sessions.

I think we're moving away from the sql.defaults cluster settings due to issues with supporting them for multi-tenancy. @RaduBerinde can probably give more context.

Disregard my most recent comment as per this comment.

[nehageorge]

TFTRs!

bors r+

[RaduBerinde]

I wonder if we want a cluster setting so users can turn it off for all sessions.

I think we're moving away from the sql.defaults cluster settings due to issues with supporting them for multi-tenancy. @RaduBerinde can probably give more context.

Not really multitenancy related, we're just adding infrastructure for setting per-role or per-db defaults for session settings, which makes these cluster settings unnecessary.

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[mgartner]

Not really multitenancy related, we're just adding infrastructure for setting per-role or per-db defaults for session settings, which makes these cluster settings unnecessary.

Makes sense. Let's merge this as-is, and @nehageorge can make an issue to track making a per-db/role default for this setting once the infrastructure is in place.

[craig]

Build succeeded:

• GitHub CI (Cockroach)

[nehageorge]

Created an issue for the future per-role/per-db setting here: #73918.