opt: avoid generating trivial constrained scans

[DrewKimball]

opt: extend constraint Contains method to handle common prefix

This patch extends the logic of Constraint.Contains to handle cases
when the columns of the left and right constraints differ beyond a common
prefix. The spans of one constraint can be compared with those of another
for containment when the following conditions are satisfied:

1. The common prefix between the left and right columns is non-empty.
2. Only one constraint can contain keys beyond the common prefix length.

These conditions ensure comparisons are valid by ensuring that comparison
only requires columns from the common prefix. Columns are only needed for
key comparisons up to the length of the shorter key, and the above conditions
guarantee that at least one key is always within the common prefix length.

Informs #114250

Release note: None

opt: avoid generating trivial constrained scans

This patch adds a check to the GenerateConstrainedScans optimizer rule
to prevent it from building trivial constrained scans that only use the
table's (non-selective) check constraints. This ensures that the resulting
plan will show a full scan as expected, and also prevents suboptimal index
joins in some cases.

Fixes #114250

Release note (sql change): The optimizer will no longer generate a
constrained scan that only uses filters from a check constraint. This
prevents cases where a constrained scan actually scans the entire table
because the constraints aren't selective.

[cockroach-teamcity]

This change is 

[mgartner]

Great find and fix!

Reviewed 4 of 4 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball and @rharding6373)

---

pkg/sql/opt/exec/execbuilder/testdata/explain line 2400 at r2 (raw file):

      └── k:6 = a:1 [outer=(1,6), constraints=(/1: (/NULL - ]; /6: (/NULL - ]), fd=(1)==(6), (6)==(1)]

# Regression test for #114250 - the EXPLAIN output should indicate a full scan.

nit: Not sure you really need this test. Either way, the message is confusing IMO since the fix doesn't have much to do about the EXPLAIN output—it's fixing a nonsense plan.

---

pkg/sql/opt/xform/testdata/rules/select line 2599 at r2 (raw file):

# scans the whole table using the table's check constraints.
exec-ddl
CREATE TABLE t114250 (x INT PRIMARY KEY USING HASH, y INT NOT NULL);

nit: it might be nice to have a test case with an explicit check constraint too.

---

pkg/sql/opt/constraint/constraint_test.go line 276 at r1 (raw file):

			"/1/4: [/1 - /1]",
			"/1: [/1 - /1]",
			true,

nit: missing field names here. You can fix the existing test cases above without field names too if you'd like.

[DrewKimball]

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

---

pkg/sql/opt/exec/execbuilder/testdata/explain line 2400 at r2 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

nit: Not sure you really need this test. Either way, the message is confusing IMO since the fix doesn't have much to do about the EXPLAIN output—it's fixing a nonsense plan.

Done.

---

pkg/sql/opt/xform/testdata/rules/select line 2599 at r2 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

nit: it might be nice to have a test case with an explicit check constraint too.

Good idea, done. Actually, it turns out you can't use USING HASH for optimizer tests anyway. I'm not sure why it worked before, but it seems like it wasn't actually testing what I wanted it to.

---

pkg/sql/opt/constraint/constraint_test.go line 276 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

nit: missing field names here. You can fix the existing test cases above without field names too if you'd like.

Oh, good catch. Fixed it.

[DrewKimball]

TFTR!

bors r+

[rharding6373]

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

---

pkg/sql/opt/constraint/columns.go line 101 at r3 (raw file):

	}
	length := 1
	for i := 0; i < len(c.otherCols) && i < len(other.otherCols); i++ {

TIL about this nice alternative to using two variables to iterate over slices. Cool trick!

[craig]

Build failed (retrying...):

• Bazel Essential CI (Cockroach)

[craig]

Build failed (retrying...):

• Bazel Essential CI (Cockroach)

[mgartner]

Reviewed 8 of 8 files at r3, 8 of 8 files at r4, all commit messages.
Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @DrewKimball)

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)