sql: refactor MetadataUpdater to properly share txns/collections

[fqazi]

Previously, the code that we added for the metadata updater, which
is used to update descriptor metadata had code paths that could
end up creating new transactions/collections. This was problematic because
those updates were not executing under the same user transaction.
This patch simplifies the metadata updater logic to no longer
have a factory and share collections/transactions properly.

Release note: None

[cockroach-teamcity]

This change is 

[ajwerner]

Great change, see how you feel about my one suggestion. Otherwise,

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

---

pkg/sql/descmetadata/metadata_updater.go line 56 at r1 (raw file):

	// code modifying this metadata would use a circular executor that would ignore
	// any settings set later on. We will intentionally, unset problematic settings
	// here.

You could just store the ieFactory in the struct and make a new one each time you need to use the ie?

Code quote:

	// Unfortunately, we can't use the session data unmodified, previously the
	// code modifying this metadata would use a circular executor that would ignore
	// any settings set later on. We will intentionally, unset problematic settings
	// here.

[fqazi]

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

---

pkg/sql/descmetadata/metadata_updater.go line 56 at r1 (raw file):

Previously, ajwerner wrote…

You could just store the ieFactory in the struct and make a new one each time you need to use the ie?

Modified to store the ieFactory and things required to construct one

[fqazi]

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)

[yuzefovich]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

---

pkg/sql/descmetadata/metadata_updater.go line 60 at r2 (raw file):

	// here.
	modifiedSessionData := sessionData.Clone()
	modifiedSessionData.ExperimentalDistSQLPlanningMode = sessiondatapb.ExperimentalDistSQLPlanningOn

Unexpected log messages during a test debugging brought me here: why are we setting ExperimentalDistSQLPlanningOn? That setting doesn't support many things, so many queries will perform a somewhat redundant work that will be discarded later due to unsupported features. If we need to override this particular setting (why do we need though?), we should use ExperimentalDistSQLPlanningOff.