do not use version to empty the cache

Signed-off-by: Andres Taylor <andres@planetscale.com>
vitessio · Aug 7, 2023 · 6710c49 · 6710c49
1 parent 01195ce
commit 6710c49
Show file tree

Hide file tree

Showing 8 changed files with 52 additions and 32 deletions.
diff --git a/go/vt/servenv/mysql.go b/go/vt/servenv/mysql.go
@@ -40,6 +40,7 @@ func MySQLServerVersion() string {
 // flag. It is intended for use in tests that require a specific MySQL server
 // version (for example, collations) that cannot specify that via the command
 // line.
+// Please note that semantics.ClearCachedInfoSchemaInfo() should be called any time this method is called
 func SetMySQLServerVersionForTest(version string) {
 	mySQLServerVersion = version
 }

diff --git a/go/vt/vtgate/engine/memory_sort_test.go b/go/vt/vtgate/engine/memory_sort_test.go
@@ -20,22 +20,22 @@ import (
 	"context"
 	"testing"
 
-	"vitess.io/vitess/go/vt/servenv"
-	"vitess.io/vitess/go/vt/vtgate/evalengine"
-
-	"vitess.io/vitess/go/mysql/collations"
-	"vitess.io/vitess/go/test/utils"
-
 	"github.com/stretchr/testify/require"
 
+	"vitess.io/vitess/go/mysql/collations"
 	"vitess.io/vitess/go/sqltypes"
+	"vitess.io/vitess/go/test/utils"
 	querypb "vitess.io/vitess/go/vt/proto/query"
+	"vitess.io/vitess/go/vt/servenv"
+	"vitess.io/vitess/go/vt/vtgate/evalengine"
+	"vitess.io/vitess/go/vt/vtgate/semantics"
 )
 
 func init() {
 	// We require MySQL 8.0 collations for the comparisons in the tests
 	mySQLVersion := "8.0.0"
 	servenv.SetMySQLServerVersionForTest(mySQLVersion)
+	semantics.ClearCachedInfoSchemaInfo()
 	collationEnv = collations.NewEnvironment(mySQLVersion)
 }
 

diff --git a/go/vt/vtgate/engine/ordered_aggregate_test.go b/go/vt/vtgate/engine/ordered_aggregate_test.go
@@ -28,11 +28,11 @@ import (
 	"vitess.io/vitess/go/mysql/collations"
 	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/test/utils"
-
 	binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
 	querypb "vitess.io/vitess/go/vt/proto/query"
 	"vitess.io/vitess/go/vt/servenv"
 	. "vitess.io/vitess/go/vt/vtgate/engine/opcode"
+	"vitess.io/vitess/go/vt/vtgate/semantics"
 )
 
 var collationEnv *collations.Environment
@@ -41,6 +41,7 @@ func init() {
 	// We require MySQL 8.0 collations for the comparisons in the tests
 	mySQLVersion := "8.0.0"
 	servenv.SetMySQLServerVersionForTest(mySQLVersion)
+	semantics.ClearCachedInfoSchemaInfo()
 	collationEnv = collations.NewEnvironment(mySQLVersion)
 }
 

diff --git a/go/vt/vtgate/engine/route_test.go b/go/vt/vtgate/engine/route_test.go
@@ -24,22 +24,18 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-
-	"vitess.io/vitess/go/mysql/sqlerror"
+	"github.com/stretchr/testify/require"
 
 	"vitess.io/vitess/go/mysql/collations"
+	"vitess.io/vitess/go/mysql/sqlerror"
+	"vitess.io/vitess/go/sqltypes"
+	querypb "vitess.io/vitess/go/vt/proto/query"
 	vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
 	"vitess.io/vitess/go/vt/servenv"
-	"vitess.io/vitess/go/vt/vterrors"
-
 	"vitess.io/vitess/go/vt/sqlparser"
-
+	"vitess.io/vitess/go/vt/vterrors"
 	"vitess.io/vitess/go/vt/vtgate/evalengine"
-
-	"github.com/stretchr/testify/require"
-
-	"vitess.io/vitess/go/sqltypes"
-	querypb "vitess.io/vitess/go/vt/proto/query"
+	"vitess.io/vitess/go/vt/vtgate/semantics"
 	"vitess.io/vitess/go/vt/vtgate/vindexes"
 )
 
@@ -55,6 +51,7 @@ func init() {
 	// We require MySQL 8.0 collations for the comparisons in the tests
 	mySQLVersion := "8.0.0"
 	servenv.SetMySQLServerVersionForTest(mySQLVersion)
+	semantics.ClearCachedInfoSchemaInfo()
 	collationEnv = collations.NewEnvironment(mySQLVersion)
 }
 

