planner, meta: avoid unsafe index on virtual generated temporal columns

pkg/executor/builder.go

@@ -531,13 +531,30 @@ func buildIndexLookUpChecker(b *executorBuilder, p *physicalop.PhysicalIndexLook
 	}
 }
 
+func indexContainsVirtualGeneratedTemporalWithDateColumn(tblInfo *model.TableInfo, index *model.IndexInfo) bool {
+	for _, idxCol := range index.Columns {
+		if idxCol.Offset < 0 || idxCol.Offset >= len(tblInfo.Columns) {
+			continue
+		}
+		if tblInfo.Columns[idxCol.Offset].IsVirtualGeneratedTemporalWithDateColumn() {
+			return true
+		}
+	}
+	return false
+}
+
 func (b *executorBuilder) buildCheckTable(v *plannercore.CheckTable) exec.Executor {
 	canUseFastCheck := true
+	tblInfo := v.Table.Meta()
 	for _, idx := range v.IndexInfos {
 		if idx.MVIndex || idx.IsColumnarIndex() {
 			canUseFastCheck = false
 			break
 		}
+		if indexContainsVirtualGeneratedTemporalWithDateColumn(tblInfo, idx) {
+			canUseFastCheck = false
+			break
+		}
 		for _, col := range idx.Columns {
 			if col.Length != types.UnspecifiedLength {
 				canUseFastCheck = false

pkg/executor/check_table_index.go

@@ -896,6 +896,9 @@ func getCheckSum(ctx context.Context, se sessionctx.Context, sql string) ([]grou
 
 func getGlobalCheckSum(ctx context.Context, se sessionctx.Context, sql string) (checksum uint64, count int64, err error) {
 	ctx = kv.WithInternalSourceType(ctx, kv.InternalTxnAdmin)
+	// Create a new ExecDetails for the internal SQL to avoid data race with the parent statement.
+	// The parent context may carry an ExecDetails that is still being written to by concurrent goroutines.
+	ctx = execdetails.ContextWithInitializedExecDetails(ctx)
 	rs, err := se.GetSQLExecutor().ExecuteInternal(ctx, sql)
 	if err != nil {
 		return 0, 0, err

pkg/executor/test/admintest/admin_test.go

@@ -2293,13 +2293,25 @@ func TestFastAdminCheckWithError(t *testing.T) {
 	// And the admin check shouldn't be blocked when meeting error.
 	tk := testkit.NewTestKit(t, store)
 	tk.MustExec("use test")
+	tk.MustExec("set tidb_enable_fast_table_check = 1")
 	tk.MustExec("drop table if exists admin_test")
 	tk.MustExec(`
 		create table admin_test (c1 int, c2 int,
 		key idx1(c1), key idx2(c1), key idx3(c1), key idx4(c1), key idx5(c1),
 		key idx6(c1), key idx7(c1), key idx8(c1), key idx9(c1), key idx10(c1))
 	`)
 	tk.MustExecToErr("admin check table admin_test")
+
+	tk.MustExec("drop table if exists admin_test_generated")
+	tk.MustExec(`
+		create table admin_test_generated (
+			payload json,
+			f date as (JSON_EXTRACT(payload, '$.f')),
+			key idx_f(f)
+		)
+	`)
+	tk.MustExec(`insert into admin_test_generated set payload='{"f":"2018-09-28"}'`)
+	tk.MustExec("admin check table admin_test_generated")
 }
 
 func TestFastAdminCheckQuickPassSkipBucketed(t *testing.T) {
@@ -2488,7 +2500,7 @@ func TestAdminCheckTableWithEnumAndPointGet(t *testing.T) {
 func TestFastCheckTableConcurrent(t *testing.T) {
 	// This test verifies that concurrent execution of admin check table works correctly.
 	// Note: The data race in ExecDetails (fixed by using ContextWithInitializedExecDetails
-	// in getCheckSum) cannot be detected in unit tests because mocktikv doesn't trigger
+	// in getCheckSum/getGlobalCheckSum) cannot be detected in unit tests because mocktikv doesn't trigger
 	// the network traffic writes to ExecDetails that happen in real TiKV environments.
 	store := testkit.CreateMockStore(t)
 	tk := testkit.NewTestKit(t, store)

pkg/importsdk/BUILD.bazel

@@ -54,6 +54,7 @@ go_test(
         "//pkg/lightning/config",
         "//pkg/lightning/log",
         "//pkg/lightning/mydump",
+        "//pkg/parser/ast",
         "//pkg/parser/mysql",
         "//pkg/util/table-filter",
         "@com_github_data_dog_go_sqlmock//:go-sqlmock",

pkg/meta/model/column.go

@@ -221,6 +221,20 @@ func (c *ColumnInfo) IsVirtualGenerated() bool {
 	return c.IsGenerated() && !c.GeneratedStored
 }
 
+// IsVirtualGeneratedTemporalWithDateColumn checks whether the column is a virtual generated
+// column with a temporal type that contains a date part, such as DATE, DATETIME, or TIMESTAMP.
+func (c *ColumnInfo) IsVirtualGeneratedTemporalWithDateColumn() bool {
+	if !c.IsVirtualGenerated() {
+		return false
+	}
+	switch c.GetType() {
+	case mysql.TypeDate, mysql.TypeDatetime, mysql.TypeTimestamp:
+		return true
+	default:
+		return false
+	}
+}
+
 // IsChanging checks if the column is a new column added in modify column.
 func (c *ColumnInfo) IsChanging() bool {
 	return strings.HasPrefix(c.Name.O, changingColumnPrefix)

pkg/planner/core/casetest/index/BUILD.bazel

@@ -9,7 +9,7 @@ go_test(
     ],
     data = glob(["testdata/**"]),
     flaky = True,
-    shard_count = 12,
+    shard_count = 13,
     deps = [
         "//pkg/domain",
         "//pkg/domain/infosync",

pkg/planner/core/casetest/index/index_test.go

@@ -107,6 +107,44 @@ func TestInvisibleIndex(t *testing.T) {
 	})
 }
 
+func TestVirtualGeneratedTemporalWithDateIndex(t *testing.T) {
+	testkit.RunTestUnderCascades(t, func(t *testing.T, tk *testkit.TestKit, cascades, caller string) {
+		tk.MustExec("use test")
+		tk.MustExec("drop table if exists t_issue_52520")
+		tk.MustExec("create table t_issue_52520 (a varchar(32), b date as (a), key idx_b(b))")
+		tk.MustExec("set @@sql_mode = ''")
+		tk.MustExec("insert into t_issue_52520(a) values ('2020-02-31')")
+		tk.MustExec("set @@sql_mode = 'ALLOW_INVALID_DATES'")
+
+		tk.MustNoIndexUsed("select /* issue:52520 */ b from t_issue_52520 use index(idx_b)")
+		tk.MustQuery("show warnings").CheckContain("virtual generated temporal column")
+		tk.MustQuery("select /* issue:52520 */ b from t_issue_52520 use index(idx_b)").Check(testkit.Rows("2020-02-31"))
+		tk.MustQuery("show warnings").CheckContain("virtual generated temporal column")
+
+		tk.MustNoIndexUsed("select /*+ USE_INDEX(t_issue_52520, idx_b) */ /* issue:52520 */ b from t_issue_52520")
+		tk.MustQuery("show warnings").CheckContain("virtual generated temporal column")
+		tk.MustQuery("select /*+ USE_INDEX(t_issue_52520, idx_b) */ /* issue:52520 */ b from t_issue_52520").Check(testkit.Rows("2020-02-31"))
+		tk.MustQuery("show warnings").CheckContain("virtual generated temporal column")
+
+		tk.MustNoIndexUsed("select /* issue:52520 */ b from t_issue_52520 use index(idx_b) where b = '2020-02-31'")
+		tk.MustQuery("show warnings").CheckContain("virtual generated temporal column")
+		tk.MustQuery("select /* issue:52520 */ b from t_issue_52520 use index(idx_b) where b = '2020-02-31'").Check(testkit.Rows("2020-02-31"))
+		tk.MustQuery("show warnings").CheckContain("virtual generated temporal column")
+
+		tk.MustNoIndexUsed("select /*+ USE_INDEX(t_issue_52520, idx_b) */ /* issue:52520 */ b from t_issue_52520 where b = '2020-02-31'")
+		tk.MustQuery("show warnings").CheckContain("virtual generated temporal column")
+		tk.MustQuery("select /*+ USE_INDEX(t_issue_52520, idx_b) */ /* issue:52520 */ b from t_issue_52520 where b = '2020-02-31'").Check(testkit.Rows("2020-02-31"))
+		tk.MustQuery("show warnings").CheckContain("virtual generated temporal column")
+
+		tk.MustNoIndexUsed("select /* issue:52520 */ b from t_issue_52520 ignore index(idx_b) where b = '2020-02-31'")
+		tk.MustQuery("show warnings").Check(testkit.Rows())
+
+		tk.MustExec("drop table if exists t_issue_52520_prefix")
+		tk.MustExec("create table t_issue_52520_prefix (a varchar(32), c int, b date as (a), key idx_safe(c), key idx_unsafe(b))")
+		tk.MustContainErrMsg("select /* issue:52520 */ c from t_issue_52520_prefix use index(idx) where c = 1", "Key 'idx' doesn't exist")
+	})
+}
+
 func TestRangeDerivation(t *testing.T) {
 	testkit.RunTestUnderCascades(t, func(t *testing.T, testKit *testkit.TestKit, cascades, caller string) {
 		testKit.MustExec("use test")
@@ -297,6 +335,14 @@ func TestInvertedIndex(t *testing.T) {
 		testKit.MustNoIndexUsed("select * from t ignore index(idx_b) where b = 2")
 		testKit.MustNoIndexUsed("select * from t ignore index(idx_c) where c = 3")
 		testKit.MustNoIndexUsed("select * from t ignore index(idx_d) where d < 1")
+
+		testKit.MustExec("drop table if exists t_issue_52520_columnar")
+		testKit.MustExec("create table t_issue_52520_columnar (a varchar(32), b date as (a))")
+		testKit.MustExec("alter table t_issue_52520_columnar set tiflash replica 1")
+		testKit.MustExec("alter table t_issue_52520_columnar add columnar index idx_b (b) using inverted")
+		testkit.SetTiFlashReplica(t, dom, "test", "t_issue_52520_columnar")
+		testKit.MustNoIndexUsed("select /* issue:52520 */ b from t_issue_52520_columnar use index(idx_b)")
+		testKit.MustQuery("show warnings").CheckContain("virtual generated temporal column")
 	})
 }
 

pkg/planner/core/planbuilder.go

@@ -1154,32 +1154,71 @@ func (*PlanBuilder) detectSelectWindow(sel *ast.SelectStmt) bool {
 	return false
 }
 
-func getPathByIndexName(paths []*util.AccessPath, idxName ast.CIStr, tblInfo *model.TableInfo) *util.AccessPath {
-	var indexPrefixPath *util.AccessPath
-	prefixMatches := 0
-	for _, path := range paths {
-		// Only accept tikv's primary key table path.
+type indexHintResolveStatus int
+
+const (
+	indexHintResolveNotFound indexHintResolveStatus = iota
+	indexHintResolvePublicPath
+	indexHintResolveFilteredUnsafe
+)
+
+func resolveIndexHintByName(publicPaths []*util.AccessPath, filteredUnsafeIndexes []*model.IndexInfo, idxName ast.CIStr, tblInfo *model.TableInfo) (*util.AccessPath, *model.IndexInfo, indexHintResolveStatus) {
+	// Exact names win over prefixes. Check public paths first so a valid index is
+	// never shadowed by a filtered index that happens to share the exact name.
+	for _, path := range publicPaths {
 		if path.IsTiKVTablePath() && isPrimaryIndex(idxName) && tblInfo.HasClusteredIndex() {
-			return path
+			return path, nil, indexHintResolvePublicPath
 		}
-		// If it's not a tikv table path and the index is nil, it could not be any index path.
-		if path.Index == nil {
-			continue
+		if path.Index != nil && path.Index.Name.L == idxName.L {
+			return path, nil, indexHintResolvePublicPath
 		}
-		if path.Index.Name.L == idxName.L {
-			return path
+	}
+	for _, index := range filteredUnsafeIndexes {
+		if index.Name.L == idxName.L {
+			return nil, index, indexHintResolveFilteredUnsafe
+		}
+	}
+
+	var foundPath *util.AccessPath
+	var foundFilteredIndex *model.IndexInfo
+	prefixMatches := 0
+	for _, path := range publicPaths {
+		if path.Index != nil && strings.HasPrefix(path.Index.Name.L, idxName.L) {
+			foundPath = path
+			foundFilteredIndex = nil
+			prefixMatches++
 		}
-		if strings.HasPrefix(path.Index.Name.L, idxName.L) {
-			indexPrefixPath = path
+	}
+	for _, index := range filteredUnsafeIndexes {
+		if strings.HasPrefix(index.Name.L, idxName.L) {
+			foundPath = nil
+			foundFilteredIndex = index
 			prefixMatches++
 		}
 	}
+	if prefixMatches != 1 {
+		return nil, nil, indexHintResolveNotFound
+	}
+	if foundFilteredIndex != nil {
+		return nil, foundFilteredIndex, indexHintResolveFilteredUnsafe
+	}
+	return foundPath, nil, indexHintResolvePublicPath
+}
 
-	// Return only unique prefix matches
-	if prefixMatches == 1 {
-		return indexPrefixPath
+func indexContainsVirtualGeneratedTemporalWithDateColumn(tblInfo *model.TableInfo, index *model.IndexInfo) bool {
+	for _, idxCol := range index.Columns {
+		if idxCol.Offset < 0 || idxCol.Offset >= len(tblInfo.Columns) {
+			continue
+		}
+		if tblInfo.Columns[idxCol.Offset].IsVirtualGeneratedTemporalWithDateColumn() {
+			return true
+		}
 	}
-	return nil
+	return false
+}
+
+func genVirtualGeneratedTemporalWithDateIndexHintWarning(index *model.IndexInfo) string {
+	return fmt.Sprintf("hint is inapplicable, index %s contains a virtual generated temporal column whose values depend on sql_mode", index.Name.O)
 }
 
 func isPrimaryIndex(indexName ast.CIStr) bool {
@@ -1308,6 +1347,7 @@ func getPossibleAccessPaths(ctx base.PlanContext, tableHints *hint.PlanHints, in
 	var err error
 	// Inverted Index can not be used as access path index.
 	invertedIndexes := make(map[string]struct{})
+	var virtualGeneratedTemporalWithDateIndexes []*model.IndexInfo
 
 	// When NO_INDEX_LOOKUP_PUSHDOWN hint is specified, we should set `forceNoIndexLookUpPushDown = true` to avoid
 	// using index look up push down even if other hint or system variable `tidb_index_lookup_pushdown_policy`
@@ -1346,6 +1386,10 @@ func getPossibleAccessPaths(ctx base.PlanContext, tableHints *hint.PlanHints, in
 					continue
 				}
 			}
+			if indexContainsVirtualGeneratedTemporalWithDateColumn(tblInfo, index) {
+				virtualGeneratedTemporalWithDateIndexes = append(virtualGeneratedTemporalWithDateIndexes, index)
+				continue
+			}
 			if index.InvertedInfo != nil {
 				invertedIndexes[index.Name.L] = struct{}{}
 				continue
@@ -1441,8 +1485,15 @@ func getPossibleAccessPaths(ctx base.PlanContext, tableHints *hint.PlanHints, in
 			if _, ok := invertedIndexes[idxName.L]; ok {
 				continue
 			}
-			path := getPathByIndexName(publicPaths, idxName, tblInfo)
-			if path == nil {
+			path, filteredIndex, resolveStatus := resolveIndexHintByName(publicPaths, virtualGeneratedTemporalWithDateIndexes, idxName, tblInfo)
+			if resolveStatus == indexHintResolveFilteredUnsafe {
+				if hint.HintType != ast.HintIgnore {
+					hasUseOrForce = true
+					ctx.GetSessionVars().StmtCtx.SetHintWarning(genVirtualGeneratedTemporalWithDateIndexHintWarning(filteredIndex))
+				}
+				continue
+			}
+			if resolveStatus == indexHintResolveNotFound {
 				err := plannererrors.ErrKeyDoesNotExist.FastGenByArgs(idxName, tblInfo.Name)
 				// if hint is from comment-style sql hints, we should throw a warning instead of error.
 				if i < indexHintsLen {