evalengine: Handle zero dates correctly

This removes the different configuration for testing to ensure we have the same settings as production defaults and subsequently fixes the failing evalengine issues. Signed-off-by: Dirkjan Bussink <[email protected]>
vitessio · Nov 27, 2023 · f0c92b5 · f0c92b5
1 parent 36a52e9
commit f0c92b5
Show file tree

Hide file tree

Showing 23 changed files with 182 additions and 180 deletions.
diff --git a/config/mycnf/test-suite.cnf b/config/mycnf/test-suite.cnf
@@ -1,5 +1,5 @@
 # This sets some unsafe settings specifically for 
-# the test-suite which is currently MySQL 5.7 based
+# the test-suite which is currently MySQL 8.0 based
 # In future it should be renamed testsuite.cnf
 
 innodb_buffer_pool_size = 32M
@@ -14,13 +14,6 @@ key_buffer_size = 2M
 sync_binlog=0
 innodb_doublewrite=0
 
-# These two settings are required for the testsuite to pass, 
-# but enabling them does not spark joy. They should be removed
-# in the future. See:
-# https://github.com/vitessio/vitess/issues/5396
-
-sql_mode = STRICT_TRANS_TABLES
-
 # set a short heartbeat interval in order to detect failures quickly
 slave_net_timeout = 4
 # Disabling `super-read-only`. `test-suite` is mainly used for `vttestserver`. Since `vttestserver` uses a single MySQL for primary and replicas,

diff --git a/go/vt/vtgate/engine/concatenate.go b/go/vt/vtgate/engine/concatenate.go
@@ -105,7 +105,7 @@ func (c *Concatenate) TryExecute(ctx context.Context, vcursor VCursor, bindVars
 	err = c.coerceAndVisitResults(res, fields, func(result *sqltypes.Result) error {
 		rows = append(rows, result.Rows...)
 		return nil
-	})
+	}, vcursor.AllowZeroDate())
 	if err != nil {
 		return nil, err
 	}
@@ -116,7 +116,7 @@ func (c *Concatenate) TryExecute(ctx context.Context, vcursor VCursor, bindVars
 	}, nil
 }
 
