release-21.1: sqlsmith;mutations: blitz to fix Postgres compare_test bugs

pkg/compose/compare/compare/compare_test.go

@@ -27,7 +27,9 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/internal/sqlsmith"
 	"github.com/cockroachdb/cockroach/pkg/sql/mutations"
 	"github.com/cockroachdb/cockroach/pkg/sql/rowenc"
+	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
+	"github.com/jackc/pgx/v4"
 )
 
 var (
@@ -45,6 +47,9 @@ func TestCompare(t *testing.T) {
 			init: []string{
 				"drop schema if exists public cascade",
 				"create schema public",
+				"CREATE EXTENSION IF NOT EXISTS postgis",
+				"CREATE EXTENSION IF NOT EXISTS postgis_topology",
+				"CREATE EXTENSION IF NOT EXISTS fuzzystrmatch;",
 			},
 		},
 		"cockroach1": {
@@ -64,9 +69,10 @@ func TestCompare(t *testing.T) {
 	}
 	configs := map[string]testConfig{
 		"postgres": {
-			setup:         sqlsmith.Setups["rand-tables"],
-			setupMutators: []rowenc.Mutator{mutations.PostgresCreateTableMutator},
-			opts:          []sqlsmith.SmitherOption{sqlsmith.PostgresMode()},
+			setup:           sqlsmith.Setups["rand-tables"],
+			setupMutators:   []rowenc.Mutator{mutations.PostgresCreateTableMutator},
+			opts:            []sqlsmith.SmitherOption{sqlsmith.PostgresMode()},
+			ignoreSQLErrors: true,
 			conns: []testConn{
 				{
 					name:     "cockroach1",
@@ -79,8 +85,9 @@ func TestCompare(t *testing.T) {
 			},
 		},
 		"mutators": {
-			setup: sqlsmith.Setups["rand-tables"],
-			opts:  []sqlsmith.SmitherOption{sqlsmith.CompareMode()},
+			setup:           sqlsmith.Setups["rand-tables"],
+			opts:            []sqlsmith.SmitherOption{sqlsmith.CompareMode()},
+			ignoreSQLErrors: true,
 			conns: []testConn{
 				{
 					name:     "cockroach1",
@@ -102,14 +109,28 @@ func TestCompare(t *testing.T) {
 	}
 
 	ctx := context.Background()
+
+	// docker-compose requires us to manually check for when a container
+	// is ready to receive connections.
+	// See https://docs.docker.com/compose/startup-order/
+	for name, uri := range uris {
+		t.Logf("Checking connection to: %s", name)
+		testutils.SucceedsSoon(t, func() error {
+			_, err := pgx.Connect(ctx, uri.addr)
+			return err
+		})
+	}
+
 	for confName, config := range configs {
 		t.Run(confName, func(t *testing.T) {
+			t.Logf("starting test: %s", confName)
 			rng, _ := randutil.NewPseudoRand()
 			setup := config.setup(rng)
 			setup, _ = mutations.ApplyString(rng, setup, config.setupMutators...)
 
 			conns := map[string]cmpconn.Conn{}
 			for _, testCn := range config.conns {
+				t.Logf("initializing connection: %s", testCn.name)
 				uri, ok := uris[testCn.name]
 				if !ok {
 					t.Fatalf("bad connection name: %s", testCn.name)
@@ -140,13 +161,13 @@ func TestCompare(t *testing.T) {
 			for {
 				select {
 				case <-until:
+					t.Logf("done with test: %s", confName)
 					return
 				default:
 				}
 				query := smither.Generate()
-				query, _ = mutations.ApplyString(rng, query, mutations.PostgresMutator)
 				if err := cmpconn.CompareConns(
-					ctx, time.Second*30, conns, "" /* prep */, query, true, /* ignoreSQLErrors */
+					ctx, time.Second*30, conns, "" /* prep */, query, config.ignoreSQLErrors,
 				); err != nil {
 					path := filepath.Join(*flagArtifacts, confName+".log")
 					if err := ioutil.WriteFile(path, []byte(err.Error()), 0666); err != nil {
@@ -168,10 +189,11 @@ func TestCompare(t *testing.T) {
 }
 
 type testConfig struct {
-	opts          []sqlsmith.SmitherOption
-	conns         []testConn
-	setup         sqlsmith.Setup
-	setupMutators []rowenc.Mutator
+	opts            []sqlsmith.SmitherOption
+	conns           []testConn
+	setup           sqlsmith.Setup
+	setupMutators   []rowenc.Mutator
+	ignoreSQLErrors bool
 }
 
 type testConn struct {

pkg/compose/compare/docker-compose.yml

@@ -1,7 +1,7 @@
 version: '3'
 services:
   postgres:
-    image: postgis/postgis:11-3.0
+    image: postgis/postgis:13-3.1
     environment:
       - POSTGRES_INITDB_ARGS=--locale=C
       - POSTGRES_HOST_AUTH_METHOD=trust

pkg/internal/sqlsmith/relational.go

@@ -560,7 +560,13 @@ func (s *Smither) makeSelectClause(
 			// either reference a group by column or a non-group by
 			// column in an aggregate function. It's also possible
 			// the where and order by exprs are not correct.
-			groupByRefs := fromRefs.extend()
+			var groupByRefs colRefs
+			for _, r := range fromRefs {
+				if s.postgres && r.typ.Family() == types.Box2DFamily {
+					continue
+				}
+				groupByRefs = append(groupByRefs, r)
+			}
 			s.rnd.Shuffle(len(groupByRefs), func(i, j int) {
 				groupByRefs[i], groupByRefs[j] = groupByRefs[j], groupByRefs[i]
 			})
@@ -670,8 +676,14 @@ func makeSelect(s *Smither) (tree.Statement, bool) {
 	if s.outputSort {
 		order := make(tree.OrderBy, len(refs))
 		for i, r := range refs {
+			var expr tree.Expr = r.item
+			// PostGIS cannot order box2d types, so we cast to string so the
+			// order is deterministic.
+			if s.postgres && r.typ.Family() == types.Box2DFamily {
+				expr = &tree.CastExpr{Expr: r.item, Type: types.String}
+			}
 			order[i] = &tree.Order{
-				Expr:       r.item,
+				Expr:       expr,
 				NullsOrder: tree.NullsFirst,
 			}
 		}
@@ -1106,6 +1118,10 @@ func (s *Smither) makeOrderBy(refs colRefs) tree.OrderBy {
 		if ref.typ.Family() == types.JsonFamily {
 			continue
 		}
+		// PostGIS cannot order box2d types.
+		if s.postgres && ref.typ.Family() == types.Box2DFamily {
+			continue
+		}
 		ob = append(ob, &tree.Order{
 			Expr:      ref.item,
 			Direction: s.randDirection(),

pkg/internal/sqlsmith/scalar.go

@@ -297,6 +297,16 @@ func makeBinOp(s *Smither, typ *types.T, refs colRefs) (tree.TypedExpr, bool) {
 			return nil, false
 		}
 	}
+	if s.postgres {
+		if transform, needTransform := postgresBinOpTransformations[binOpTriple{
+			op.LeftType.Family(),
+			op.Operator,
+			op.RightType.Family(),
+		}]; needTransform {
+			op.LeftType = transform.leftType
+			op.RightType = transform.rightType
+		}
+	}
 	left := makeScalar(s, op.LeftType, refs)
 	right := makeScalar(s, op.RightType, refs)
 	return castType(
@@ -314,13 +324,38 @@ type binOpTriple struct {
 	right types.Family
 }
 
+type binOpOperands struct {
+	leftType  *types.T
+	rightType *types.T
+}
+
 var ignorePostgresBinOps = map[binOpTriple]bool{
 	// Integer division in cockroach returns a different type.
 	{types.IntFamily, tree.Div, types.IntFamily}: true,
 	// Float * date isn't exact.
 	{types.FloatFamily, tree.Mult, types.DateFamily}: true,
 	{types.DateFamily, tree.Mult, types.FloatFamily}: true,
 	{types.DateFamily, tree.Div, types.FloatFamily}:  true,
+
+	// Postgres does not have separate floor division operator.
+	{types.IntFamily, tree.FloorDiv, types.IntFamily}:         true,
+	{types.FloatFamily, tree.FloorDiv, types.FloatFamily}:     true,
+	{types.DecimalFamily, tree.FloorDiv, types.DecimalFamily}: true,
+	{types.DecimalFamily, tree.FloorDiv, types.IntFamily}:     true,
+	{types.IntFamily, tree.FloorDiv, types.DecimalFamily}:     true,
+
+	{types.FloatFamily, tree.Mod, types.FloatFamily}: true,
+}
+
+// For certain operations, Postgres is picky about the operand types.
+var postgresBinOpTransformations = map[binOpTriple]binOpOperands{
+	{types.IntFamily, tree.Plus, types.DateFamily}:          {types.Int4, types.Date},
+	{types.DateFamily, tree.Plus, types.IntFamily}:          {types.Date, types.Int4},
+	{types.IntFamily, tree.Minus, types.DateFamily}:         {types.Int4, types.Date},
+	{types.DateFamily, tree.Minus, types.IntFamily}:         {types.Date, types.Int4},
+	{types.JsonFamily, tree.JSONFetchVal, types.IntFamily}:  {types.Jsonb, types.Int4},
+	{types.JsonFamily, tree.JSONFetchText, types.IntFamily}: {types.Jsonb, types.Int4},
+	{types.JsonFamily, tree.Minus, types.IntFamily}:         {types.Jsonb, types.Int4},
 }
 
 func makeFunc(s *Smither, ctx Context, typ *types.T, refs colRefs) (tree.TypedExpr, bool) {
@@ -352,16 +387,19 @@ func makeFunc(s *Smither, ctx Context, typ *types.T, refs colRefs) (tree.TypedEx
 
 	args := make(tree.TypedExprs, 0)
 	for _, argTyp := range fn.overload.Types.Types() {
+		// Postgres is picky about having Int4 arguments instead of Int8.
+		if s.postgres && argTyp.Family() == types.IntFamily {
+			argTyp = types.Int4
+		}
 		var arg tree.TypedExpr
 		// If we're a GROUP BY or window function, try to choose a col ref for the arguments.
 		if class == tree.AggregateClass || class == tree.WindowClass {
 			var ok bool
 			arg, ok = makeColRef(s, argTyp, refs)
 			if !ok {
-				// If we can't find a col ref for our
-				// aggregate function, try again with
-				// a non-aggregate.
-				return makeFunc(s, emptyCtx, typ, refs)
+				// If we can't find a col ref for our aggregate function, just use a
+				// constant.
+				arg = makeConstExpr(s, typ, refs)
 			}
 		}
 		if arg == nil {

pkg/internal/sqlsmith/schema.go

@@ -277,6 +277,9 @@ ORDER BY
 		// All non virtual tables contain implicit system columns.
 		for i := range colinfo.AllSystemColumnDescs {
 			col := &colinfo.AllSystemColumnDescs[i]
+			if s.postgres && col.ID == colinfo.MVCCTimestampColumnID {
+				continue
+			}
 			currentCols = append(currentCols, &tree.ColumnTableDef{
 				Name: tree.Name(col.Name),
 				Type: col.Type,

pkg/internal/sqlsmith/sqlsmith.go

@@ -366,8 +366,21 @@ var PostgresMode = multiOption(
 	// Some func impls differ from postgres, so skip them here.
 	// #41709
 	IgnoreFNs("^sha"),
+	IgnoreFNs("^isnan"),
+	IgnoreFNs("^crc32c"),
+	IgnoreFNs("^fnv32a"),
+	IgnoreFNs("^experimental_"),
+	IgnoreFNs("^json_set"),
+	IgnoreFNs("^concat_agg"),
+	IgnoreFNs("^to_english"),
+	IgnoreFNs("^substr$"),
 	// We use e'XX' instead of E'XX' for hex strings, so ignore these.
 	IgnoreFNs("^quote"),
 	// We have some differences here with empty string and "default"; skip until fixed.
 	IgnoreFNs("^pg_collation_for"),
+	// Postgres does not have the `.*_escape` functions.
+	IgnoreFNs("_escape$"),
+	// Some spatial functions are CockroachDB-specific.
+	IgnoreFNs("st_.*withinexclusive$"),
+	IgnoreFNs("^postgis_.*_build_date"),
 )