execbuilder: re-enable skipped tests

[mgartner]

This commit de-flakes tests that were previously disabled in #75962 by
setting column families explicitly. I originally thought these
execbuilder tests were flaky because there were two query plans with the
same cost, but the flakiness was actually the result of randomized
families in logic tests.

Fixes #75960

Release note: None

[cockroach-teamcity]

This change is 

[cucaroach]

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

[cucaroach]

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

[rharding6373]

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

---

pkg/sql/opt/exec/execbuilder/testdata/hash_sharded_index, line 203 at r1 (raw file):

  a INT8 PRIMARY KEY USING HASH WITH BUCKET_COUNT = 8,
  b INT8 NOT NULL,
  FAMILY (a, b)

Drive-by question: So without specifying the column family, we will store all columns as a single family but randomly decide whether to store them in (a,b) order or (b,a) order?

[mgartner]

---

pkg/sql/opt/exec/execbuilder/testdata/hash_sharded_index, line 203 at r1 (raw file):

Previously, rharding6373 (Rachael Harding) wrote…

Drive-by question: So without specifying the column family, we will store all columns as a single family but randomly decide whether to store them in (a,b) order or (b,a) order?

The logic tests use a column family mutator here:

cockroach/pkg/sql/logictest/logic.go

Line 2791 in 73b6819

execSQL, changed := randgen.ApplyString(t.rng, stmt.sql, randgen.ColumnFamilyMutator)

This mutates CREATE TABLE statements by randomly splitting up columns into a random number of families if there are no families explicitly specified in the CREATE TABLE statement:

cockroach/pkg/sql/randgen/mutator.go

Lines 821 to 870 in 8b5f53c

	func columnFamilyMutator(rng *rand.Rand, stmt tree.Statement) (changed bool) {
	ast, ok := stmt.(*tree.CreateTable)
	if !ok {
	return false
	}
	var columns []tree.Name
	for _, def := range ast.Defs {
	switch def := def.(type) {
	case *tree.FamilyTableDef:
	return false
	case *tree.ColumnTableDef:
	if def.HasColumnFamily() {
	return false
	}
	if !def.Computed.Virtual {
	columns = append(columns, def.Name)
	}
	}
	}
	if len(columns) <= 1 {
	return false
	}
	// Any columns not specified in column families
	// are auto assigned to the first family, so
	// there's no requirement to exhaust columns here.
	rng.Shuffle(len(columns), func(i, j int) {
	columns[i], columns[j] = columns[j], columns[i]
	})
	fd := &tree.FamilyTableDef{}
	for {
	if len(columns) == 0 {
	if len(fd.Columns) > 0 {
	ast.Defs = append(ast.Defs, fd)
	}
	break
	}
	fd.Columns = append(fd.Columns, columns[0])
	columns = columns[1:]
	// 50% chance to make a new column family.
	if rng.Intn(2) != 0 {
	ast.Defs = append(ast.Defs, fd)
	fd = &tree.FamilyTableDef{}
	}
	}
	return true
	}

This is only happens for logic tests. In production, column families are always deterministic. I believe the default is to include all columns in a single family.

[mgartner]

bors r+

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build failed:

• GitHub CI (Cockroach)

[mgartner]

Looks like a flake.

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)