diff --git a/go/vt/vtgate/evalengine/api_compare_test.go b/go/vt/vtgate/evalengine/api_compare_test.go
@@ -23,6 +23,8 @@ import (
 	"testing"
 	"time"
 
+	"vitess.io/vitess/go/vt/vtgate/semantics"
+
 	"github.com/stretchr/testify/require"
 
 	"vitess.io/vitess/go/mysql/collations"
@@ -56,6 +58,7 @@ func init() {
 	// We require MySQL 8.0 collations for the comparisons in the tests
 	mySQLVersion := "8.0.0"
 	servenv.SetMySQLServerVersionForTest(mySQLVersion)
+	semantics.ClearCachedInfoSchemaInfo()
 	collationEnv = collations.NewEnvironment(mySQLVersion)
 }
 

diff --git a/go/vt/vtgate/evalengine/integration/comparison_test.go b/go/vt/vtgate/evalengine/integration/comparison_test.go
@@ -27,6 +27,8 @@ import (
 	"testing"
 	"time"
 
+	"vitess.io/vitess/go/vt/vtgate/semantics"
+
 	"github.com/spf13/pflag"
 
 	"vitess.io/vitess/go/mysql/format"
@@ -256,6 +258,7 @@ func TestMySQL(t *testing.T) {
 	// We require MySQL 8.0 collations for the comparisons in the tests
 
 	servenv.SetMySQLServerVersionForTest(conn.ServerVersion)
+	semantics.ClearCachedInfoSchemaInfo()
 	collationEnv = collations.NewEnvironment(conn.ServerVersion)
 	servenv.OnParse(registerFlags)
 	initTimezoneData(t, conn)

diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go
@@ -102,8 +102,13 @@ func TestPlan(t *testing.T) {
 
 func TestSystemTables57(t *testing.T) {
 	// first we move everything to use 5.7 logic
+	oldVer := servenv.MySQLServerVersion()
 	servenv.SetMySQLServerVersionForTest("5.7")
-	defer servenv.SetMySQLServerVersionForTest("")
+	semantics.ClearCachedInfoSchemaInfo()
+	defer func() {
+		servenv.SetMySQLServerVersionForTest(oldVer)
+		semantics.ClearCachedInfoSchemaInfo()
+	}()
 	vschemaWrapper := &vschemaWrapper{v: loadSchema(t, "vschemas/schema.json", true)}
 	testOutputTempDir := makeTestOutput(t)
 	testFile(t, "info_schema57_cases.json", testOutputTempDir, vschemaWrapper, false)
@@ -193,8 +198,13 @@ func TestOneWithTPCHVSchema(t *testing.T) {
 
 func TestOneWith57Version(t *testing.T) {
 	// first we move everything to use 5.7 logic
+	oldVer := servenv.MySQLServerVersion()
 	servenv.SetMySQLServerVersionForTest("5.7")
-	defer servenv.SetMySQLServerVersionForTest("")
+	semantics.ClearCachedInfoSchemaInfo()
+	defer func() {
+		servenv.SetMySQLServerVersionForTest(oldVer)
+		semantics.ClearCachedInfoSchemaInfo()
+	}()
 	vschema := &vschemaWrapper{v: loadSchema(t, "vschemas/schema.json", true)}
 
 	testFile(t, "onecase.json", "", vschema, false)

diff --git a/go/vt/vtgate/semantics/info_schema.go b/go/vt/vtgate/semantics/info_schema.go
@@ -1671,27 +1671,32 @@ type infoSchemaWithColumns struct {
 	infoSchemaData map[string][]vindexes.Column
 }
 
-// We cache this information. If the registered mysql version is changed, we flush the cache
+// We cache this information. If the version is changed in a test, this field also needs to be cleared
 var cacheSchemaInfo map[string][]vindexes.Column
-var cachedVersion string
 
 // newSchemaInfo returns a SchemaInformation that has the column information for all info_schema tables
 func newSchemaInfo(inner SchemaInformation) SchemaInformation {
-	version := servenv.MySQLServerVersion()
-
-	if version != cachedVersion || cacheSchemaInfo == nil {
-		// we don't have a valid cached info_schema map with column information
-		if strings.HasPrefix(version, "5.7") {
-			cacheSchemaInfo = getInfoSchema57()
-		} else {
-			cacheSchemaInfo = getInfoSchema80()
-		}
-		cachedVersion = version
+	if cacheSchemaInfo == nil {
+		// yes, this is slightly racy, but the worst that can happen is that we build it more times
+		// than needed. It's not going to lead to any actual issues
+		loadSchemaInfo()
 	}
-
 	return &infoSchemaWithColumns{inner: inner, infoSchemaData: cacheSchemaInfo}
 }
 
+func ClearCachedInfoSchemaInfo() {
+	cacheSchemaInfo = nil
+}
+
+func loadSchemaInfo() {
+	version := servenv.MySQLServerVersion()
+	if strings.HasPrefix(version, "5.7") {
+		cacheSchemaInfo = getInfoSchema57()
+	} else {
+		cacheSchemaInfo = getInfoSchema80()
+	}
+}
+
 // FindTableOrVindex implements the SchemaInformation interface
 func (i *infoSchemaWithColumns) FindTableOrVindex(tbl sqlparser.TableName) (*vindexes.Table, vindexes.Vindex, string, topodatapb.TabletType, key.Destination, error) {
 	if !strings.EqualFold(tbl.Qualifier.String(), "information_schema") {