release-22.2: stats: fix buckets for INT2 and INT4

[blathers-crl]

Backport 1/1 commits from #88083 on behalf of @yuzefovich.

/cc @cockroachdb/release

---

Previously, if we needed to create "outer" histogram buckets (which is the case when minimum and maximum values in the column weren't sampled yet they contributed to the distinct count) for INT2 and INT4 types, we would use the values that exceeded the supported range for those types. This could lead to incorrect estimation later on when those "outer" buckets are used during the costing as well as the histograms would need to be manually edited to be injected. This is now fixed by handling these two types separately.

Fixes: #76887.

Release note: None

---

Release justification: bug fix.

[blathers-crl]

Thanks for opening a backport.

Please check the backport criteria before merging:

• Patches should only be created for serious issues or test-only changes.
• Patches should not break backwards-compatibility.
• Patches should change as little code as possible.
• Patches should not change on-disk formats or node communication protocols.
• Patches should not add new functionality.
• Patches must not add, edit, or otherwise modify cluster versions; or add version gates.

If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.

• There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
• The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
• New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
• The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

• What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
• Will this work in a cluster of mixed patch versions? Did we test that?
• If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

[cockroach-teamcity]

This change is 

[yuzefovich]

Looks like TestAdjustCounts/random fails under race, so I'll need to take a look at that first, before merging this.

[michae2]

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

[yuzefovich]

Squashed #88261 into this commit, will wait for @rytaft to approve as EM.

[rytaft]

Reviewed 9 of 10 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @blathers-crl[bot] and @yuzefovich)

---

pkg/sql/stats/histogram.go line 365 at r2 (raw file):

		switch t.Width() {
		case 16:
			if i == math.MinInt16 {

What if i is less than MinInt16? Can that ever happen? Same question below for 32 and for MaxInt16 / MaxInt32 also

(sorry I didn't review the PR on master -- guessing this is fine, but just wanted to check...)

[yuzefovich]

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

---

pkg/sql/stats/histogram.go line 365 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

What if i is less than MinInt16? Can that ever happen? Same question below for 32 and for MaxInt16 / MaxInt32 also

(sorry I didn't review the PR on master -- guessing this is fine, but just wanted to check...)

Hm, this shouldn't really happen because it would imply that we sampled such a value which would mean that we didn't perform the out of bounds check on the write path. However, I'm not confident that we actually do those checks in all places, so it'd be worth being conservative here. I'll open up a PR for this - thanks!

[rytaft]

Reviewable status: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @yuzefovich)

---

pkg/sql/stats/histogram.go line 365 at r2 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

Hm, this shouldn't really happen because it would imply that we sampled such a value which would mean that we didn't perform the out of bounds check on the write path. However, I'm not confident that we actually do those checks in all places, so it'd be worth being conservative here. I'll open up a PR for this - thanks!

Thanks!

[yuzefovich]

Squashed #88300 into this commit.