-func (c *Concatenate) coerceValuesTo(row sqltypes.Row, fields []*querypb.Field) error {
+func (c *Concatenate) coerceValuesTo(row sqltypes.Row, fields []*querypb.Field, allowZeroDate bool) error {
 	if len(row) != len(fields) {
 		return errWrongNumberOfColumnsInSelect
 	}
@@ -126,7 +126,7 @@ func (c *Concatenate) coerceValuesTo(row sqltypes.Row, fields []*querypb.Field)
 			continue
 		}
 		if fields[i].Type != value.Type() {
-			newValue, err := evalengine.CoerceTo(value, fields[i].Type)
+			newValue, err := evalengine.CoerceTo(value, fields[i].Type, allowZeroDate)
 			if err != nil {
 				return err
 			}
@@ -228,16 +228,17 @@ func (c *Concatenate) sequentialExec(ctx context.Context, vcursor VCursor, bindV
 
 // TryStreamExecute performs a streaming exec.
 func (c *Concatenate) TryStreamExecute(ctx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable, _ bool, callback func(*sqltypes.Result) error) error {
+	allowZeroDate := vcursor.AllowZeroDate()
 	if vcursor.Session().InTransaction() {
 		// as we are in a transaction, we need to execute all queries inside a single connection,
 		// which holds the single transaction we have
-		return c.sequentialStreamExec(ctx, vcursor, bindVars, callback)
+		return c.sequentialStreamExec(ctx, vcursor, bindVars, callback, allowZeroDate)
 	}
 	// not in transaction, so execute in parallel.
-	return c.parallelStreamExec(ctx, vcursor, bindVars, callback)
+	return c.parallelStreamExec(ctx, vcursor, bindVars, callback, allowZeroDate)
 }
 
-func (c *Concatenate) parallelStreamExec(inCtx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable, in func(*sqltypes.Result) error) error {
+func (c *Concatenate) parallelStreamExec(inCtx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable, in func(*sqltypes.Result) error, allowZeroDate bool) error {
 	// Scoped context; any early exit triggers cancel() to clean up ongoing work.
 	ctx, cancel := context.WithCancel(inCtx)
 	defer cancel()
@@ -271,7 +272,7 @@ func (c *Concatenate) parallelStreamExec(inCtx context.Context, vcursor VCursor,
 		// Apply type coercion if needed.
 		if needsCoercion {
 			for _, row := range res.Rows {
-				if err := c.coerceValuesTo(row, fields); err != nil {
+				if err := c.coerceValuesTo(row, fields, allowZeroDate); err != nil {
 					return err
 				}
 			}
@@ -340,7 +341,7 @@ func (c *Concatenate) parallelStreamExec(inCtx context.Context, vcursor VCursor,
 	return wg.Wait()
 }
 
-func (c *Concatenate) sequentialStreamExec(ctx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable, callback func(*sqltypes.Result) error) error {
+func (c *Concatenate) sequentialStreamExec(ctx context.Context, vcursor VCursor, bindVars map[string]*querypb.BindVariable, callback func(*sqltypes.Result) error, allowZeroDate bool) error {
 	// all the below fields ensure that the fields are sent only once.
 	results := make([][]*sqltypes.Result, len(c.Sources))
 
@@ -374,7 +375,7 @@ func (c *Concatenate) sequentialStreamExec(ctx context.Context, vcursor VCursor,
 		return err
 	}
 	for _, res := range results {
-		if err = c.coerceAndVisitResults(res, fields, callback); err != nil {
+		if err = c.coerceAndVisitResults(res, fields, callback, allowZeroDate); err != nil {
 			return err
 		}
 	}
@@ -386,6 +387,7 @@ func (c *Concatenate) coerceAndVisitResults(
 	res []*sqltypes.Result,
 	fields []*querypb.Field,
 	callback func(*sqltypes.Result) error,
+	allowZeroDate bool,
 ) error {
 	for _, r := range res {
 		if len(r.Rows) > 0 &&
@@ -402,7 +404,7 @@ func (c *Concatenate) coerceAndVisitResults(
 		}
 		if needsCoercion {
 			for _, row := range r.Rows {
-				err := c.coerceValuesTo(row, fields)
+				err := c.coerceValuesTo(row, fields, allowZeroDate)
 				if err != nil {
 					return err
 				}

diff --git a/go/vt/vtgate/engine/distinct.go b/go/vt/vtgate/engine/distinct.go
@@ -44,8 +44,9 @@ type (
 		Type  evalengine.Type
 	}
 	probeTable struct {
-		seenRows  map[evalengine.HashCode][]sqltypes.Row
-		checkCols []CheckCol
+		seenRows      map[evalengine.HashCode][]sqltypes.Row
+		checkCols     []CheckCol
+		allowZeroDate bool
 	}
 )
 
@@ -119,14 +120,14 @@ func (pt *probeTable) hashCodeForRow(inputRow sqltypes.Row) (evalengine.HashCode
 			return 0, vterrors.VT13001("index out of range in row when creating the DISTINCT hash code")
 		}
 		col := inputRow[checkCol.Col]
-		hashcode, err := evalengine.NullsafeHashcode(col, checkCol.Type.Collation(), col.Type())
+		hashcode, err := evalengine.NullsafeHashcode(col, checkCol.Type.Collation(), col.Type(), pt.allowZeroDate)
 		if err != nil {
 			if err != evalengine.UnsupportedCollationHashError || checkCol.WsCol == nil {
 				return 0, err
 			}
 			checkCol = checkCol.SwitchToWeightString()
 			pt.checkCols[i] = checkCol
-			hashcode, err = evalengine.NullsafeHashcode(inputRow[checkCol.Col], checkCol.Type.Collation(), col.Type())
+			hashcode, err = evalengine.NullsafeHashcode(inputRow[checkCol.Col], checkCol.Type.Collation(), col.Type(), pt.allowZeroDate)
 			if err != nil {
 				return 0, err
 			}

diff --git a/go/vt/vtgate/engine/fake_vcursor_test.go b/go/vt/vtgate/engine/fake_vcursor_test.go
@@ -135,6 +135,10 @@ func (t *noopVCursor) TimeZone() *time.Location {
 	return nil
 }
 
+func (t *noopVCursor) AllowZeroDate() bool {
+	return false
+}
+
 func (t *noopVCursor) ExecutePrimitive(ctx context.Context, primitive Primitive, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error) {
 	return primitive.TryExecute(ctx, t, bindVars, wantfields)
 }

diff --git a/go/vt/vtgate/engine/hash_join.go b/go/vt/vtgate/engine/hash_join.go
@@ -75,6 +75,7 @@ type (
 		lhsKey, rhsKey int
 		cols           []int
 		hasher         vthash.Hasher
+		allowZeroDate  bool
 	}
 
 	probeTableEntry struct {
@@ -283,7 +284,7 @@ func (pt *hashJoinProbeTable) addLeftRow(r sqltypes.Row) error {
 }
 
 func (pt *hashJoinProbeTable) hash(val sqltypes.Value) (vthash.Hash, error) {
-	err := evalengine.NullsafeHashcode128(&pt.hasher, val, pt.coll, pt.typ)
+	err := evalengine.NullsafeHashcode128(&pt.hasher, val, pt.coll, pt.typ, pt.allowZeroDate)
 	if err != nil {
 		return vthash.Hash{}, err
 	}

diff --git a/go/vt/vtgate/engine/primitive.go b/go/vt/vtgate/engine/primitive.go
@@ -88,6 +88,7 @@ type (
 
 		ConnCollation() collations.ID
 		TimeZone() *time.Location
+		AllowZeroDate() bool
 
 		ExecuteLock(ctx context.Context, rs *srvtopo.ResolvedShard, query *querypb.BoundQuery, lockFuncType sqlparser.LockingFuncType) (*sqltypes.Result, error)
 

diff --git a/go/vt/vtgate/evalengine/api_coerce.go b/go/vt/vtgate/evalengine/api_coerce.go
@@ -23,8 +23,8 @@ import (
 	"vitess.io/vitess/go/vt/vterrors"
 )
 
-func CoerceTo(value sqltypes.Value, typ sqltypes.Type) (sqltypes.Value, error) {
-	cast, err := valueToEvalCast(value, value.Type(), collations.Unknown)
+func CoerceTo(value sqltypes.Value, typ sqltypes.Type, allowZeroDate bool) (sqltypes.Value, error) {
+	cast, err := valueToEvalCast(value, value.Type(), collations.Unknown, allowZeroDate)
 	if err != nil {
 		return sqltypes.Value{}, err
 	}

diff --git a/go/vt/vtgate/evalengine/api_hash.go b/go/vt/vtgate/evalengine/api_hash.go
@@ -34,8 +34,8 @@ type HashCode = uint64
 
 // NullsafeHashcode returns an int64 hashcode that is guaranteed to be the same
 // for two values that are considered equal by `NullsafeCompare`.
-func NullsafeHashcode(v sqltypes.Value, collation collations.ID, coerceType sqltypes.Type) (HashCode, error) {
-	e, err := valueToEvalCast(v, coerceType, collation)
+func NullsafeHashcode(v sqltypes.Value, collation collations.ID, coerceType sqltypes.Type, allowZeroDate bool) (HashCode, error) {
+	e, err := valueToEvalCast(v, coerceType, collation, allowZeroDate)
 	if err != nil {
 		return 0, err
 	}
@@ -75,7 +75,7 @@ var ErrHashCoercionIsNotExact = vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "
 // for two values that are considered equal by `NullsafeCompare`.
 // This can be used to avoid having to do comparison checks after a hash,
 // since we consider the 128 bits of entropy enough to guarantee uniqueness.
-func NullsafeHashcode128(hash *vthash.Hasher, v sqltypes.Value, collation collations.ID, coerceTo sqltypes.Type) error {
+func NullsafeHashcode128(hash *vthash.Hasher, v sqltypes.Value, collation collations.ID, coerceTo sqltypes.Type, allowZeroDate bool) error {
 	switch {
 	case v.IsNull(), sqltypes.IsNull(coerceTo):
 		hash.Write16(hashPrefixNil)
@@ -97,7 +97,7 @@ func NullsafeHashcode128(hash *vthash.Hasher, v sqltypes.Value, collation collat
 		case v.IsText(), v.IsBinary():
 			f, _ = fastparse.ParseFloat64(v.RawStr())
 		default:
-			return nullsafeHashcode128Default(hash, v, collation, coerceTo)
+			return nullsafeHashcode128Default(hash, v, collation, coerceTo, allowZeroDate)
 		}
 		if err != nil {
 			return err
@@ -137,7 +137,7 @@ func NullsafeHashcode128(hash *vthash.Hasher, v sqltypes.Value, collation collat
 			}
 			neg = i < 0
 		default:
-			return nullsafeHashcode128Default(hash, v, collation, coerceTo)
+			return nullsafeHashcode128Default(hash, v, collation, coerceTo, allowZeroDate)
 		}
 		if err != nil {
 			return err
@@ -180,7 +180,7 @@ func NullsafeHashcode128(hash *vthash.Hasher, v sqltypes.Value, collation collat
 				u, err = uint64(fval), nil
 			}
 		default:
-			return nullsafeHashcode128Default(hash, v, collation, coerceTo)
+			return nullsafeHashcode128Default(hash, v, collation, coerceTo, allowZeroDate)
 		}
 		if err != nil {
 			return err
@@ -223,20 +223,20 @@ func NullsafeHashcode128(hash *vthash.Hasher, v sqltypes.Value, collation collat
 			fval, _ := fastparse.ParseFloat64(v.RawStr())
 			dec = decimal.NewFromFloat(fval)
 		default:
-			return nullsafeHashcode128Default(hash, v, collation, coerceTo)
+			return nullsafeHashcode128Default(hash, v, collation, coerceTo, allowZeroDate)
 		}
 		hash.Write16(hashPrefixDecimal)
 		dec.Hash(hash)
 	default:
-		return nullsafeHashcode128Default(hash, v, collation, coerceTo)
+		return nullsafeHashcode128Default(hash, v, collation, coerceTo, allowZeroDate)
 	}
 	return nil
 }
 
-func nullsafeHashcode128Default(hash *vthash.Hasher, v sqltypes.Value, collation collations.ID, coerceTo sqltypes.Type) error {
+func nullsafeHashcode128Default(hash *vthash.Hasher, v sqltypes.Value, collation collations.ID, coerceTo sqltypes.Type, allowZeroDate bool) error {
 	// Slow path to handle all other types. This uses the generic
 	// logic for value casting to ensure we match MySQL here.
-	e, err := valueToEvalCast(v, coerceTo, collation)
+	e, err := valueToEvalCast(v, coerceTo, collation, allowZeroDate)
 	if err != nil {
 		return err
 	}

diff --git a/go/vt/vtgate/evalengine/api_hash_test.go b/go/vt/vtgate/evalengine/api_hash_test.go
@@ -56,10 +56,10 @@ func TestHashCodes(t *testing.T) {
 			require.NoError(t, err)
 			require.Equalf(t, tc.equal, cmp == 0, "got %v %s %v (expected %s)", tc.static, equality(cmp == 0).Operator(), tc.dynamic, equality(tc.equal))
 
-			h1, err := NullsafeHashcode(tc.static, collations.CollationUtf8mb4ID, tc.static.Type())
+			h1, err := NullsafeHashcode(tc.static, collations.CollationUtf8mb4ID, tc.static.Type(), false)
 			require.NoError(t, err)
 
-			h2, err := NullsafeHashcode(tc.dynamic, collations.CollationUtf8mb4ID, tc.static.Type())
+			h2, err := NullsafeHashcode(tc.dynamic, collations.CollationUtf8mb4ID, tc.static.Type(), false)
 			require.ErrorIs(t, err, tc.err)
 
 			assert.Equalf(t, tc.equal, h1 == h2, "HASH(%v) %s HASH(%v) (expected %s)", tc.static, equality(h1 == h2).Operator(), tc.dynamic, equality(tc.equal))
@@ -82,9 +82,9 @@ func TestHashCodesRandom(t *testing.T) {
 		typ, err := coerceTo(v1.Type(), v2.Type())
 		require.NoError(t, err)
 
-		hash1, err := NullsafeHashcode(v1, collation, typ)
+		hash1, err := NullsafeHashcode(v1, collation, typ, false)
 		require.NoError(t, err)
-		hash2, err := NullsafeHashcode(v2, collation, typ)
+		hash2, err := NullsafeHashcode(v2, collation, typ, false)
 		require.NoError(t, err)
 		if cmp == 0 {
 			equal++
@@ -142,11 +142,11 @@ func TestHashCodes128(t *testing.T) {
 			require.Equalf(t, tc.equal, cmp == 0, "got %v %s %v (expected %s)", tc.static, equality(cmp == 0).Operator(), tc.dynamic, equality(tc.equal))
 
 			hasher1 := vthash.New()
-			err = NullsafeHashcode128(&hasher1, tc.static, collations.CollationUtf8mb4ID, tc.static.Type())
+			err = NullsafeHashcode128(&hasher1, tc.static, collations.CollationUtf8mb4ID, tc.static.Type(), false)
 			require.NoError(t, err)
 
 			hasher2 := vthash.New()
-			err = NullsafeHashcode128(&hasher2, tc.dynamic, collations.CollationUtf8mb4ID, tc.static.Type())
+			err = NullsafeHashcode128(&hasher2, tc.dynamic, collations.CollationUtf8mb4ID, tc.static.Type(), false)
 			require.ErrorIs(t, err, tc.err)
 
 			h1 := hasher1.Sum128()
@@ -172,10 +172,10 @@ func TestHashCodesRandom128(t *testing.T) {
 		require.NoError(t, err)
 
 		hasher1 := vthash.New()
-		err = NullsafeHashcode128(&hasher1, v1, collation, typ)
+		err = NullsafeHashcode128(&hasher1, v1, collation, typ, false)
 		require.NoError(t, err)
 		hasher2 := vthash.New()
-		err = NullsafeHashcode128(&hasher2, v2, collation, typ)
+		err = NullsafeHashcode128(&hasher2, v2, collation, typ, false)
 		require.NoError(t, err)
 		if cmp == 0 {
 			equal++

diff --git a/go/vt/vtgate/evalengine/compiler.go b/go/vt/vtgate/evalengine/compiler.go
@@ -30,9 +30,10 @@ import (
 type frame func(env *ExpressionEnv) int
 
 type compiler struct {
-	collation    collations.ID
-	dynamicTypes []ctype
-	asm          assembler
+	collation     collations.ID
+	dynamicTypes  []ctype
+	asm           assembler
+	allowZeroDate bool
 }
 
 type CompilerLog interface {
@@ -257,7 +258,7 @@ func (c *compiler) compileToDate(doct ctype, offset int) ctype {
 	case sqltypes.Date:
 		return doct
 	default:
-		c.asm.Convert_xD(offset)
+		c.asm.Convert_xD(offset, c.allowZeroDate)
 	}
 	return ctype{Type: sqltypes.Date, Col: collationBinary, Flag: flagNullable}
 }
@@ -268,7 +269,7 @@ func (c *compiler) compileToDateTime(doct ctype, offset, prec int) ctype {
 		c.asm.Convert_tp(offset, prec)
 		return doct
 	default:
-		c.asm.Convert_xDT(offset, prec)
+		c.asm.Convert_xDT(offset, prec, c.allowZeroDate)
 	}
 	return ctype{Type: sqltypes.Datetime, Size: int32(prec), Col: collationBinary, Flag: flagNullable}
 }