From 6710c49d1eb03a29b4cf6633827c84f185e692c5 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 7 Aug 2023 11:39:53 +0200 Subject: [PATCH] do not use version to empty the cache Signed-off-by: Andres Taylor --- go/vt/servenv/mysql.go | 1 + go/vt/vtgate/engine/memory_sort_test.go | 12 +++---- go/vt/vtgate/engine/ordered_aggregate_test.go | 3 +- go/vt/vtgate/engine/route_test.go | 17 +++++----- go/vt/vtgate/evalengine/api_compare_test.go | 3 ++ .../evalengine/integration/comparison_test.go | 3 ++ go/vt/vtgate/planbuilder/plan_test.go | 14 +++++++-- go/vt/vtgate/semantics/info_schema.go | 31 +++++++++++-------- 8 files changed, 52 insertions(+), 32 deletions(-) diff --git a/go/vt/servenv/mysql.go b/go/vt/servenv/mysql.go index 94019a1c42c..5f5c74a86bb 100644 --- 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 index 0af0564dc7c..3fc5e61c32b 100644 --- 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 index 539e81b4c59..5a996090e84 100644 --- 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 index 7d5b1d1bbea..95c7c3adcb9 100644 --- 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 index e4fb5d38470..85c95c2b48a 100644 --- 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 index 9677a39bad0..0a123cd6a31 100644 --- 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 index 755a5fbf916..b626de451a6 100644 --- 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 index 38fc5df465a..5d592a081a8 100644 --- 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") {