sql: prevent crash on range scan on virtual index

[jordanlewis]

Fixes #56440.

Previously, the database would panic when a user performed a range scan
against a virtual table that had a virtual index. For example, the
queries:

SELECT * FROM pg_catalog.pg_type WHERE oid IN (19, 20)
SELECT * FROM pg_catalog.pg_class WHERE oid BETWEEN 20 AND 50

would cause this crash, since both of those tables have virtual indexes
defined on their oid columns.

This crash is now corrected.

Release note (bug fix): prevent a crash, introduced in the 20.2 series,
caused by range scans over virtual tables with virtual indexes.

[cockroach-teamcity]

This change is 

[RaduBerinde]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @RaduBerinde)

---

pkg/sql/virtual_schema.go, line 572 at r1 (raw file):

		// from the end and use them as a filter for the remaining rows of the
		// table.
		currentConstraint := idxConstraint.Spans.Count() - 1

[nit] currentConstraintSpan or currentSpan

---

pkg/sql/virtual_schema.go, line 589 at r1 (raw file):

			// constraint span's value.
			found, err := virtualIndex.populate(ctx, constraintDatum, p, dbDesc,
				addRowIfPassesFilter(idxConstraint))

This is pretty strange, we use the entire constraint each time except during the fallback. Why don't we want to just check against the current Span rather than the entire constraint? Well.. I'm not sure I understand why we need to check at all, doesn't constraintDatum guarantee the results will be inside the single-key span?

[jordanlewis]

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

---

pkg/sql/virtual_schema.go, line 589 at r1 (raw file):

Previously, RaduBerinde wrote…

This is pretty strange, we use the entire constraint each time except during the fallback. Why don't we want to just check against the current Span rather than the entire constraint? Well.. I'm not sure I understand why we need to check at all, doesn't constraintDatum guarantee the results will be inside the single-key span?

You're right, I don't know why I did it this way. We want to cherry pick this, so I'm not going to change the behavior in this PR in case something more subtle goes wrong, but I'll open up another PR for it.

[RaduBerinde]

Reviewable status: complete! 1 of 0 LGTMs obtained

[jordanlewis]

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)

[roncrdb]

What release will include this fix?

[jordanlewis]

It looks like I forgot to backport this to release-20.2, unfortunately. It will be in 20.2.2 or 20.2.3.

[roncrdb]

@jordanlewis Thanks for the update.