expression: fix the issue that extracting `day_microsecond/day_second…

…/day_minute/day_hour` from `Time` type emits wrong result (#36297)

close #34998

expression/builtin_time.go

@@ -2565,36 +2565,31 @@ func (c *extractFunctionClass) getFunction(ctx sessionctx.Context, args []Expres
 		return nil, err
 	}
 
-	datetimeUnits := map[string]struct{}{
-		"DAY":             {},
-		"WEEK":            {},
-		"MONTH":           {},
-		"QUARTER":         {},
-		"YEAR":            {},
-		"DAY_MICROSECOND": {},
-		"DAY_SECOND":      {},
-		"DAY_MINUTE":      {},
-		"DAY_HOUR":        {},
-		"YEAR_MONTH":      {},
-	}
-	isDatetimeUnit := true
 	args[0] = WrapWithCastAsString(ctx, args[0])
-	if _, isCon := args[0].(*Constant); isCon {
-		unit, _, err1 := args[0].EvalString(ctx, chunk.Row{})
-		if err1 != nil {
-			return nil, err1
-		}
-		_, isDatetimeUnit = datetimeUnits[unit]
+	unit, _, err := args[0].EvalString(ctx, chunk.Row{})
+	if err != nil {
+		return nil, err
 	}
+	isClockUnit := types.IsClockUnit(unit)
+	isDateUnit := types.IsDateUnit(unit)
 	var bf baseBuiltinFunc
-	if isDatetimeUnit {
-		if args[1].GetType().EvalType() != types.ETString {
-			bf, err = newBaseBuiltinFuncWithTp(ctx, c.funcName, args, types.ETInt, types.ETString, types.ETDatetime)
+	if isClockUnit && isDateUnit {
+		// For unit DAY_MICROSECOND/DAY_SECOND/DAY_MINUTE/DAY_HOUR, the interpretation of the second argument depends on its evaluation type:
+		// 1. Datetime/timestamp/time are interchangeably interpreted as time. For example:
+		// extract(day_second from time('02:03:04')) = 20304
+		// extract(day_second from datetime('2001-01-01 02:03:04')) = 20304
+		// 2. Otherwise are interpreted as datetime. For example:
+		// extract(day_second from '2001-01-01 02:03:04') = 1020304
+		// extract(day_second from 20010101020304) = 1020304
+		// Note the heading 1 (the "day" portion) in results of the above two cases.
+		// They are why these units are special -
+		if args[1].GetType().EvalType() == types.ETDatetime || args[1].GetType().EvalType() == types.ETTimestamp || args[1].GetType().EvalType() == types.ETDuration {
+			bf, err = newBaseBuiltinFuncWithTp(ctx, c.funcName, args, types.ETInt, types.ETString, types.ETDuration)
 			if err != nil {
 				return nil, err
 			}
-			sig = &builtinExtractDatetimeSig{bf}
-			sig.setPbCode(tipb.ScalarFuncSig_ExtractDatetime)
+			sig = &builtinExtractDurationSig{bf}
+			sig.setPbCode(tipb.ScalarFuncSig_ExtractDuration)
 		} else {
 			bf, err = newBaseBuiltinFuncWithTp(ctx, c.funcName, args, types.ETInt, types.ETString, types.ETString)
 			if err != nil {
@@ -2604,13 +2599,22 @@ func (c *extractFunctionClass) getFunction(ctx sessionctx.Context, args []Expres
 			sig = &builtinExtractDatetimeFromStringSig{bf}
 			sig.setPbCode(tipb.ScalarFuncSig_ExtractDatetimeFromString)
 		}
-	} else {
+	} else if isClockUnit {
+		// Clock units interpret the second argument as time.
 		bf, err = newBaseBuiltinFuncWithTp(ctx, c.funcName, args, types.ETInt, types.ETString, types.ETDuration)
 		if err != nil {
 			return nil, err
 		}
 		sig = &builtinExtractDurationSig{bf}
 		sig.setPbCode(tipb.ScalarFuncSig_ExtractDuration)
+	} else {
+		// Date units interpret the second argument as datetime.
+		bf, err = newBaseBuiltinFuncWithTp(ctx, c.funcName, args, types.ETInt, types.ETString, types.ETDatetime)
+		if err != nil {
+			return nil, err
+		}
+		sig = &builtinExtractDatetimeSig{bf}
+		sig.setPbCode(tipb.ScalarFuncSig_ExtractDatetime)
 	}
 	return sig, nil
 }
@@ -2625,7 +2629,7 @@ func (b *builtinExtractDatetimeFromStringSig) Clone() builtinFunc {
 	return newSig
 }
 
-// evalInt evals a builtinExtractDatetimeSig.
+// evalInt evals a builtinExtractDatetimeFromStringSig.
 // See https://dev.mysql.com/doc/refman/5.7/en/date-and-time-functions.html#function_extract
 func (b *builtinExtractDatetimeFromStringSig) evalInt(row chunk.Row) (int64, bool, error) {
 	unit, isNull, err := b.args[0].EvalString(b.ctx, row)
@@ -2637,8 +2641,7 @@ func (b *builtinExtractDatetimeFromStringSig) evalInt(row chunk.Row) (int64, boo
 		return 0, isNull, err
 	}
 	sc := b.ctx.GetSessionVars().StmtCtx
-	switch strings.ToUpper(unit) {
-	case "DAY_MICROSECOND", "DAY_SECOND", "DAY_MINUTE", "DAY_HOUR":
+	if types.IsClockUnit(unit) && types.IsDateUnit(unit) {
 		dur, _, err := types.ParseDuration(sc, dtStr, types.GetFsp(dtStr))
 		if err != nil {
 			return 0, true, err
@@ -2656,22 +2659,8 @@ func (b *builtinExtractDatetimeFromStringSig) evalInt(row chunk.Row) (int64, boo
 		}
 		return res, err != nil, err
 	}
-	dt, err := types.ParseDatetime(sc, dtStr)
-	if err != nil {
-		if !terror.ErrorEqual(err, types.ErrWrongValue) {
-			return 0, true, err
-		}
-	}
-	if dt.IsZero() {
-		dt.SetFsp(b.args[1].GetType().GetDecimal())
-		if b.ctx.GetSessionVars().SQLMode.HasNoZeroDateMode() {
-			isNull, err := handleInvalidZeroTime(b.ctx, dt)
-			return 0, isNull, err
-		}
-		return 0, false, nil
-	}
-	res, err := types.ExtractDatetimeNum(&dt, unit)
-	return res, err != nil, err
+
+	panic("Unexpected unit for extract")
 }
 
 type builtinExtractDatetimeSig struct {
@@ -6454,19 +6443,6 @@ func (b *builtinTidbParseTsoSig) evalTime(row chunk.Row) (types.Time, bool, erro
 	return result, false, nil
 }
 
-func handleInvalidZeroTime(ctx sessionctx.Context, t types.Time) (bool, error) {
-	// MySQL compatibility, #11203
-	// 0 | 0.0 should be converted to null without warnings
-	n, err := t.ToNumber().ToInt()
-	isOriginalIntOrDecimalZero := err == nil && n == 0
-	// Args like "0000-00-00", "0000-00-00 00:00:00" set Fsp to 6
-	isOriginalStringZero := t.Fsp() > 0
-	if isOriginalIntOrDecimalZero && !isOriginalStringZero {
-		return false, nil
-	}
-	return true, handleInvalidTimeError(ctx, types.ErrWrongValue.GenWithStackByArgs(types.DateTimeStr, t.String()))
-}
-
 // tidbBoundedStalenessFunctionClass reads a time window [a, b] and compares it with the latest SafeTS
 // to determine which TS to use in a read only transaction.
 type tidbBoundedStalenessFunctionClass struct {

expression/expr_to_pb_test.go

@@ -551,7 +551,10 @@ func TestExprPushDownToFlash(t *testing.T) {
 	exprs = append(exprs, function)
 
 	// ExtractDatetime: can be pushed
-	function, err = NewFunction(mock.NewContext(), ast.Extract, types.NewFieldType(mysql.TypeLonglong), stringColumn, datetimeColumn)
+	extractDatetimeUnitCol := new(Constant)
+	extractDatetimeUnitCol.Value = types.NewStringDatum("day")
+	extractDatetimeUnitCol.RetType = types.NewFieldType(mysql.TypeString)
+	function, err = NewFunction(mock.NewContext(), ast.Extract, types.NewFieldType(mysql.TypeLonglong), extractDatetimeUnitCol, datetimeColumn)
 	require.NoError(t, err)
 	exprs = append(exprs, function)
 
@@ -961,7 +964,10 @@ func TestExprPushDownToFlash(t *testing.T) {
 	exprs = append(exprs, function)
 
 	// ExtractDatetimeFromString: can not be pushed
-	function, err = NewFunction(mock.NewContext(), ast.Extract, types.NewFieldType(mysql.TypeLonglong), stringColumn, stringColumn)
+	extractDatetimeFromStringUnitCol := new(Constant)
+	extractDatetimeFromStringUnitCol.Value = types.NewStringDatum("day_microsecond")
+	extractDatetimeFromStringUnitCol.RetType = types.NewFieldType(mysql.TypeString)
+	function, err = NewFunction(mock.NewContext(), ast.Extract, types.NewFieldType(mysql.TypeLonglong), extractDatetimeFromStringUnitCol, stringColumn)
 	require.NoError(t, err)
 	exprs = append(exprs, function)
 

expression/integration_serial_test.go

@@ -2611,7 +2611,11 @@ func TestTimeBuiltin(t *testing.T) {
 	result = tk.MustQuery("select extract(day_hour from '2017-01-01 12:12:12'), extract(day_hour from '01 12:12:12'), extract(day_hour from '12:12:12'), extract(day_hour from '01 00:00:00.89')")
 	result.Check(testkit.Rows("112 36 12 24"))
 	result = tk.MustQuery("select extract(day_microsecond from cast('2017-01-01 12:12:12' as datetime)), extract(day_second from cast('2017-01-01 12:12:12' as datetime)), extract(day_minute from cast('2017-01-01 12:12:12' as datetime)), extract(day_hour from cast('2017-01-01 12:12:12' as datetime))")
-	result.Check(testkit.Rows("1121212000000 1121212 11212 112"))
+	result.Check(testkit.Rows("121212000000 121212 1212 12"))
+	result = tk.MustQuery("select extract(day_microsecond from cast(20010101020304.050607 as decimal(20,6))), extract(day_second from cast(20010101020304.050607 as decimal(20,6))), extract(day_minute from cast(20010101020304.050607 as decimal(20,6))), extract(day_hour from cast(20010101020304.050607 as decimal(20,6))), extract(day from cast(20010101020304.050607 as decimal(20,6)))")
+	result.Check(testkit.Rows("1020304050607 1020304 10203 102 1"))
+	result = tk.MustQuery("select extract(day_microsecond from cast(1020304.050607 as decimal(20,6))), extract(day_second from cast(1020304.050607 as decimal(20,6))), extract(day_minute from cast(1020304.050607 as decimal(20,6))), extract(day_hour from cast(1020304.050607 as decimal(20,6))), extract(day from cast(1020304.050607 as decimal(20,6)))")
+	result.Check(testkit.Rows("1020304050607 1020304 10203 102 4"))
 
 	// for adddate, subdate
 	dateArithmeticalTests := []struct {

expression/integration_test.go

@@ -7448,3 +7448,14 @@ func TestImcompleteDateFunc(t *testing.T) {
 	tk.MustQuery("select YEARWEEK('1998-10-00')").Check(testkit.Rows("<nil>"))
 	tk.MustQuery("select YEARWEEK('1998-00-11')").Check(testkit.Rows("<nil>"))
 }
+
+func TestIssue34998(t *testing.T) {
+	store, clean := testkit.CreateMockStore(t)
+	defer clean()
+	tk := testkit.NewTestKit(t, store)
+	tk.MustExec("use test")
+	tk.MustExec("CREATE TABLE `PK_S_MULTI_43`(`COL1` time(2) NOT NULL, `COL2` time(2) NOT NULL, `COL3` time(2) DEFAULT NULL, PRIMARY KEY(`COL1`,`COL2`))")
+	tk.MustExec("insert into PK_S_MULTI_43(col1, col2) values('-512:37:22.00', '-512:37:22.00')")
+	tk.MustQuery("select extract(day_microsecond from '-512:37:22.00')").Check(testkit.Rows("-5123722000000"))
+	tk.MustQuery("select extract(day_microsecond from col1) from PK_S_MULTI_43").Check(testkit.Rows("-5123722000000"))
+}