From 949df8b383b504bf0c75de2594cee71145b672f1 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 25 Jul 2023 11:30:51 -0700 Subject: [PATCH 01/18] bug fix for error message with missing value --- memory/database.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/memory/database.go b/memory/database.go index 08d18562f6..9897080351 100644 --- a/memory/database.go +++ b/memory/database.go @@ -415,7 +415,7 @@ func (d *Database) CreateView(ctx *sql.Context, name string, selectStatement, cr func (d *Database) DropView(ctx *sql.Context, name string) error { _, ok := d.views[name] if !ok { - return sql.ErrViewDoesNotExist.New(name) + return sql.ErrViewDoesNotExist.New(d.name, name) } delete(d.views, name) From 8f999d85f580c70c7e776f78bc9ee3f8b1e4e893 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 25 Jul 2023 11:38:34 -0700 Subject: [PATCH 02/18] Preparing for ANSI_QUOTES SQL mode support for GMS --- server/golden/proxy.go | 5 +++++ server/golden/validator.go | 5 +++++ server/handler.go | 16 ++++++++++++++++ sql/analyzer/check_constraints.go | 1 + 4 files changed, 27 insertions(+) diff --git a/server/golden/proxy.go b/server/golden/proxy.go index 76c2b03954..e3220db1d9 100644 --- a/server/golden/proxy.go +++ b/server/golden/proxy.go @@ -25,6 +25,7 @@ import ( "github.com/dolthub/vitess/go/mysql" "github.com/dolthub/vitess/go/sqltypes" querypb "github.com/dolthub/vitess/go/vt/proto/query" + "github.com/dolthub/vitess/go/vt/sqlparser" mysql2 "github.com/go-sql-driver/mysql" "github.com/gocraft/dbr/v2" "github.com/sirupsen/logrus" @@ -41,6 +42,10 @@ type MySqlProxy struct { conns map[uint32]proxyConn } +func (h MySqlProxy) ParserOptionsForConnection(_ *mysql.Conn) (sqlparser.ParserOptions, error) { + return sqlparser.ParserOptions{}, nil +} + type proxyConn struct { *dbr.Connection *logrus.Entry diff --git a/server/golden/validator.go b/server/golden/validator.go index 06929c46f0..087a22eead 100644 --- a/server/golden/validator.go +++ b/server/golden/validator.go @@ -24,6 +24,7 @@ import ( "github.com/dolthub/vitess/go/mysql" "github.com/dolthub/vitess/go/sqltypes" "github.com/dolthub/vitess/go/vt/proto/query" + "github.com/dolthub/vitess/go/vt/sqlparser" _ "github.com/go-sql-driver/mysql" "github.com/sirupsen/logrus" "golang.org/x/sync/errgroup" @@ -37,6 +38,10 @@ type Validator struct { logger *logrus.Logger } +func (v Validator) ParserOptionsForConnection(_ *mysql.Conn) (sqlparser.ParserOptions, error) { + return sqlparser.ParserOptions{}, nil +} + // NewValidatingHandler creates a new Validator wrapping a MySQL connection. func NewValidatingHandler(handler mysql.Handler, mySqlConn string, logger *logrus.Logger) (Validator, error) { golden, err := NewMySqlProxyHandler(logger, mySqlConn) diff --git a/server/handler.go b/server/handler.go index 6df9d1439a..dbfad18c5a 100644 --- a/server/handler.go +++ b/server/handler.go @@ -27,6 +27,7 @@ import ( "github.com/dolthub/vitess/go/netutil" "github.com/dolthub/vitess/go/sqltypes" "github.com/dolthub/vitess/go/vt/proto/query" + "github.com/dolthub/vitess/go/vt/sqlparser" "github.com/go-kit/kit/metrics/discard" "github.com/sirupsen/logrus" "go.opentelemetry.io/otel/attribute" @@ -108,6 +109,8 @@ func (h *Handler) ComPrepare(c *mysql.Conn, query string) ([]*query.Field, error analyzed, err = h.e.PrepareQuery(ctx, query) } if err != nil { + // TODO: Look at error logging in other handler functions + logrus.StandardLogger().Errorf("ComPrepare ERROR: %s", err.Error()) err := sql.CastSQLError(err) return nil, err } @@ -129,6 +132,19 @@ func (h *Handler) ComResetConnection(c *mysql.Conn) { // TODO: handle reset logic } +func (h *Handler) ParserOptionsForConnection(c *mysql.Conn) (sqlparser.ParserOptions, error) { + ctx, err := h.sm.NewContext(c) + if err != nil { + return sqlparser.ParserOptions{}, err + } + + options := sqlparser.ParserOptions{ + AnsiQuotes: parse.UseAnsiQuotes(ctx), + } + + return options, nil +} + // ConnectionClosed reports that a connection has been closed. func (h *Handler) ConnectionClosed(c *mysql.Conn) { defer func() { diff --git a/sql/analyzer/check_constraints.go b/sql/analyzer/check_constraints.go index e2e4bad86e..7c49fe61d9 100644 --- a/sql/analyzer/check_constraints.go +++ b/sql/analyzer/check_constraints.go @@ -298,6 +298,7 @@ func loadChecksFromTable(ctx *sql.Context, table sql.Table) ([]*sql.CheckConstra func ConvertCheckDefToConstraint(ctx *sql.Context, check *sql.CheckDefinition) (*sql.CheckConstraint, error) { parseStr := fmt.Sprintf("select %s", check.CheckExpression) + // TODO: Move to ParseWithOptions and test ANSI_QUOTES support with check constraints parsed, err := sqlparser.Parse(parseStr) if err != nil { return nil, err From 8ee6e3fd35310a920a698935464c431af209cd88 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 25 Jul 2023 14:56:12 -0700 Subject: [PATCH 03/18] Plugging in initial implementation for ParserOptionsForConnection --- enginetest/enginetests.go | 12 +++ enginetest/memory_engine_test.go | 8 ++ enginetest/queries/ansi_quotes_queries.go | 109 ++++++++++++++++++++++ server/golden/proxy.go | 4 - server/golden/validator.go | 6 +- server/handler.go | 11 ++- sql/sql_mode.go | 70 ++++++++++++++ 7 files changed, 210 insertions(+), 10 deletions(-) create mode 100644 enginetest/queries/ansi_quotes_queries.go create mode 100644 sql/sql_mode.go diff --git a/enginetest/enginetests.go b/enginetest/enginetests.go index 17ebd930c5..6997c2355a 100644 --- a/enginetest/enginetests.go +++ b/enginetest/enginetests.go @@ -381,6 +381,18 @@ func TestReadOnlyVersionedQueries(t *testing.T, harness Harness) { } } +func TestAnsiQuotesSqlMode(t *testing.T, harness Harness) { + for _, tt := range queries.AnsiQuotesTests { + TestScript(t, harness, tt) + } +} + +func TestAnsiQuotesSqlModePrepared(t *testing.T, harness Harness) { + for _, tt := range queries.AnsiQuotesTests { + TestScriptPrepared(t, harness, tt) + } +} + // TestQueryPlans tests generating the correct query plans for various queries using databases and tables provided by // the given harness. func TestQueryPlans(t *testing.T, harness Harness, planTests []queries.QueryPlanTest) { diff --git a/enginetest/memory_engine_test.go b/enginetest/memory_engine_test.go index bb568e39a7..7c7555954f 100644 --- a/enginetest/memory_engine_test.go +++ b/enginetest/memory_engine_test.go @@ -424,6 +424,14 @@ func TestVersionedQueriesPrepared(t *testing.T) { } } +func TestAnsiQuotesSqlMode(t *testing.T) { + enginetest.TestAnsiQuotesSqlMode(t, enginetest.NewDefaultMemoryHarness()) +} + +func TestAnsiQuotesSqlModePrepared(t *testing.T) { + enginetest.TestAnsiQuotesSqlModePrepared(t, enginetest.NewDefaultMemoryHarness()) +} + // Tests of choosing the correct execution plan independent of result correctness. Mostly useful for confirming that // the right indexes are being used for joining tables. func TestQueryPlans(t *testing.T) { diff --git a/enginetest/queries/ansi_quotes_queries.go b/enginetest/queries/ansi_quotes_queries.go new file mode 100644 index 0000000000..510e6c1201 --- /dev/null +++ b/enginetest/queries/ansi_quotes_queries.go @@ -0,0 +1,109 @@ +// Copyright 2023 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package queries + +import ( + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/plan" + "github.com/dolthub/go-mysql-server/sql/types" +) + +// TODO: Test with sql_mode=ANSI, too +// TODO: Look for all places that use the parse function +// TODO: Other things to test? check constraint expressions? Column default values? +var AnsiQuotesTests = []ScriptTest{ + { + Name: "ANSI_QUOTES SQL_Mode", + SetUpScript: []string{ + "SET @@sql_mode='ANSI_QUOTES,NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';", + "create table auctions (ai int auto_increment, id varchar(32), data varchar(100), primary key (ai));", + "insert into auctions (id, data) values (42, 'forty-two');", + }, + Assertions: []ScriptTestAssertion{ + { + // When ANSI_QUOTES mode is enabled, double quotes become identifier quotes. + Query: `select "data" from auctions order by "ai" desc;`, + Expected: []sql.Row{{"forty-two"}}, + }, + { + // Backtick quotes are always valid as identifier characters, even if + // ANSI_QUOTES mode is enabled. + Query: "select `data` from auctions order by `ai` desc;", + Expected: []sql.Row{{"forty-two"}}, + }, + { + Query: `PREPARE prep1 FROM 'select "data" from auctions order by "ai" desc;'`, + Expected: []sql.Row{{types.OkResult{RowsAffected: 0x0, InsertID: 0x0, Info: plan.PrepareInfo{}}}}, + }, + { + Query: `PREPARE prep2 FROM 'INSERT INTO auctions (id, "data") VALUES (?, ?);';`, + Expected: []sql.Row{{types.OkResult{RowsAffected: 0x0, InsertID: 0x0, Info: plan.PrepareInfo{}}}}, + }, + { + Query: `select "data", '"' from auctions order by "ai";`, + Expected: []sql.Row{{"forty-two", "\""}}, + }, + { + Query: `select "data", '\"' from auctions order by "ai";`, + Expected: []sql.Row{{"forty-two", "\""}}, + }, + { + // /~https://github.com/dolthub/dolt/issues/6305 + Query: `CREATE TABLE public_keys (item INTEGER, type CHAR(4), hash INTEGER, len INTEGER, "public" VARCHAR(8000))`, + Expected: []sql.Row{{types.NewOkResult(0)}}, + }, + { + Query: `select '''foo''';`, + Expected: []sql.Row{{`'foo'`}}, + }, + { + Query: `select """""foo""""";`, + Expected: []sql.Row{{`""foo""`}}, + }, + { + Query: `create view view1 as select public_keys."public" from public_keys;`, + Expected: []sql.Row{}, + }, + { + // TODO: Double check this behavior with MySQL + Query: "select ```foo```;", + ExpectedErrStr: "column \"`foo`\" could not be found in any table in scope", + }, + { + // Disable ANSI_QUOTES and make sure we can still run queries + Query: `SET @@sql_mode='NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, + Expected: []sql.Row{{}}, + }, + { + Query: `select "data" from auctions order by "ai" desc;`, + Expected: []sql.Row{{"data"}}, + }, + { + Query: `show tables;`, + Expected: []sql.Row{{"auctions"}, {"myview"}, {"public_keys"}, {"view1"}}, + }, + { + // TODO: This is failing, because we can't parse the view definition + // TODO: Should we always record views and fragments in non-ANSI_QUOTES format? What does MySQL do? + Query: `show create table view1;`, + Expected: []sql.Row{{`""foo""`}}, + }, + { + Query: `show create table public_keys;`, + Expected: []sql.Row{{"public_keys", "CREATE TABLE `public_keys` (\n `item` int,\n `type` char(4),\n `hash` int,\n `len` int,\n `public` varchar(8000)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}}, + }, + }, + }, +} diff --git a/server/golden/proxy.go b/server/golden/proxy.go index 970423c32f..e3220db1d9 100644 --- a/server/golden/proxy.go +++ b/server/golden/proxy.go @@ -205,10 +205,6 @@ func (h MySqlProxy) ComQuery( return err } -func (h MySqlProxy) ParserOptionsForConnection(c *mysql.Conn) (sqlparser.ParserOptions, error) { - return sqlparser.ParserOptions{}, nil -} - func (h MySqlProxy) processQuery( c *mysql.Conn, proxy proxyConn, diff --git a/server/golden/validator.go b/server/golden/validator.go index 6fe064fd27..ebae109784 100644 --- a/server/golden/validator.go +++ b/server/golden/validator.go @@ -38,10 +38,6 @@ type Validator struct { logger *logrus.Logger } -func (v Validator) ParserOptionsForConnection(_ *mysql.Conn) (sqlparser.ParserOptions, error) { - return sqlparser.ParserOptions{}, nil -} - // NewValidatingHandler creates a new Validator wrapping a MySQL connection. func NewValidatingHandler(handler mysql.Handler, mySqlConn string, logger *logrus.Logger) (Validator, error) { golden, err := NewMySqlProxyHandler(logger, mySqlConn) @@ -156,7 +152,7 @@ func (v Validator) WarningCount(c *mysql.Conn) uint16 { return 0 } -func (v Validator) ParserOptionsForConnection(c *mysql.Conn) (sqlparser.ParserOptions, error) { +func (v Validator) ParserOptionsForConnection(_ *mysql.Conn) (sqlparser.ParserOptions, error) { return sqlparser.ParserOptions{}, nil } diff --git a/server/handler.go b/server/handler.go index 7d8c85431c..12cb4184bf 100644 --- a/server/handler.go +++ b/server/handler.go @@ -133,7 +133,16 @@ func (h *Handler) ComResetConnection(c *mysql.Conn) { } func (h *Handler) ParserOptionsForConnection(c *mysql.Conn) (sqlparser.ParserOptions, error) { - return sqlparser.ParserOptions{}, nil + ctx, err := h.sm.NewContext(c) + if err != nil { + return sqlparser.ParserOptions{}, err + } + + sqlMode, err := sql.LoadSqlMode(ctx) + if err != nil { + return sqlparser.ParserOptions{}, err + } + return sqlMode.ParserOptions(), nil } // ConnectionClosed reports that a connection has been closed. diff --git a/sql/sql_mode.go b/sql/sql_mode.go new file mode 100644 index 0000000000..2ffcf99eac --- /dev/null +++ b/sql/sql_mode.go @@ -0,0 +1,70 @@ +// Copyright 2023 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package sql + +import ( + "fmt" + "github.com/dolthub/vitess/go/vt/sqlparser" + "strings" +) + +// SqlMode encodes the SQL mode string and provides methods for querying the enabled modes. +type SqlMode struct { + modes map[string]struct{} +} + +// LoadSqlMode loads the SQL mode using the session data contained in |ctx| and returns a SqlMode +// instance that can be used to query which modes are enabled. +func LoadSqlMode(ctx *Context) (*SqlMode, error) { + sqlMode, err := ctx.Session.GetSessionVariable(ctx, "SQL_MODE") + if err != nil { + return nil, err + } + + sqlModeString, ok := sqlMode.(string) + if !ok { + return nil, fmt.Errorf("unable to parse sql_mode: %v", sqlMode) + } + + sqlModeString = strings.ToLower(sqlModeString) + elements := strings.Split(sqlModeString, ",") + modes := map[string]struct{}{} + for _, element := range elements { + modes[element] = struct{}{} + } + + return &SqlMode{ + modes: modes, + }, nil +} + +// AnsiQuotes returns true if the ANSI_QUOTES SQL mode is enabled. Note that the ANSI mode is a compound mode that +// includes ANSI_QUOTES and other options, so if ANSI or ANSI_QUOTES is enabled, this function will return true. +func (s *SqlMode) AnsiQuotes() bool { + return s.ModeEnabled("ansi_quotes") || s.ModeEnabled("ansi") +} + +// ModeEnabled returns true if |mode| is enabled in the SQL mode string. +func (s *SqlMode) ModeEnabled(mode string) bool { + _, ok := s.modes[strings.ToLower(mode)] + return ok +} + +// ParserOptions returns a ParserOptions struct, with options set based on what SQL modes are enabled. +func (s *SqlMode) ParserOptions() sqlparser.ParserOptions { + return sqlparser.ParserOptions{ + AnsiQuotes: s.AnsiQuotes(), + } +} From 7262e2e295a396899913f1c0f439316255381f8e Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Fri, 28 Jul 2023 12:17:44 -0700 Subject: [PATCH 04/18] First pass at support for ANSI_QUOTES SQL mode in GMS --- enginetest/queries/ansi_quotes_queries.go | 196 +++++++++++++++++++--- memory/database.go | 12 +- server/handler.go | 9 + sql/analyzer/load_events.go | 6 +- sql/analyzer/load_triggers.go | 6 +- sql/analyzer/resolve_views.go | 5 +- sql/analyzer/triggers.go | 3 +- sql/databases.go | 4 + sql/events.go | 2 + sql/parse/parse.go | 41 ++++- sql/plan/ddl_event.go | 8 +- sql/rowexec/ddl.go | 6 + 12 files changed, 263 insertions(+), 35 deletions(-) diff --git a/enginetest/queries/ansi_quotes_queries.go b/enginetest/queries/ansi_quotes_queries.go index 510e6c1201..5d2cd99a08 100644 --- a/enginetest/queries/ansi_quotes_queries.go +++ b/enginetest/queries/ansi_quotes_queries.go @@ -20,12 +20,10 @@ import ( "github.com/dolthub/go-mysql-server/sql/types" ) -// TODO: Test with sql_mode=ANSI, too -// TODO: Look for all places that use the parse function -// TODO: Other things to test? check constraint expressions? Column default values? +// TODO: Audit all places that use the vitess and GMS parse functions var AnsiQuotesTests = []ScriptTest{ { - Name: "ANSI_QUOTES SQL_Mode", + Name: "ANSI_QUOTES: basic cases", SetUpScript: []string{ "SET @@sql_mode='ANSI_QUOTES,NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';", "create table auctions (ai int auto_increment, id varchar(32), data varchar(100), primary key (ai));", @@ -59,28 +57,20 @@ var AnsiQuotesTests = []ScriptTest{ Query: `select "data", '\"' from auctions order by "ai";`, Expected: []sql.Row{{"forty-two", "\""}}, }, - { - // /~https://github.com/dolthub/dolt/issues/6305 - Query: `CREATE TABLE public_keys (item INTEGER, type CHAR(4), hash INTEGER, len INTEGER, "public" VARCHAR(8000))`, - Expected: []sql.Row{{types.NewOkResult(0)}}, - }, { Query: `select '''foo''';`, Expected: []sql.Row{{`'foo'`}}, }, { - Query: `select """""foo""""";`, - Expected: []sql.Row{{`""foo""`}}, - }, - { - Query: `create view view1 as select public_keys."public" from public_keys;`, - Expected: []sql.Row{}, + Query: `select """""foo""""";`, + ExpectedErrStr: `column "\"\"foo\"\"" could not be found in any table in scope`, }, { // TODO: Double check this behavior with MySQL Query: "select ```foo```;", ExpectedErrStr: "column \"`foo`\" could not be found in any table in scope", }, + // --- Assertions AFTER turning ANSI_QUOTES off --- { // Disable ANSI_QUOTES and make sure we can still run queries Query: `SET @@sql_mode='NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, @@ -92,17 +82,183 @@ var AnsiQuotesTests = []ScriptTest{ }, { Query: `show tables;`, - Expected: []sql.Row{{"auctions"}, {"myview"}, {"public_keys"}, {"view1"}}, + Expected: []sql.Row{{"auctions"}, {"myview"}}, + }, + }, + }, + { + Name: "ANSI_QUOTES: ANSI mode includes ANSI_QUOTES", + SetUpScript: []string{ + `SET @@sql_mode='ANSI';`, + }, + Assertions: []ScriptTestAssertion{ + { + // Assert that we can create a table using ANSI style quotes + Query: `create table "t" ("pk" int primary key, "data" varchar(100));`, + Expected: []sql.Row{{types.NewOkResult(0)}}, + }, + { + Query: `insert into t ("pk", "data") values (1, 'one');`, + Expected: []sql.Row{{types.NewOkResult(1)}}, + }, + { + Query: `select "pk", "data" from "t" order by "pk" asc;`, + Expected: []sql.Row{{1, "one"}}, + }, + }, + }, + { + Name: "ANSI_QUOTES: views", + SetUpScript: []string{ + `SET @@sql_mode='ANSI_QUOTES,NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, + }, + Assertions: []ScriptTestAssertion{ + { + // /~https://github.com/dolthub/dolt/issues/6305 + Query: `CREATE TABLE public_keys (item INTEGER, type CHAR(4), hash INTEGER, "count" INTEGER, "public" VARCHAR(8000))`, + Expected: []sql.Row{{types.NewOkResult(0)}}, + }, + { + Query: `create view view1 as select public_keys."public", public_keys."count" from public_keys;`, + Expected: []sql.Row{}, + }, + { + Query: `show tables;`, + Expected: []sql.Row{{"myview"}, {"public_keys"}, {"view1"}}, + }, + { + Query: `show create table view1;`, + Expected: []sql.Row{{"view1", "CREATE VIEW `view1` AS select public_keys.\"public\", public_keys.\"count\" from public_keys", "utf8mb4", "utf8mb4_0900_bin"}}, + }, + { + // Disable ANSI_QUOTES mode + Query: `SET @@sql_mode='NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, + Expected: []sql.Row{{}}, }, { - // TODO: This is failing, because we can't parse the view definition - // TODO: Should we always record views and fragments in non-ANSI_QUOTES format? What does MySQL do? Query: `show create table view1;`, - Expected: []sql.Row{{`""foo""`}}, + Expected: []sql.Row{{"view1", "CREATE VIEW `view1` AS select public_keys.\"public\", public_keys.\"count\" from public_keys", "utf8mb4", "utf8mb4_0900_bin"}}, }, { Query: `show create table public_keys;`, - Expected: []sql.Row{{"public_keys", "CREATE TABLE `public_keys` (\n `item` int,\n `type` char(4),\n `hash` int,\n `len` int,\n `public` varchar(8000)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}}, + Expected: []sql.Row{{"public_keys", "CREATE TABLE `public_keys` (\n `item` int,\n `type` char(4),\n `hash` int,\n `count` int,\n `public` varchar(8000)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}}, + }, + }, + }, + { + Name: "ANSI_QUOTES: triggers", + SetUpScript: []string{ + `SET @@sql_mode='ANSI_QUOTES,NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, + `create table t (pk int primary key, name varchar(32), data varchar(100));`, + `create trigger ansi_quotes_trigger BEFORE INSERT ON "t" FOR EACH ROW SET new."data" = 'triggered!';`, + `insert into t values (1, 'John', 'FooBar');`, + }, + Assertions: []ScriptTestAssertion{ + { + // Assert the trigger ran correctly with ANSI_QUOTES mode enabled + Query: `select "name", "data" from t order by "pk";`, + Expected: []sql.Row{{"John", "triggered!"}}, + }, + { + // Disable ANSI_QUOTES mode + Query: `SET @@sql_mode='NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, + Expected: []sql.Row{{}}, + }, + { + Query: `insert into t values (2, 'George', 'SomethingElse');`, + Expected: []sql.Row{{types.NewOkResult(1)}}, + }, + { + // Assert the trigger still runs correctly after disabling ANSI_QUOTES mode + Query: `select name, data from t where pk=2;`, + Expected: []sql.Row{{"George", "triggered!"}}, + }, + }, + }, + { + // TODO: How about when we merge? We apply check constraints and column default there, too + // (and they both use the new planbuilder package) + Name: "ANSI_QUOTES: column defaults", + SetUpScript: []string{ + `SET @@sql_mode='ANSI_QUOTES,NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, + `create table t ("pk" int primary key, "name" varchar(20), data varchar(100) DEFAULT(CONCAT("name", '!')));`, + `insert into t (pk, name) values (1, 'John');`, + }, + Assertions: []ScriptTestAssertion{ + { + // Assert the column default is applied correctly when ANSI_QUOTES mode is enabled + Query: `select "name", "data" from t where "pk"=1;`, + Expected: []sql.Row{{"John", "John!"}}, + }, + { + // Disable ANSI_QUOTES mode + Query: `SET @@sql_mode='NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, + Expected: []sql.Row{{}}, + }, + { + // Insert a row with ANSI_QUOTES mode disabled + // TODO: Using DEFAULT in the values clause doesn't seem to work? + Query: `insert into t (pk, name) values (2, 'Jill');`, + Expected: []sql.Row{{types.NewOkResult(1)}}, + }, + { + // Assert the column default was applied correctly when ANSI_QUOTES mode is disabled + Query: `select name, data from t where pk=2;`, + Expected: []sql.Row{{"Jill", "Jill!"}}, + }, + }, + }, + { + Name: "ANSI_QUOTES: check constraints", + SetUpScript: []string{ + `SET @@sql_mode='ANSI_QUOTES,NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, + `create table t (pk int primary key, data varchar(100), CONSTRAINT ansi_check CHECK ("data" != 'forbidden'));`, + }, + Assertions: []ScriptTestAssertion{ + { + // Assert the check constraint runs correctly in ANSI_QUOTES mode + Query: `insert into t values (1, 'forbidden');`, + ExpectedErrStr: `Check constraint "ansi_check" violated`, + }, + { + // Disable ANSI_QUOTES mode + Query: `SET @@sql_mode='NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, + Expected: []sql.Row{{}}, + }, + { + // Assert the check constraint runs correctly when ANSI_QUOTES mode is disabled + Query: `insert into t values (1, 'forbidden');`, + ExpectedErrStr: `Check constraint "ansi_check" violated`, + }, + }, + }, + { + Name: "ANSI_QUOTES: events", + SetUpScript: []string{ + `SET @@sql_mode='ANSI_QUOTES,NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, + `create table t (pk int primary key, "count" int);`, + `insert into t values (1, 0);`, + }, + Assertions: []ScriptTestAssertion{ + { + // Assert the check constraint runs correctly in ANSI_QUOTES mode + Query: `CREATE EVENT myevent + ON SCHEDULE EVERY 1 SECOND STARTS '2037-10-16 23:59:00' DO + UPDATE "t" SET "count"="count"+1;`, + Expected: []sql.Row{{types.NewOkResult(0)}}, + }, + { + Query: `SHOW EVENTS;`, + Expected: []sql.Row{{"mydb", "myevent", "`root`@`localhost`", "SYSTEM", "RECURRING", nil, "1", "SECOND", "2037-10-16 23:59:00", nil, "ENABLED", 0, "utf8mb4", "utf8mb4_0900_bin", "utf8mb4_0900_bin"}}, + }, + { + // Disable ANSI_QUOTES mode and make sure we can still list and run events + Query: `SET @@sql_mode='NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, + Expected: []sql.Row{{}}, + }, + { + Query: `SHOW EVENTS;`, + Expected: []sql.Row{{"mydb", "myevent", "`root`@`localhost`", "SYSTEM", "RECURRING", nil, "1", "SECOND", "2037-10-16 23:59:00", nil, "ENABLED", 0, "utf8mb4", "utf8mb4_0900_bin", "utf8mb4_0900_bin"}}, }, }, }, diff --git a/memory/database.go b/memory/database.go index 9897080351..5f60139e7e 100644 --- a/memory/database.go +++ b/memory/database.go @@ -407,7 +407,17 @@ func (d *Database) CreateView(ctx *sql.Context, name string, selectStatement, cr return sql.ErrExistingView.New(name) } - d.views[name] = sql.ViewDefinition{Name: name, TextDefinition: selectStatement, CreateViewStatement: createViewStmt} + sqlMode, err := sql.LoadSqlMode(ctx) + if err != nil { + return err + } + + d.views[name] = sql.ViewDefinition{ + Name: name, + TextDefinition: selectStatement, + CreateViewStatement: createViewStmt, + AnsiQuotes: sqlMode.AnsiQuotes(), + } return nil } diff --git a/server/handler.go b/server/handler.go index 12cb4184bf..f700217719 100644 --- a/server/handler.go +++ b/server/handler.go @@ -26,6 +26,7 @@ import ( "github.com/dolthub/vitess/go/mysql" "github.com/dolthub/vitess/go/netutil" "github.com/dolthub/vitess/go/sqltypes" + "github.com/dolthub/vitess/go/vt/log" "github.com/dolthub/vitess/go/vt/proto/query" "github.com/dolthub/vitess/go/vt/sqlparser" "github.com/go-kit/kit/metrics/discard" @@ -65,6 +66,14 @@ const ( MultiStmtModeOn MultiStmtMode = 1 ) +func init() { + // Set the log.Error and log.Errorf functions in Vitess so that any errors + // logged by Vitess will appear in our logs. Without this, errors from Vitess + // can be silently swallowed, which makes debugging failures harder. + log.Error = logrus.StandardLogger().Error + log.Errorf = logrus.StandardLogger().Errorf +} + // Handler is a connection handler for a SQLe engine, implementing the Vitess mysql.Handler interface. type Handler struct { e *sqle.Engine diff --git a/sql/analyzer/load_events.go b/sql/analyzer/load_events.go index a22a16ca37..812f5bdbe7 100644 --- a/sql/analyzer/load_events.go +++ b/sql/analyzer/load_events.go @@ -15,11 +15,12 @@ package analyzer import ( - "github.com/dolthub/go-mysql-server/sql/transform" + "github.com/dolthub/vitess/go/vt/sqlparser" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/parse" "github.com/dolthub/go-mysql-server/sql/plan" + "github.com/dolthub/go-mysql-server/sql/transform" ) // loadEvents loads any events that are required for a plan node to operate properly (except for nodes dealing with @@ -86,7 +87,8 @@ func loadEventFromDb(ctx *sql.Context, db sql.Database, name string) (sql.EventD } func getEventDetailsFromEventDefinition(ctx *sql.Context, event sql.EventDefinition) (sql.EventDetails, error) { - parsedCreateEvent, err := parse.Parse(ctx, event.CreateStatement) + parsedCreateEvent, err := parse.ParseWithOptions(ctx, event.CreateStatement, + sqlparser.ParserOptions{AnsiQuotes: event.AnsiQuotes}) if err != nil { return sql.EventDetails{}, err } diff --git a/sql/analyzer/load_triggers.go b/sql/analyzer/load_triggers.go index fb5a18e93b..b5d99bc572 100644 --- a/sql/analyzer/load_triggers.go +++ b/sql/analyzer/load_triggers.go @@ -17,11 +17,12 @@ package analyzer import ( "strings" - "github.com/dolthub/go-mysql-server/sql/transform" + "github.com/dolthub/vitess/go/vt/sqlparser" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/parse" "github.com/dolthub/go-mysql-server/sql/plan" + "github.com/dolthub/go-mysql-server/sql/transform" ) // loadTriggers loads any triggers that are required for a plan node to operate properly (except for nodes dealing with @@ -104,7 +105,8 @@ func loadTriggersFromDb(ctx *sql.Context, db sql.Database) ([]*plan.CreateTrigge return nil, err } for _, trigger := range triggers { - parsedTrigger, err := parse.Parse(ctx, trigger.CreateStatement) + parsedTrigger, err := parse.ParseWithOptions(ctx, trigger.CreateStatement, + sqlparser.ParserOptions{AnsiQuotes: trigger.AnsiQuotes}) if err != nil { return nil, err } diff --git a/sql/analyzer/resolve_views.go b/sql/analyzer/resolve_views.go index e0217b9d82..8d99dc269c 100644 --- a/sql/analyzer/resolve_views.go +++ b/sql/analyzer/resolve_views.go @@ -17,6 +17,8 @@ package analyzer import ( "fmt" + "github.com/dolthub/vitess/go/vt/sqlparser" + "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/parse" "github.com/dolthub/go-mysql-server/sql/plan" @@ -56,7 +58,8 @@ func resolveViews(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scope, return nil, transform.SameTree, verr } if vdok { - query, qerr := parse.Parse(ctx, viewDef.TextDefinition) + query, qerr := parse.ParseWithOptions(ctx, viewDef.TextDefinition, + sqlparser.ParserOptions{AnsiQuotes: viewDef.AnsiQuotes}) if qerr != nil { return nil, transform.SameTree, qerr } diff --git a/sql/analyzer/triggers.go b/sql/analyzer/triggers.go index a5663b4e76..6642bb42d0 100644 --- a/sql/analyzer/triggers.go +++ b/sql/analyzer/triggers.go @@ -184,7 +184,8 @@ func applyTriggers(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scope, } for _, trigger := range triggers { - parsedTrigger, err := parse.Parse(ctx, trigger.CreateStatement) + parsedTrigger, err := parse.ParseWithOptions(ctx, trigger.CreateStatement, + sqlparser.ParserOptions{AnsiQuotes: trigger.AnsiQuotes}) if err != nil { return nil, transform.SameTree, err } diff --git a/sql/databases.go b/sql/databases.go index 20131311ae..41f9f0a23a 100644 --- a/sql/databases.go +++ b/sql/databases.go @@ -189,6 +189,9 @@ type TriggerDefinition struct { CreateStatement string // The time that the trigger was created. CreatedAt time.Time + // AnsiQuotes is true if this trigger definition was defined using the ANSI_QUOTES SQL mode, meaning that any + // double quotes should be treated as identifier quotes, instead of string literal quotes. + AnsiQuotes bool } // TemporaryTableDatabase is a database that can query the session (which manages the temporary table state) to @@ -263,6 +266,7 @@ type ViewDefinition struct { Name string TextDefinition string CreateViewStatement string + AnsiQuotes bool } // GetTableInsensitive implements a case-insensitive map lookup for tables keyed off of the table name. diff --git a/sql/events.go b/sql/events.go index 380a162887..47b577636b 100644 --- a/sql/events.go +++ b/sql/events.go @@ -32,6 +32,8 @@ type EventDefinition struct { CreatedAt time.Time // The time that the event was last altered. LastAltered time.Time + // True if this event definition relies on ANSI_QUOTES mode for parsing its SQL + AnsiQuotes bool } // EventDetails are the details of the event. diff --git a/sql/parse/parse.go b/sql/parse/parse.go index b80e9f61a8..694dec35e8 100644 --- a/sql/parse/parse.go +++ b/sql/parse/parse.go @@ -69,17 +69,39 @@ const ( colKeyFulltextKey ) -// Parse parses the given SQL sentence and returns the corresponding node. +// Parse parses the given SQL |query| and returns the corresponding node. The +// query will be parsed with the current session's SQL mode. Use the ParseWithOptions +// function if you want to control what SQL mode is used for parsing the query. func Parse(ctx *sql.Context, query string) (sql.Node, error) { - n, _, _, err := parse(ctx, query, false) + sqlMode, err := sql.LoadSqlMode(ctx) + if err != nil { + return nil, err + } + + n, _, _, err := parse(ctx, query, false, sqlMode.ParserOptions()) + return n, err +} + +// ParseWithOptions parses the specified |query| using the specified |options| which +// control parser behavior (e.g. ANSI_QUOTES mode). +func ParseWithOptions(ctx *sql.Context, query string, options sqlparser.ParserOptions) (sql.Node, error) { + n, _, _, err := parse(ctx, query, false, options) return n, err } +// ParseOne parses the first SQL statement from |query| and returns the sql.Node, the +// statement that was parsed, and the remainder of |query| that was not parsed. The +// statement will be parsed with the current session's SQL mode. func ParseOne(ctx *sql.Context, query string) (sql.Node, string, string, error) { - return parse(ctx, query, true) + sqlMode, err := sql.LoadSqlMode(ctx) + if err != nil { + return nil, "", "", err + } + + return parse(ctx, query, true, sqlMode.ParserOptions()) } -func parse(ctx *sql.Context, query string, multi bool) (sql.Node, string, string, error) { +func parse(ctx *sql.Context, query string, multi bool, options sqlparser.ParserOptions) (sql.Node, string, string, error) { span, ctx := ctx.Span("parse", trace.WithAttributes(attribute.String("query", query))) defer span.End() @@ -96,10 +118,10 @@ func parse(ctx *sql.Context, query string, multi bool) (sql.Node, string, string parsed = s if !multi { - stmt, err = sqlparser.Parse(s) + stmt, err = sqlparser.ParseWithOptions(s, options) } else { var ri int - stmt, ri, err = sqlparser.ParseOne(s) + stmt, ri, err = sqlparser.ParseOneWithOptions(s, options) if ri != 0 && ri < len(s) { parsed = s[:ri] parsed = strings.TrimSpace(parsed) @@ -464,7 +486,12 @@ func convertPrepare(ctx *sql.Context, n *sqlparser.Prepare) (sql.Node, error) { } } - childStmt, err := sqlparser.Parse(expr) + sqlMode, err := sql.LoadSqlMode(ctx) + if err != nil { + return nil, err + } + + childStmt, err := sqlparser.ParseWithOptions(expr, sqlMode.ParserOptions()) if err != nil { return nil, err } diff --git a/sql/plan/ddl_event.go b/sql/plan/ddl_event.go index c8d4d6f186..c9c8778b0b 100644 --- a/sql/plan/ddl_event.go +++ b/sql/plan/ddl_event.go @@ -321,14 +321,20 @@ func (c *createEventIter) Next(ctx *sql.Context) (sql.Row, error) { } } + sqlMode, err := sql.LoadSqlMode(ctx) + if err != nil { + return nil, err + } + var eventDefinition = sql.EventDefinition{ Name: c.eventDetails.Name, CreateStatement: c.eventDetails.CreateEventStatement(), CreatedAt: c.eventDetails.Created, LastAltered: c.eventDetails.LastAltered, + AnsiQuotes: sqlMode.AnsiQuotes(), } - err := c.eventDb.SaveEvent(ctx, eventDefinition) + err = c.eventDb.SaveEvent(ctx, eventDefinition) if err != nil { if sql.ErrEventAlreadyExists.Is(err) && c.ifNotExists { ctx.Session.Warn(&sql.Warning{ diff --git a/sql/rowexec/ddl.go b/sql/rowexec/ddl.go index ced304087c..76915d6db9 100644 --- a/sql/rowexec/ddl.go +++ b/sql/rowexec/ddl.go @@ -925,11 +925,17 @@ func (b *BaseBuilder) buildCreateProcedure(ctx *sql.Context, n *plan.CreateProce } func (b *BaseBuilder) buildCreateTrigger(ctx *sql.Context, n *plan.CreateTrigger, row sql.Row) (sql.RowIter, error) { + sqlMode, err := sql.LoadSqlMode(ctx) + if err != nil { + return nil, err + } + return &createTriggerIter{ definition: sql.TriggerDefinition{ Name: n.TriggerName, CreateStatement: n.CreateTriggerString, CreatedAt: n.CreatedAt, + AnsiQuotes: sqlMode.AnsiQuotes(), }, db: n.Database(), }, nil From 2a7987848e9cae758f122b7c0509f59f7fa2fa99 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Mon, 31 Jul 2023 16:17:21 -0700 Subject: [PATCH 05/18] Fixing information_schema support for triggers that use ANSI_QUOTES --- enginetest/queries/ansi_quotes_queries.go | 10 ++++++++++ sql/information_schema/information_schema.go | 5 ++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/enginetest/queries/ansi_quotes_queries.go b/enginetest/queries/ansi_quotes_queries.go index 5d2cd99a08..9e78236eac 100644 --- a/enginetest/queries/ansi_quotes_queries.go +++ b/enginetest/queries/ansi_quotes_queries.go @@ -159,6 +159,11 @@ var AnsiQuotesTests = []ScriptTest{ Query: `select "name", "data" from t order by "pk";`, Expected: []sql.Row{{"John", "triggered!"}}, }, + { + // Assert that we can read and parse the trigger definition from information_schema + Query: `select action_statement from information_schema.triggers where trigger_name='ansi_quotes_trigger';`, + Expected: []sql.Row{{`SET new."data" = 'triggered!'`}}, + }, { // Disable ANSI_QUOTES mode Query: `SET @@sql_mode='NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, @@ -173,6 +178,11 @@ var AnsiQuotesTests = []ScriptTest{ Query: `select name, data from t where pk=2;`, Expected: []sql.Row{{"George", "triggered!"}}, }, + { + // Assert that we can still read and parse the trigger definition from information_schema + Query: `select action_statement from information_schema.triggers where trigger_name='ansi_quotes_trigger';`, + Expected: []sql.Row{{`SET new."data" = 'triggered!'`}}, + }, }, }, { diff --git a/sql/information_schema/information_schema.go b/sql/information_schema/information_schema.go index e1f18a3946..927d39e4a3 100644 --- a/sql/information_schema/information_schema.go +++ b/sql/information_schema/information_schema.go @@ -1806,6 +1806,8 @@ func triggersRowIter(ctx *Context, c Catalog) (RowIter, error) { if err != nil { return nil, err } + + // TODO: This should be the SQL_MODE at the time the trigger was created, not the current session's SQL_MODE sysVal, err := ctx.Session.GetSessionVariable(ctx, "sql_mode") if err != nil { return nil, err @@ -1836,7 +1838,8 @@ func triggersRowIter(ctx *Context, c Catalog) (RowIter, error) { var triggerPlans []*plan.CreateTrigger for _, trigger := range triggers { - parsedTrigger, err := parse.Parse(ctx, trigger.CreateStatement) + parsedTrigger, err := parse.ParseWithOptions(ctx, trigger.CreateStatement, + sqlparser.ParserOptions{AnsiQuotes: trigger.AnsiQuotes}) if err != nil { return nil, err } From e1b43406884358912366826c6f09a4c43eb16040 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Mon, 31 Jul 2023 16:22:16 -0700 Subject: [PATCH 06/18] Fixing information_schema support for views that use ANSI_QUOTES --- enginetest/queries/ansi_quotes_queries.go | 10 ++++++++++ sql/information_schema/information_schema.go | 3 ++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/enginetest/queries/ansi_quotes_queries.go b/enginetest/queries/ansi_quotes_queries.go index 9e78236eac..ef586ac617 100644 --- a/enginetest/queries/ansi_quotes_queries.go +++ b/enginetest/queries/ansi_quotes_queries.go @@ -130,6 +130,11 @@ var AnsiQuotesTests = []ScriptTest{ Query: `show create table view1;`, Expected: []sql.Row{{"view1", "CREATE VIEW `view1` AS select public_keys.\"public\", public_keys.\"count\" from public_keys", "utf8mb4", "utf8mb4_0900_bin"}}, }, + { + // Assert that we can load and parse views for information_schema when ANSI_QUOTES mode is enabled + Query: `select table_name, view_definition from information_schema.views where table_name='view1';`, + Expected: []sql.Row{{"view1", `select public_keys."public", public_keys."count" from public_keys`}}, + }, { // Disable ANSI_QUOTES mode Query: `SET @@sql_mode='NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, @@ -143,6 +148,11 @@ var AnsiQuotesTests = []ScriptTest{ Query: `show create table public_keys;`, Expected: []sql.Row{{"public_keys", "CREATE TABLE `public_keys` (\n `item` int,\n `type` char(4),\n `hash` int,\n `count` int,\n `public` varchar(8000)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}}, }, + { + // Assert that we can still load and parse views for information_schema when ANSI_QUOTES mode is disabled + Query: `select table_name, view_definition from information_schema.views where table_name='view1';`, + Expected: []sql.Row{{"view1", `select public_keys."public", public_keys."count" from public_keys`}}, + }, }, }, { diff --git a/sql/information_schema/information_schema.go b/sql/information_schema/information_schema.go index 927d39e4a3..fc8922edff 100644 --- a/sql/information_schema/information_schema.go +++ b/sql/information_schema/information_schema.go @@ -2044,7 +2044,8 @@ func viewsRowIter(ctx *Context, catalog Catalog) (RowIter, error) { if !hasGlobalShowViewPriv && !hasDbShowViewPriv && !privTblSet.Has(PrivilegeType_ShowView) { continue } - parsedView, err := parse.Parse(ctx, view.CreateViewStatement) + parsedView, err := parse.ParseWithOptions(ctx, view.CreateViewStatement, + sqlparser.ParserOptions{AnsiQuotes: view.AnsiQuotes}) if err != nil { return nil, err } From a671eaad4b163db7dfa3ec23776efdb5ff7e8978 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Mon, 31 Jul 2023 17:27:29 -0700 Subject: [PATCH 07/18] Fixing support for stored procedures that use ANSI_QUOTES (including information_schema.ROUTINES support) --- enginetest/queries/ansi_quotes_queries.go | 36 +++++++ sql/analyzer/assign_routines.go | 29 +++-- sql/analyzer/stored_procedures.go | 7 +- sql/information_schema/routines_table.go | 126 +++++++++++++--------- sql/procedures.go | 1 + sql/rowexec/ddl.go | 5 + 6 files changed, 135 insertions(+), 69 deletions(-) diff --git a/enginetest/queries/ansi_quotes_queries.go b/enginetest/queries/ansi_quotes_queries.go index ef586ac617..255c484968 100644 --- a/enginetest/queries/ansi_quotes_queries.go +++ b/enginetest/queries/ansi_quotes_queries.go @@ -195,6 +195,42 @@ var AnsiQuotesTests = []ScriptTest{ }, }, }, + { + Name: "ANSI_QUOTES: stored procedures", + SetUpScript: []string{ + `SET @@sql_mode='ANSI_QUOTES,NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, + `create table t (pk int primary key, name varchar(32), data varchar(100));`, + `create procedure AnsiProcedure() BEGIN SELECT "name" from "t" where "pk" = 1; END`, + `insert into t values (1, 'John', 'FooBar');`, + }, + Assertions: []ScriptTestAssertion{ + { + // Assert the procedure runs correctly with ANSI_QUOTES mode enabled + Query: `call AnsiProcedure();`, + Expected: []sql.Row{{"John"}}, + }, + { + // Assert that we can read and parse the procedure definition from information_schema + Query: `select routine_definition from information_schema.routines where routine_name='AnsiProcedure';`, + Expected: []sql.Row{{`BEGIN SELECT "name" from "t" where "pk" = 1; END`}}, + }, + { + // Disable ANSI_QUOTES mode + Query: `SET @@sql_mode='NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, + Expected: []sql.Row{{}}, + }, + { + // Assert the procedure runs correctly with ANSI_QUOTES mode disabled + Query: `call AnsiProcedure();`, + Expected: []sql.Row{{"John"}}, + }, + { + // Assert that we can read and parse the procedure definition from information_schema + Query: `select routine_definition from information_schema.routines where routine_name='AnsiProcedure';`, + Expected: []sql.Row{{`BEGIN SELECT "name" from "t" where "pk" = 1; END`}}, + }, + }, + }, { // TODO: How about when we merge? We apply check constraints and column default there, too // (and they both use the new planbuilder package) diff --git a/sql/analyzer/assign_routines.go b/sql/analyzer/assign_routines.go index ac186495af..d5da07c169 100644 --- a/sql/analyzer/assign_routines.go +++ b/sql/analyzer/assign_routines.go @@ -25,7 +25,7 @@ type RoutineTable interface { sql.Table // AssignProcedures assigns a map of db-procedures to the routines table. - AssignProcedures(p map[string][]*plan.Procedure) sql.Table + AssignProcedures(p map[string][]sql.StoredProcedureDetails) // TODO: also should assign FUNCTIONS } @@ -43,26 +43,23 @@ func assignRoutines(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scope case *plan.ResolvedTable: nc := *node ct, ok := nc.Table.(RoutineTable) - - var err error - scope, err = loadStoredProcedures(ctx, a, n, scope, sel) - if err != nil { - return nil, false, err + if !ok { + return node, transform.SameTree, nil } - dbs := a.Catalog.AllDatabases(ctx) - pm := make(map[string][]*plan.Procedure) - for _, db := range dbs { - if scope != nil && scope.Procedures != nil { - pm[db.Name()] = scope.Procedures.AllForDatabase(db.Name()) + procedureMap := make(map[string][]sql.StoredProcedureDetails) + for _, db := range a.Catalog.AllDatabases(ctx) { + if storedProcedureDb, ok := db.(sql.StoredProcedureDatabase); ok { + procedures, err := storedProcedureDb.GetStoredProcedures(ctx) + if err != nil { + return node, transform.SameTree, err + } + procedureMap[db.Name()] = procedures } } - if ok { - nc.Table = ct.AssignProcedures(pm) - return &nc, transform.NewTree, nil - } - return node, transform.SameTree, nil + ct.AssignProcedures(procedureMap) + return &nc, transform.NewTree, nil default: return node, transform.SameTree, nil } diff --git a/sql/analyzer/stored_procedures.go b/sql/analyzer/stored_procedures.go index e4bdcdc3e3..80ee8c1f54 100644 --- a/sql/analyzer/stored_procedures.go +++ b/sql/analyzer/stored_procedures.go @@ -25,6 +25,7 @@ import ( "github.com/dolthub/go-mysql-server/sql/parse" "github.com/dolthub/go-mysql-server/sql/plan" "github.com/dolthub/go-mysql-server/sql/transform" + "github.com/dolthub/vitess/go/vt/sqlparser" ) // loadStoredProcedures loads non-built-in stored procedures for all databases on relevant calls. @@ -51,7 +52,8 @@ func loadStoredProcedures(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan } for _, procedure := range procedures { - parsedProcedure, err := parse.Parse(ctx, procedure.CreateStatement) + parsedProcedure, err := parse.ParseWithOptions(ctx, procedure.CreateStatement, + sqlparser.ParserOptions{AnsiQuotes: procedure.AnsiQuotes}) if err != nil { return nil, err } @@ -312,7 +314,8 @@ func applyProcedures(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scop } return nil, transform.SameTree, err } - parsedProcedure, err := parse.Parse(ctx, procedure.CreateStatement) + parsedProcedure, err := parse.ParseWithOptions(ctx, procedure.CreateStatement, + sqlparser.ParserOptions{AnsiQuotes: procedure.AnsiQuotes}) if err != nil { return nil, transform.SameTree, err } diff --git a/sql/information_schema/routines_table.go b/sql/information_schema/routines_table.go index fac423d76c..cdbf72474b 100644 --- a/sql/information_schema/routines_table.go +++ b/sql/information_schema/routines_table.go @@ -18,26 +18,25 @@ import ( "bytes" "fmt" - "github.com/dolthub/go-mysql-server/sql/mysql_db" - . "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/mysql_db" "github.com/dolthub/go-mysql-server/sql/parse" "github.com/dolthub/go-mysql-server/sql/plan" "github.com/dolthub/go-mysql-server/sql/types" + "github.com/dolthub/vitess/go/vt/sqlparser" ) type routineTable struct { name string schema Schema catalog Catalog - procedures map[string][]*plan.Procedure + procedures map[string][]StoredProcedureDetails + // functions - rowIter func(*Context, Catalog, map[string][]*plan.Procedure) (RowIter, error) + rowIter func(*Context, Catalog, map[string][]StoredProcedureDetails) (RowIter, error) } -var ( - _ Table = (*routineTable)(nil) -) +var _ Table = (*routineTable)(nil) var doltProcedureAliasSet = map[string]interface{}{ "dadd": nil, @@ -65,10 +64,8 @@ func (t *routineTable) AssignCatalog(cat Catalog) Table { return t } -func (r *routineTable) AssignProcedures(p map[string][]*plan.Procedure) Table { - // TODO: should also assign functions +func (r *routineTable) AssignProcedures(p map[string][]StoredProcedureDetails) { r.procedures = p - return r } // Name implements the sql.Table interface. @@ -109,7 +106,7 @@ func (r *routineTable) PartitionRows(context *Context, partition Partition) (Row } // routinesRowIter implements the sql.RowIter for the information_schema.ROUTINES table. -func routinesRowIter(ctx *Context, c Catalog, p map[string][]*plan.Procedure) (RowIter, error) { +func routinesRowIter(ctx *Context, c Catalog, p map[string][]StoredProcedureDetails) (RowIter, error) { var rows []Row var ( securityType string @@ -126,6 +123,8 @@ func routinesRowIter(ctx *Context, c Catalog, p map[string][]*plan.Procedure) (R return nil, err } + // TODO: information_schema.routines should show the SQL_MODE used when the routine was defined, + // not the current session's SQL_MODE sysVal, err := ctx.Session.GetSessionVariable(ctx, "sql_mode") if err != nil { return nil, err @@ -153,30 +152,43 @@ func routinesRowIter(ctx *Context, c Catalog, p map[string][]*plan.Procedure) (R } dbCollation := plan.GetDatabaseCollation(ctx, db) for _, procedure := range procedures { + parsedNode, err := parse.ParseWithOptions(ctx, procedure.CreateStatement, + sqlparser.ParserOptions{AnsiQuotes: procedure.AnsiQuotes}) + if err != nil { + return nil, err + } + + parsedCreateProcedure, ok := parsedNode.(*plan.CreateProcedure) + if !ok { + // TODO: fix me + panic("uhoh!") + } + // Skip dolt procedure aliases to show in this table if _, isAlias := doltProcedureAliasSet[procedure.Name]; isAlias { continue } // We skip external procedures if the variable to show them is set to false - if showExternalProcedures.(int8) == 0 && procedure.IsExternal() { + if showExternalProcedures.(int8) == 0 && parsedCreateProcedure.IsExternal() { continue } - parsedProcedure, err := parse.Parse(ctx, procedure.CreateProcedureString) + parsedProcedure, err := parse.ParseWithOptions(ctx, parsedCreateProcedure.CreateProcedureString, + sqlparser.ParserOptions{AnsiQuotes: procedure.AnsiQuotes}) if err != nil { return nil, err } procedurePlan, ok := parsedProcedure.(*plan.CreateProcedure) if !ok { - return nil, ErrProcedureCreateStatementInvalid.New(procedure.CreateProcedureString) + return nil, ErrProcedureCreateStatementInvalid.New(parsedCreateProcedure.CreateProcedureString) } routineDef := procedurePlan.BodyString - definer := removeBackticks(procedure.Definer) + definer := removeBackticks(parsedCreateProcedure.Definer) securityType = "DEFINER" isDeterministic = "NO" // YES or NO sqlDataAccess = "CONTAINS SQL" - for _, ch := range procedure.Characteristics { + for _, ch := range parsedCreateProcedure.Characteristics { if ch == plan.Characteristic_LanguageSql { } @@ -198,41 +210,41 @@ func routinesRowIter(ctx *Context, c Catalog, p map[string][]*plan.Procedure) (R } } - if procedure.SecurityContext == plan.ProcedureSecurityContext_Invoker { + if parsedCreateProcedure.SecurityContext == plan.ProcedureSecurityContext_Invoker { securityType = "INVOKER" } rows = append(rows, Row{ - procedure.Name, // specific_name NOT NULL - "def", // routine_catalog - dbName, // routine_schema - procedure.Name, // routine_name NOT NULL - "PROCEDURE", // routine_type NOT NULL - "", // data_type - nil, // character_maximum_length - nil, // character_octet_length - nil, // numeric_precision - nil, // numeric_scale - nil, // datetime_precision - nil, // character_set_name - nil, // collation_name - nil, // dtd_identifier - "SQL", // routine_body NOT NULL - routineDef, // routine_definition - nil, // external_name - "SQL", // external_language NOT NULL - "SQL", // parameter_style NOT NULL - isDeterministic, // is_deterministic NOT NULL - sqlDataAccess, // sql_data_access NOT NULL - nil, // sql_path - securityType, // security_type NOT NULL - procedure.CreatedAt.UTC(), // created NOT NULL - procedure.ModifiedAt.UTC(), // last_altered NOT NULL - sqlMode, // sql_mode NOT NULL - procedure.Comment, // routine_comment NOT NULL - definer, // definer NOT NULL - characterSetClient, // character_set_client NOT NULL - collationConnection, // collation_connection NOT NULL - dbCollation.String(), // database_collation NOT NULL + procedure.Name, // specific_name NOT NULL + "def", // routine_catalog + dbName, // routine_schema + procedure.Name, // routine_name NOT NULL + "PROCEDURE", // routine_type NOT NULL + "", // data_type + nil, // character_maximum_length + nil, // character_octet_length + nil, // numeric_precision + nil, // numeric_scale + nil, // datetime_precision + nil, // character_set_name + nil, // collation_name + nil, // dtd_identifier + "SQL", // routine_body NOT NULL + routineDef, // routine_definition + nil, // external_name + "SQL", // external_language NOT NULL + "SQL", // parameter_style NOT NULL + isDeterministic, // is_deterministic NOT NULL + sqlDataAccess, // sql_data_access NOT NULL + nil, // sql_path + securityType, // security_type NOT NULL + procedure.CreatedAt.UTC(), // created NOT NULL + procedure.ModifiedAt.UTC(), // last_altered NOT NULL + sqlMode, // sql_mode NOT NULL + parsedCreateProcedure.Comment, // routine_comment NOT NULL + definer, // definer NOT NULL + characterSetClient, // character_set_client NOT NULL + collationConnection, // collation_connection NOT NULL + dbCollation.String(), // database_collation NOT NULL }) } } @@ -243,7 +255,7 @@ func routinesRowIter(ctx *Context, c Catalog, p map[string][]*plan.Procedure) (R } // parametersRowIter implements the sql.RowIter for the information_schema.PARAMETERS table. -func parametersRowIter(ctx *Context, c Catalog, p map[string][]*plan.Procedure) (RowIter, error) { +func parametersRowIter(ctx *Context, c Catalog, p map[string][]StoredProcedureDetails) (RowIter, error) { var rows []Row showExternalProcedures, err := ctx.GetSessionVariable(ctx, "show_external_procedures") @@ -259,16 +271,28 @@ func parametersRowIter(ctx *Context, c Catalog, p map[string][]*plan.Procedure) continue } for _, procedure := range procedures { + parsedNode, err := parse.ParseWithOptions(ctx, procedure.CreateStatement, + sqlparser.ParserOptions{AnsiQuotes: procedure.AnsiQuotes}) + if err != nil { + return nil, err + } + + parsedCreateProcedure, ok := parsedNode.(*plan.CreateProcedure) + if !ok { + // TODO: Fix me! + panic("uh oh2!") + } + // Skip dolt procedure aliases to show in this table if _, isAlias := doltProcedureAliasSet[procedure.Name]; isAlias { continue } // We skip external procedures if the variable to show them is set to false - if showExternalProcedures.(int8) == 0 && procedure.IsExternal() { + if showExternalProcedures.(int8) == 0 && parsedCreateProcedure.IsExternal() { continue } - for i, param := range procedure.Params { + for i, param := range parsedCreateProcedure.Params { var ( ordinalPos = uint64(i + 1) datetimePrecision interface{} diff --git a/sql/procedures.go b/sql/procedures.go index 486eec9956..d5ba432369 100644 --- a/sql/procedures.go +++ b/sql/procedures.go @@ -26,6 +26,7 @@ type StoredProcedureDetails struct { CreateStatement string // The CREATE statement for this stored procedure. CreatedAt time.Time // The time that the stored procedure was created. ModifiedAt time.Time // The time of the last modification to the stored procedure. + AnsiQuotes bool // True if ANSI_QUOTES sql mode was used when defining this stored procedure. } // ExternalStoredProcedureDetails are the details of an external stored procedure. Compared to standard stored diff --git a/sql/rowexec/ddl.go b/sql/rowexec/ddl.go index 76915d6db9..f2f5e4c277 100644 --- a/sql/rowexec/ddl.go +++ b/sql/rowexec/ddl.go @@ -913,12 +913,17 @@ func (b *BaseBuilder) buildCreateTable(ctx *sql.Context, n *plan.CreateTable, ro } func (b *BaseBuilder) buildCreateProcedure(ctx *sql.Context, n *plan.CreateProcedure, row sql.Row) (sql.RowIter, error) { + sqlMode, err := sql.LoadSqlMode(ctx) + if err != nil { + return nil, err + } return &createProcedureIter{ spd: sql.StoredProcedureDetails{ Name: n.Name, CreateStatement: n.CreateProcedureString, CreatedAt: n.CreatedAt, ModifiedAt: n.ModifiedAt, + AnsiQuotes: sqlMode.AnsiQuotes(), }, db: n.Database(), }, nil From 6745a476d10258b7eb9041a3bc9f14944eaee210 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 1 Aug 2023 09:34:55 -0700 Subject: [PATCH 08/18] Small fix for test query syntax --- enginetest/queries/variable_queries.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/enginetest/queries/variable_queries.go b/enginetest/queries/variable_queries.go index 54d4fbd46f..dcc1f889f2 100644 --- a/enginetest/queries/variable_queries.go +++ b/enginetest/queries/variable_queries.go @@ -155,8 +155,8 @@ var VariableQueries = []ScriptTest{ { Name: "set system variable with expressions", SetUpScript: []string{ - `set lc_messages = "123", @@auto_increment_increment = 1`, - `set lc_messages = concat(@@lc_messages, "456"), @@auto_increment_increment = @@auto_increment_increment + 3`, + `set lc_messages = '123', @@auto_increment_increment = 1`, + `set lc_messages = concat(@@lc_messages, '456'), @@auto_increment_increment = @@auto_increment_increment + 3`, }, Query: "SELECT @@lc_messages, @@auto_increment_increment", Expected: []sql.Row{ From 7691c4a5e5421518bab1bcf2fbfef1ea666c8b2e Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 1 Aug 2023 09:42:32 -0700 Subject: [PATCH 09/18] Initial planbuilder support for parsing options --- sql/planbuilder/parse.go | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/sql/planbuilder/parse.go b/sql/planbuilder/parse.go index 9a530db647..001259eeab 100644 --- a/sql/planbuilder/parse.go +++ b/sql/planbuilder/parse.go @@ -13,8 +13,16 @@ import ( "github.com/dolthub/go-mysql-server/sql/plan" ) -// Parse parses the given SQL sentence and returns the corresponding node. +// Parse parses the given SQL |query| using the default parsing settings and returns the corresponding node. func Parse(ctx *sql.Context, cat sql.Catalog, query string) (ret sql.Node, err error) { + sqlMode, err := sql.LoadSqlMode(ctx) + if err != nil { + return nil, err + } + return ParseWithOptions(ctx, cat, query, sqlMode.ParserOptions()) +} + +func ParseWithOptions(ctx *sql.Context, cat sql.Catalog, query string, options sqlparser.ParserOptions) (ret sql.Node, err error) { defer func() { if r := recover(); r != nil { switch r := r.(type) { @@ -25,15 +33,20 @@ func Parse(ctx *sql.Context, cat sql.Catalog, query string) (ret sql.Node, err e } } }() - n, _, _, err := parse(ctx, cat, query, false) + n, _, _, err := parse(ctx, cat, query, false, options) return n, err } func ParseOne(ctx *sql.Context, cat sql.Catalog, query string) (sql.Node, string, string, error) { - return parse(ctx, cat, query, true) + sqlMode, err := sql.LoadSqlMode(ctx) + if err != nil { + return nil, "", "", err + } + + return parse(ctx, cat, query, true, sqlMode.ParserOptions()) } -func parse(ctx *sql.Context, cat sql.Catalog, query string, multi bool) (sql.Node, string, string, error) { +func parse(ctx *sql.Context, cat sql.Catalog, query string, multi bool, options sqlparser.ParserOptions) (sql.Node, string, string, error) { span, ctx := ctx.Span("parse", trace.WithAttributes(attribute.String("query", query))) defer span.End() @@ -50,10 +63,10 @@ func parse(ctx *sql.Context, cat sql.Catalog, query string, multi bool) (sql.Nod parsed = s if !multi { - stmt, err = sqlparser.Parse(s) + stmt, err = sqlparser.ParseWithOptions(s, options) } else { var ri int - stmt, ri, err = sqlparser.ParseOne(s) + stmt, ri, err = sqlparser.ParseOneWithOptions(s, options) if ri != 0 && ri < len(s) { parsed = s[:ri] parsed = strings.TrimSpace(parsed) From e99db28f6bbb8eb5a1411da0ea4423710e0d17fd Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 1 Aug 2023 10:03:50 -0700 Subject: [PATCH 10/18] Tidying up tests --- enginetest/queries/ansi_quotes_queries.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/enginetest/queries/ansi_quotes_queries.go b/enginetest/queries/ansi_quotes_queries.go index 255c484968..080d549465 100644 --- a/enginetest/queries/ansi_quotes_queries.go +++ b/enginetest/queries/ansi_quotes_queries.go @@ -66,11 +66,9 @@ var AnsiQuotesTests = []ScriptTest{ ExpectedErrStr: `column "\"\"foo\"\"" could not be found in any table in scope`, }, { - // TODO: Double check this behavior with MySQL Query: "select ```foo```;", ExpectedErrStr: "column \"`foo`\" could not be found in any table in scope", }, - // --- Assertions AFTER turning ANSI_QUOTES off --- { // Disable ANSI_QUOTES and make sure we can still run queries Query: `SET @@sql_mode='NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, @@ -226,6 +224,8 @@ var AnsiQuotesTests = []ScriptTest{ }, { // Assert that we can read and parse the procedure definition from information_schema + // TODO: This one doesn't work yet, until we fix information_schema ROUTINES table support for parsing ANSI_QUOTES + Skip: true, Query: `select routine_definition from information_schema.routines where routine_name='AnsiProcedure';`, Expected: []sql.Row{{`BEGIN SELECT "name" from "t" where "pk" = 1; END`}}, }, @@ -253,7 +253,6 @@ var AnsiQuotesTests = []ScriptTest{ }, { // Insert a row with ANSI_QUOTES mode disabled - // TODO: Using DEFAULT in the values clause doesn't seem to work? Query: `insert into t (pk, name) values (2, 'Jill');`, Expected: []sql.Row{{types.NewOkResult(1)}}, }, From 807886a6d1375ff3c4a00a6be4b52e404b94b1b5 Mon Sep 17 00:00:00 2001 From: fulghum Date: Tue, 1 Aug 2023 17:44:46 +0000 Subject: [PATCH 11/18] [ga-format-pr] Run ./format_repo.sh to fix formatting --- sql/analyzer/stored_procedures.go | 6 +++--- sql/analyzer/triggers.go | 2 +- sql/sql_mode.go | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/sql/analyzer/stored_procedures.go b/sql/analyzer/stored_procedures.go index 1f2327cb66..9cd1041730 100644 --- a/sql/analyzer/stored_procedures.go +++ b/sql/analyzer/stored_procedures.go @@ -18,6 +18,7 @@ import ( "fmt" "strings" + "github.com/dolthub/vitess/go/vt/sqlparser" "gopkg.in/src-d/go-errors.v1" "github.com/dolthub/go-mysql-server/sql" @@ -26,7 +27,6 @@ import ( "github.com/dolthub/go-mysql-server/sql/plan" "github.com/dolthub/go-mysql-server/sql/planbuilder" "github.com/dolthub/go-mysql-server/sql/transform" - "github.com/dolthub/vitess/go/vt/sqlparser" ) // loadStoredProcedures loads non-built-in stored procedures for all databases on relevant calls. @@ -61,7 +61,7 @@ func loadStoredProcedures(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan b.ProcCtx().Active = false } else { parsedProcedure, err = parse.ParseWithOptions(ctx, procedure.CreateStatement, - sqlparser.ParserOptions{AnsiQuotes: procedure.AnsiQuotes}) + sqlparser.ParserOptions{AnsiQuotes: procedure.AnsiQuotes}) } if err != nil { return nil, err @@ -338,7 +338,7 @@ func applyProcedures(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scop b.ProcCtx().Active = false } else { parsedProcedure, err = parse.ParseWithOptions(ctx, procedure.CreateStatement, - sqlparser.ParserOptions{AnsiQuotes: procedure.AnsiQuotes}) + sqlparser.ParserOptions{AnsiQuotes: procedure.AnsiQuotes}) } if err != nil { return nil, transform.SameTree, err diff --git a/sql/analyzer/triggers.go b/sql/analyzer/triggers.go index 04d37ef004..9ef2604480 100644 --- a/sql/analyzer/triggers.go +++ b/sql/analyzer/triggers.go @@ -193,7 +193,7 @@ func applyTriggers(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scope, b.TriggerCtx().Call = false } else { parsedTrigger, err = parse.ParseWithOptions(ctx, trigger.CreateStatement, - sqlparser.ParserOptions{AnsiQuotes: trigger.AnsiQuotes}) + sqlparser.ParserOptions{AnsiQuotes: trigger.AnsiQuotes}) } if err != nil { return nil, transform.SameTree, err diff --git a/sql/sql_mode.go b/sql/sql_mode.go index 2ffcf99eac..43d99460c8 100644 --- a/sql/sql_mode.go +++ b/sql/sql_mode.go @@ -16,8 +16,9 @@ package sql import ( "fmt" - "github.com/dolthub/vitess/go/vt/sqlparser" "strings" + + "github.com/dolthub/vitess/go/vt/sqlparser" ) // SqlMode encodes the SQL mode string and provides methods for querying the enabled modes. From 77017aac3c9e632cfc114a84f2f4552560e8f78c Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 1 Aug 2023 11:31:58 -0700 Subject: [PATCH 12/18] Initializing system variables for parser test --- sql/parse/parse_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sql/parse/parse_test.go b/sql/parse/parse_test.go index da0abbb93c..fa4e4973f5 100644 --- a/sql/parse/parse_test.go +++ b/sql/parse/parse_test.go @@ -34,6 +34,7 @@ import ( "github.com/dolthub/go-mysql-server/sql/expression/function/aggregation" "github.com/dolthub/go-mysql-server/sql/plan" "github.com/dolthub/go-mysql-server/sql/types" + "github.com/dolthub/go-mysql-server/sql/variables" ) var showCollationProjection = plan.NewProject([]sql.Expression{ @@ -5040,6 +5041,7 @@ END`, for _, tt := range fixtures { t.Run(tt.input, func(t *testing.T) { + variables.InitSystemVariables() require := require.New(t) ctx := sql.NewEmptyContext() p, err := Parse(ctx, tt.input) From 5b17ec67c5a828b5d5980299ef1868e30adf7215 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Mon, 7 Aug 2023 11:29:36 -0700 Subject: [PATCH 13/18] Tidying up --- enginetest/queries/ansi_quotes_queries.go | 6 +-- server/handler.go | 3 +- sql/analyzer/check_constraints.go | 1 - sql/rowexec/ddl.go | 1 - sql/sql_mode.go | 16 ++++++-- sql/sql_mode_test.go | 45 +++++++++++++++++++++++ 6 files changed, 59 insertions(+), 13 deletions(-) create mode 100644 sql/sql_mode_test.go diff --git a/enginetest/queries/ansi_quotes_queries.go b/enginetest/queries/ansi_quotes_queries.go index 080d549465..6271f761e8 100644 --- a/enginetest/queries/ansi_quotes_queries.go +++ b/enginetest/queries/ansi_quotes_queries.go @@ -20,7 +20,6 @@ import ( "github.com/dolthub/go-mysql-server/sql/types" ) -// TODO: Audit all places that use the vitess and GMS parse functions var AnsiQuotesTests = []ScriptTest{ { Name: "ANSI_QUOTES: basic cases", @@ -232,8 +231,6 @@ var AnsiQuotesTests = []ScriptTest{ }, }, { - // TODO: How about when we merge? We apply check constraints and column default there, too - // (and they both use the new planbuilder package) Name: "ANSI_QUOTES: column defaults", SetUpScript: []string{ `SET @@sql_mode='ANSI_QUOTES,NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, @@ -267,7 +264,7 @@ var AnsiQuotesTests = []ScriptTest{ Name: "ANSI_QUOTES: check constraints", SetUpScript: []string{ `SET @@sql_mode='ANSI_QUOTES,NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES';`, - `create table t (pk int primary key, data varchar(100), CONSTRAINT ansi_check CHECK ("data" != 'forbidden'));`, + `create table t (pk int primary key, data varchar(100), CONSTRAINT "ansi_check" CHECK ("data" != 'forbidden'));`, }, Assertions: []ScriptTestAssertion{ { @@ -296,7 +293,6 @@ var AnsiQuotesTests = []ScriptTest{ }, Assertions: []ScriptTestAssertion{ { - // Assert the check constraint runs correctly in ANSI_QUOTES mode Query: `CREATE EVENT myevent ON SCHEDULE EVERY 1 SECOND STARTS '2037-10-16 23:59:00' DO UPDATE "t" SET "count"="count"+1;`, diff --git a/server/handler.go b/server/handler.go index f700217719..059ac1d365 100644 --- a/server/handler.go +++ b/server/handler.go @@ -118,8 +118,7 @@ func (h *Handler) ComPrepare(c *mysql.Conn, query string) ([]*query.Field, error analyzed, err = h.e.PrepareQuery(ctx, query) } if err != nil { - // TODO: Look at error logging in other handler functions - logrus.StandardLogger().Errorf("ComPrepare ERROR: %s", err.Error()) + logrus.WithField("query", query).Errorf("unable to prepare query: %s", err.Error()) err := sql.CastSQLError(err) return nil, err } diff --git a/sql/analyzer/check_constraints.go b/sql/analyzer/check_constraints.go index 5c3e7bbe2d..35e4cc6ee7 100644 --- a/sql/analyzer/check_constraints.go +++ b/sql/analyzer/check_constraints.go @@ -303,7 +303,6 @@ func loadChecksFromTable(ctx *sql.Context, table sql.Table) ([]*sql.CheckConstra func ConvertCheckDefToConstraint(ctx *sql.Context, check *sql.CheckDefinition, t sql.Table) (*sql.CheckConstraint, error) { parseStr := fmt.Sprintf("select %s", check.CheckExpression) - // TODO: Move to ParseWithOptions and test ANSI_QUOTES support with check constraints parsed, err := sqlparser.Parse(parseStr) if err != nil { return nil, err diff --git a/sql/rowexec/ddl.go b/sql/rowexec/ddl.go index 0b3ee73569..f6db2f1b3c 100644 --- a/sql/rowexec/ddl.go +++ b/sql/rowexec/ddl.go @@ -941,7 +941,6 @@ func (b *BaseBuilder) buildCreateTrigger(ctx *sql.Context, n *plan.CreateTrigger if err != nil { return nil, err } - return &createTriggerIter{ definition: sql.TriggerDefinition{ Name: n.TriggerName, diff --git a/sql/sql_mode.go b/sql/sql_mode.go index 43d99460c8..aec4bd8e65 100644 --- a/sql/sql_mode.go +++ b/sql/sql_mode.go @@ -39,6 +39,12 @@ func LoadSqlMode(ctx *Context) (*SqlMode, error) { return nil, fmt.Errorf("unable to parse sql_mode: %v", sqlMode) } + return NewSqlModeFromString(sqlModeString), nil +} + +// NewSqlModeFromString returns a new SqlMode instance, constructed from the specified |sqlModeString| that +// has a comma delimited list of SQL modes (e.g. "ONLY_FULLY_GROUP_BY,ANSI_QUOTES"). +func NewSqlModeFromString(sqlModeString string) *SqlMode { sqlModeString = strings.ToLower(sqlModeString) elements := strings.Split(sqlModeString, ",") modes := map[string]struct{}{} @@ -46,9 +52,7 @@ func LoadSqlMode(ctx *Context) (*SqlMode, error) { modes[element] = struct{}{} } - return &SqlMode{ - modes: modes, - }, nil + return &SqlMode{modes: modes} } // AnsiQuotes returns true if the ANSI_QUOTES SQL mode is enabled. Note that the ANSI mode is a compound mode that @@ -57,7 +61,11 @@ func (s *SqlMode) AnsiQuotes() bool { return s.ModeEnabled("ansi_quotes") || s.ModeEnabled("ansi") } -// ModeEnabled returns true if |mode| is enabled in the SQL mode string. +// ModeEnabled returns true if |mode| was explicitly specified in the SQL_MODE string that was used to +// create this SqlMode instance. Note this function does not support expanding compound modes into the +// individual modes they contain (e.g. if "ANSI" is the SQL_MODE string, then this function will not +// report that "ANSI_QUOTES" is enabled). To deal with compound modes, use the mode specific functions, +// such as SqlMode::AnsiQuotes(). func (s *SqlMode) ModeEnabled(mode string) bool { _, ok := s.modes[strings.ToLower(mode)] return ok diff --git a/sql/sql_mode_test.go b/sql/sql_mode_test.go new file mode 100644 index 0000000000..f98b9b2dfe --- /dev/null +++ b/sql/sql_mode_test.go @@ -0,0 +1,45 @@ +// Copyright 2023 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package sql + +import ( + "github.com/stretchr/testify/require" + "testing" +) + +func TestSqlMode(t *testing.T) { + // Test that ANSI mode includes ANSI_QUOTES mode + sqlMode := NewSqlModeFromString("only_full_group_by,ansi") + require.True(t, sqlMode.AnsiQuotes()) + require.True(t, sqlMode.ModeEnabled("ansi")) + require.True(t, sqlMode.ModeEnabled("ANSI")) + require.True(t, sqlMode.ModeEnabled("ONLY_FULL_GROUP_BY")) + require.False(t, sqlMode.ModeEnabled("fake_mode")) + require.True(t, sqlMode.ParserOptions().AnsiQuotes) + + sqlMode = NewSqlModeFromString("AnSi_quotEs") + require.True(t, sqlMode.AnsiQuotes()) + require.True(t, sqlMode.ModeEnabled("ansi_quotes")) + require.True(t, sqlMode.ModeEnabled("ANSI_quoTes")) + require.False(t, sqlMode.ModeEnabled("fake_mode")) + require.True(t, sqlMode.ParserOptions().AnsiQuotes) + + // Test when SQL_MODE does not include ANSI_QUOTES + sqlMode = NewSqlModeFromString("ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES") + require.False(t, sqlMode.AnsiQuotes()) + require.True(t, sqlMode.ModeEnabled("STRICT_TRANS_TABLES")) + require.False(t, sqlMode.ModeEnabled("ansi_quotes")) + require.False(t, sqlMode.ParserOptions().AnsiQuotes) +} From 3264b64fc5fc7c7642398c332e2661d9bebe3465 Mon Sep 17 00:00:00 2001 From: fulghum Date: Mon, 7 Aug 2023 19:18:11 +0000 Subject: [PATCH 14/18] [ga-format-pr] Run ./format_repo.sh to fix formatting --- sql/sql_mode_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sql/sql_mode_test.go b/sql/sql_mode_test.go index f98b9b2dfe..4597d67345 100644 --- a/sql/sql_mode_test.go +++ b/sql/sql_mode_test.go @@ -15,8 +15,9 @@ package sql import ( - "github.com/stretchr/testify/require" "testing" + + "github.com/stretchr/testify/require" ) func TestSqlMode(t *testing.T) { From 711015d6258ad13e0aabbe3bc50e2ac575ac91af Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Mon, 7 Aug 2023 13:09:43 -0700 Subject: [PATCH 15/18] Switching to track SQL_MODE, instead of just ANSI_QUOTES --- memory/database.go | 2 +- sql/analyzer/load_events.go | 4 +--- sql/analyzer/load_triggers.go | 7 +++---- sql/analyzer/resolve_views.go | 4 +--- sql/analyzer/stored_procedures.go | 5 ++--- sql/analyzer/triggers.go | 4 ++-- sql/databases.go | 8 ++++---- sql/events.go | 4 ++-- sql/information_schema/information_schema.go | 8 +++++--- sql/plan/ddl_event.go | 2 +- sql/procedures.go | 2 +- sql/rowexec/ddl.go | 4 ++-- sql/sql_mode.go | 13 +++++++++++-- sql/sql_mode_test.go | 4 ++++ 14 files changed, 40 insertions(+), 31 deletions(-) diff --git a/memory/database.go b/memory/database.go index b421a7c43f..f6f6b089b3 100644 --- a/memory/database.go +++ b/memory/database.go @@ -441,7 +441,7 @@ func (d *Database) CreateView(ctx *sql.Context, name string, selectStatement, cr Name: name, TextDefinition: selectStatement, CreateViewStatement: createViewStmt, - AnsiQuotes: sqlMode.AnsiQuotes(), + SqlMode: sqlMode.String(), } return nil } diff --git a/sql/analyzer/load_events.go b/sql/analyzer/load_events.go index 812f5bdbe7..e92c63ff51 100644 --- a/sql/analyzer/load_events.go +++ b/sql/analyzer/load_events.go @@ -15,8 +15,6 @@ package analyzer import ( - "github.com/dolthub/vitess/go/vt/sqlparser" - "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/parse" "github.com/dolthub/go-mysql-server/sql/plan" @@ -88,7 +86,7 @@ func loadEventFromDb(ctx *sql.Context, db sql.Database, name string) (sql.EventD func getEventDetailsFromEventDefinition(ctx *sql.Context, event sql.EventDefinition) (sql.EventDetails, error) { parsedCreateEvent, err := parse.ParseWithOptions(ctx, event.CreateStatement, - sqlparser.ParserOptions{AnsiQuotes: event.AnsiQuotes}) + sql.NewSqlModeFromString(event.SqlMode).ParserOptions()) if err != nil { return sql.EventDetails{}, err } diff --git a/sql/analyzer/load_triggers.go b/sql/analyzer/load_triggers.go index 6b5513db67..e6bdeb80b4 100644 --- a/sql/analyzer/load_triggers.go +++ b/sql/analyzer/load_triggers.go @@ -17,8 +17,6 @@ package analyzer import ( "strings" - "github.com/dolthub/vitess/go/vt/sqlparser" - "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/parse" "github.com/dolthub/go-mysql-server/sql/plan" @@ -107,10 +105,11 @@ func loadTriggersFromDb(ctx *sql.Context, a *Analyzer, db sql.Database) ([]*plan } for _, trigger := range triggers { var parsedTrigger sql.Node + sqlMode := sql.NewSqlModeFromString(trigger.SqlMode) if ctx.Version == sql.VersionExperimental { - parsedTrigger, err = planbuilder.ParseWithOptions(ctx, a.Catalog, trigger.CreateStatement, sqlparser.ParserOptions{AnsiQuotes: trigger.AnsiQuotes}) + parsedTrigger, err = planbuilder.ParseWithOptions(ctx, a.Catalog, trigger.CreateStatement, sqlMode.ParserOptions()) } else { - parsedTrigger, err = parse.ParseWithOptions(ctx, trigger.CreateStatement, sqlparser.ParserOptions{AnsiQuotes: trigger.AnsiQuotes}) + parsedTrigger, err = parse.ParseWithOptions(ctx, trigger.CreateStatement, sqlMode.ParserOptions()) } if err != nil { return nil, err diff --git a/sql/analyzer/resolve_views.go b/sql/analyzer/resolve_views.go index 8d99dc269c..2f898d4b36 100644 --- a/sql/analyzer/resolve_views.go +++ b/sql/analyzer/resolve_views.go @@ -17,8 +17,6 @@ package analyzer import ( "fmt" - "github.com/dolthub/vitess/go/vt/sqlparser" - "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/parse" "github.com/dolthub/go-mysql-server/sql/plan" @@ -59,7 +57,7 @@ func resolveViews(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scope, } if vdok { query, qerr := parse.ParseWithOptions(ctx, viewDef.TextDefinition, - sqlparser.ParserOptions{AnsiQuotes: viewDef.AnsiQuotes}) + sql.NewSqlModeFromString(viewDef.SqlMode).ParserOptions()) if qerr != nil { return nil, transform.SameTree, qerr } diff --git a/sql/analyzer/stored_procedures.go b/sql/analyzer/stored_procedures.go index 5e0d71dcfa..53698e5a55 100644 --- a/sql/analyzer/stored_procedures.go +++ b/sql/analyzer/stored_procedures.go @@ -18,7 +18,6 @@ import ( "fmt" "strings" - "github.com/dolthub/vitess/go/vt/sqlparser" "gopkg.in/src-d/go-errors.v1" "github.com/dolthub/go-mysql-server/sql" @@ -70,7 +69,7 @@ func loadStoredProcedures(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan } } else { parsedProcedure, err = parse.ParseWithOptions(ctx, procedure.CreateStatement, - sqlparser.ParserOptions{AnsiQuotes: procedure.AnsiQuotes}) + sql.NewSqlModeFromString(procedure.SqlMode).ParserOptions()) if err != nil { return nil, err } @@ -344,7 +343,7 @@ func applyProcedures(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scop parsedProcedure, _, _, err = b.Parse(procedure.CreateStatement, false) } else { parsedProcedure, err = parse.ParseWithOptions(ctx, procedure.CreateStatement, - sqlparser.ParserOptions{AnsiQuotes: procedure.AnsiQuotes}) + sql.NewSqlModeFromString(procedure.SqlMode).ParserOptions()) } if err != nil { return nil, transform.SameTree, err diff --git a/sql/analyzer/triggers.go b/sql/analyzer/triggers.go index 9ef2604480..cbf4ba7533 100644 --- a/sql/analyzer/triggers.go +++ b/sql/analyzer/triggers.go @@ -192,8 +192,8 @@ func applyTriggers(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scope, parsedTrigger, _, _, err = b.Parse(trigger.CreateStatement, false) b.TriggerCtx().Call = false } else { - parsedTrigger, err = parse.ParseWithOptions(ctx, trigger.CreateStatement, - sqlparser.ParserOptions{AnsiQuotes: trigger.AnsiQuotes}) + sqlMode := sql.NewSqlModeFromString(trigger.SqlMode) + parsedTrigger, err = parse.ParseWithOptions(ctx, trigger.CreateStatement, sqlMode.ParserOptions()) } if err != nil { return nil, transform.SameTree, err diff --git a/sql/databases.go b/sql/databases.go index 41f9f0a23a..f305c435e5 100644 --- a/sql/databases.go +++ b/sql/databases.go @@ -189,9 +189,9 @@ type TriggerDefinition struct { CreateStatement string // The time that the trigger was created. CreatedAt time.Time - // AnsiQuotes is true if this trigger definition was defined using the ANSI_QUOTES SQL mode, meaning that any - // double quotes should be treated as identifier quotes, instead of string literal quotes. - AnsiQuotes bool + // SqlMode holds the SQL_MODE that was in use when this trigger was originally defined. It contains information + // needed for how to parse the trigger's SQL, such as whether ANSI_QUOTES mode is enabled. + SqlMode string } // TemporaryTableDatabase is a database that can query the session (which manages the temporary table state) to @@ -266,7 +266,7 @@ type ViewDefinition struct { Name string TextDefinition string CreateViewStatement string - AnsiQuotes bool + SqlMode string } // GetTableInsensitive implements a case-insensitive map lookup for tables keyed off of the table name. diff --git a/sql/events.go b/sql/events.go index 47b577636b..851ff15d1a 100644 --- a/sql/events.go +++ b/sql/events.go @@ -32,8 +32,8 @@ type EventDefinition struct { CreatedAt time.Time // The time that the event was last altered. LastAltered time.Time - // True if this event definition relies on ANSI_QUOTES mode for parsing its SQL - AnsiQuotes bool + // The SQL_MODE string in use when this event was defined. + SqlMode string } // EventDetails are the details of the event. diff --git a/sql/information_schema/information_schema.go b/sql/information_schema/information_schema.go index 096a86a0e0..99306728d4 100644 --- a/sql/information_schema/information_schema.go +++ b/sql/information_schema/information_schema.go @@ -1877,7 +1877,9 @@ func triggersRowIter(ctx *Context, c Catalog) (RowIter, error) { return nil, err } - // TODO: This should be the SQL_MODE at the time the trigger was created, not the current session's SQL_MODE + // TODO: This should be the SQL_MODE at the time the trigger was created, not the current session's SQL_MODE. + // We track that information now, but this code needs to be refactored a bit in order to display it, since + // trigger definitions are converted to trigger plan nodes and then used for generating output. sysVal, err := ctx.Session.GetSessionVariable(ctx, "sql_mode") if err != nil { return nil, err @@ -1909,7 +1911,7 @@ func triggersRowIter(ctx *Context, c Catalog) (RowIter, error) { var triggerPlans []*plan.CreateTrigger for _, trigger := range triggers { parsedTrigger, err := parse.ParseWithOptions(ctx, trigger.CreateStatement, - sqlparser.ParserOptions{AnsiQuotes: trigger.AnsiQuotes}) + NewSqlModeFromString(trigger.SqlMode).ParserOptions()) if err != nil { return nil, err } @@ -2115,7 +2117,7 @@ func viewsRowIter(ctx *Context, catalog Catalog) (RowIter, error) { continue } parsedView, err := parse.ParseWithOptions(ctx, view.CreateViewStatement, - sqlparser.ParserOptions{AnsiQuotes: view.AnsiQuotes}) + NewSqlModeFromString(view.SqlMode).ParserOptions()) if err != nil { return nil, err } diff --git a/sql/plan/ddl_event.go b/sql/plan/ddl_event.go index c9c8778b0b..5461af6b55 100644 --- a/sql/plan/ddl_event.go +++ b/sql/plan/ddl_event.go @@ -331,7 +331,7 @@ func (c *createEventIter) Next(ctx *sql.Context) (sql.Row, error) { CreateStatement: c.eventDetails.CreateEventStatement(), CreatedAt: c.eventDetails.Created, LastAltered: c.eventDetails.LastAltered, - AnsiQuotes: sqlMode.AnsiQuotes(), + SqlMode: sqlMode.String(), } err = c.eventDb.SaveEvent(ctx, eventDefinition) diff --git a/sql/procedures.go b/sql/procedures.go index d5ba432369..acbba18eb7 100644 --- a/sql/procedures.go +++ b/sql/procedures.go @@ -26,7 +26,7 @@ type StoredProcedureDetails struct { CreateStatement string // The CREATE statement for this stored procedure. CreatedAt time.Time // The time that the stored procedure was created. ModifiedAt time.Time // The time of the last modification to the stored procedure. - AnsiQuotes bool // True if ANSI_QUOTES sql mode was used when defining this stored procedure. + SqlMode string // The SQL_MODE when this procedure was defined. } // ExternalStoredProcedureDetails are the details of an external stored procedure. Compared to standard stored diff --git a/sql/rowexec/ddl.go b/sql/rowexec/ddl.go index f6db2f1b3c..aab8d36e2a 100644 --- a/sql/rowexec/ddl.go +++ b/sql/rowexec/ddl.go @@ -930,7 +930,7 @@ func (b *BaseBuilder) buildCreateProcedure(ctx *sql.Context, n *plan.CreateProce CreateStatement: n.CreateProcedureString, CreatedAt: n.CreatedAt, ModifiedAt: n.ModifiedAt, - AnsiQuotes: sqlMode.AnsiQuotes(), + SqlMode: sqlMode.String(), }, db: n.Database(), }, nil @@ -946,7 +946,7 @@ func (b *BaseBuilder) buildCreateTrigger(ctx *sql.Context, n *plan.CreateTrigger Name: n.TriggerName, CreateStatement: n.CreateTriggerString, CreatedAt: n.CreatedAt, - AnsiQuotes: sqlMode.AnsiQuotes(), + SqlMode: sqlMode.String(), }, db: n.Database(), }, nil diff --git a/sql/sql_mode.go b/sql/sql_mode.go index aec4bd8e65..20f10d5f9e 100644 --- a/sql/sql_mode.go +++ b/sql/sql_mode.go @@ -23,7 +23,8 @@ import ( // SqlMode encodes the SQL mode string and provides methods for querying the enabled modes. type SqlMode struct { - modes map[string]struct{} + modes map[string]struct{} + modeString string } // LoadSqlMode loads the SQL mode using the session data contained in |ctx| and returns a SqlMode @@ -52,7 +53,10 @@ func NewSqlModeFromString(sqlModeString string) *SqlMode { modes[element] = struct{}{} } - return &SqlMode{modes: modes} + return &SqlMode{ + modes: modes, + modeString: strings.ToUpper(sqlModeString), + } } // AnsiQuotes returns true if the ANSI_QUOTES SQL mode is enabled. Note that the ANSI mode is a compound mode that @@ -77,3 +81,8 @@ func (s *SqlMode) ParserOptions() sqlparser.ParserOptions { AnsiQuotes: s.AnsiQuotes(), } } + +// String returns the SQL_MODE string representing this SqlMode instance. +func (s *SqlMode) String() string { + return s.modeString +} diff --git a/sql/sql_mode_test.go b/sql/sql_mode_test.go index f98b9b2dfe..1161c43bc2 100644 --- a/sql/sql_mode_test.go +++ b/sql/sql_mode_test.go @@ -28,13 +28,16 @@ func TestSqlMode(t *testing.T) { require.True(t, sqlMode.ModeEnabled("ONLY_FULL_GROUP_BY")) require.False(t, sqlMode.ModeEnabled("fake_mode")) require.True(t, sqlMode.ParserOptions().AnsiQuotes) + require.Equal(t, "ONLY_FULL_GROUP_BY,ANSI", sqlMode.String()) + // Test a mixed case SQL_MODE string where only ANSI_QUOTES is enabled sqlMode = NewSqlModeFromString("AnSi_quotEs") require.True(t, sqlMode.AnsiQuotes()) require.True(t, sqlMode.ModeEnabled("ansi_quotes")) require.True(t, sqlMode.ModeEnabled("ANSI_quoTes")) require.False(t, sqlMode.ModeEnabled("fake_mode")) require.True(t, sqlMode.ParserOptions().AnsiQuotes) + require.Equal(t, "ANSI_QUOTES", sqlMode.String()) // Test when SQL_MODE does not include ANSI_QUOTES sqlMode = NewSqlModeFromString("ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES") @@ -42,4 +45,5 @@ func TestSqlMode(t *testing.T) { require.True(t, sqlMode.ModeEnabled("STRICT_TRANS_TABLES")) require.False(t, sqlMode.ModeEnabled("ansi_quotes")) require.False(t, sqlMode.ParserOptions().AnsiQuotes) + require.Equal(t, "ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES", sqlMode.String()) } From 96fdbf0a064410b061fe27bdb79bc826c99ec5c4 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Mon, 7 Aug 2023 14:13:54 -0700 Subject: [PATCH 16/18] Tidying up --- server/handler.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/handler.go b/server/handler.go index 059ac1d365..a36b42cf0e 100644 --- a/server/handler.go +++ b/server/handler.go @@ -69,7 +69,8 @@ const ( func init() { // Set the log.Error and log.Errorf functions in Vitess so that any errors // logged by Vitess will appear in our logs. Without this, errors from Vitess - // can be silently swallowed, which makes debugging failures harder. + // can be swallowed (e.g. any parse error for a ComPrepare event is silently + // swallowed without this wired up), which makes debugging failures harder. log.Error = logrus.StandardLogger().Error log.Errorf = logrus.StandardLogger().Errorf } From d037bf94cb86ac3691c12d9dbd26a5a7373903c7 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 8 Aug 2023 13:08:09 -0700 Subject: [PATCH 17/18] PR feedback --- enginetest/queries/ansi_quotes_queries.go | 16 +++++++++- sql/sql_mode_test.go | 38 +++++++++++------------ 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/enginetest/queries/ansi_quotes_queries.go b/enginetest/queries/ansi_quotes_queries.go index 6271f761e8..8fd6e107e5 100644 --- a/enginetest/queries/ansi_quotes_queries.go +++ b/enginetest/queries/ansi_quotes_queries.go @@ -84,7 +84,9 @@ var AnsiQuotesTests = []ScriptTest{ }, }, { - Name: "ANSI_QUOTES: ANSI mode includes ANSI_QUOTES", + // ANSI mode is a special "combination" mode that includes ANSI_QUOTES and other modes + // https://dev.mysql.com/doc/refman/8.0/en/sql-mode.html#sql-mode-combo + Name: "ANSI_QUOTES: ANSI combination mode", SetUpScript: []string{ `SET @@sql_mode='ANSI';`, }, @@ -115,6 +117,10 @@ var AnsiQuotesTests = []ScriptTest{ Query: `CREATE TABLE public_keys (item INTEGER, type CHAR(4), hash INTEGER, "count" INTEGER, "public" VARCHAR(8000))`, Expected: []sql.Row{{types.NewOkResult(0)}}, }, + { + Query: `insert into public_keys("item", "type", "hash", "count", "public") values (42, 'type', 1010, 1, 'public');`, + Expected: []sql.Row{{types.NewOkResult(1)}}, + }, { Query: `create view view1 as select public_keys."public", public_keys."count" from public_keys;`, Expected: []sql.Row{}, @@ -127,6 +133,10 @@ var AnsiQuotesTests = []ScriptTest{ Query: `show create table view1;`, Expected: []sql.Row{{"view1", "CREATE VIEW `view1` AS select public_keys.\"public\", public_keys.\"count\" from public_keys", "utf8mb4", "utf8mb4_0900_bin"}}, }, + { + Query: `select "public", "count" from view1;`, + Expected: []sql.Row{{"public", 1}}, + }, { // Assert that we can load and parse views for information_schema when ANSI_QUOTES mode is enabled Query: `select table_name, view_definition from information_schema.views where table_name='view1';`, @@ -145,6 +155,10 @@ var AnsiQuotesTests = []ScriptTest{ Query: `show create table public_keys;`, Expected: []sql.Row{{"public_keys", "CREATE TABLE `public_keys` (\n `item` int,\n `type` char(4),\n `hash` int,\n `count` int,\n `public` varchar(8000)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}}, }, + { + Query: "select public, `count` from view1;", + Expected: []sql.Row{{"public", 1}}, + }, { // Assert that we can still load and parse views for information_schema when ANSI_QUOTES mode is disabled Query: `select table_name, view_definition from information_schema.views where table_name='view1';`, diff --git a/sql/sql_mode_test.go b/sql/sql_mode_test.go index 286723e72d..fc08e2fc8b 100644 --- a/sql/sql_mode_test.go +++ b/sql/sql_mode_test.go @@ -17,34 +17,34 @@ package sql import ( "testing" - "github.com/stretchr/testify/require" + "github.com/stretchr/testify/assert" ) func TestSqlMode(t *testing.T) { // Test that ANSI mode includes ANSI_QUOTES mode sqlMode := NewSqlModeFromString("only_full_group_by,ansi") - require.True(t, sqlMode.AnsiQuotes()) - require.True(t, sqlMode.ModeEnabled("ansi")) - require.True(t, sqlMode.ModeEnabled("ANSI")) - require.True(t, sqlMode.ModeEnabled("ONLY_FULL_GROUP_BY")) - require.False(t, sqlMode.ModeEnabled("fake_mode")) - require.True(t, sqlMode.ParserOptions().AnsiQuotes) - require.Equal(t, "ONLY_FULL_GROUP_BY,ANSI", sqlMode.String()) + assert.True(t, sqlMode.AnsiQuotes()) + assert.True(t, sqlMode.ModeEnabled("ansi")) + assert.True(t, sqlMode.ModeEnabled("ANSI")) + assert.True(t, sqlMode.ModeEnabled("ONLY_FULL_GROUP_BY")) + assert.False(t, sqlMode.ModeEnabled("fake_mode")) + assert.True(t, sqlMode.ParserOptions().AnsiQuotes) + assert.Equal(t, "ONLY_FULL_GROUP_BY,ANSI", sqlMode.String()) // Test a mixed case SQL_MODE string where only ANSI_QUOTES is enabled sqlMode = NewSqlModeFromString("AnSi_quotEs") - require.True(t, sqlMode.AnsiQuotes()) - require.True(t, sqlMode.ModeEnabled("ansi_quotes")) - require.True(t, sqlMode.ModeEnabled("ANSI_quoTes")) - require.False(t, sqlMode.ModeEnabled("fake_mode")) - require.True(t, sqlMode.ParserOptions().AnsiQuotes) - require.Equal(t, "ANSI_QUOTES", sqlMode.String()) + assert.True(t, sqlMode.AnsiQuotes()) + assert.True(t, sqlMode.ModeEnabled("ansi_quotes")) + assert.True(t, sqlMode.ModeEnabled("ANSI_quoTes")) + assert.False(t, sqlMode.ModeEnabled("fake_mode")) + assert.True(t, sqlMode.ParserOptions().AnsiQuotes) + assert.Equal(t, "ANSI_QUOTES", sqlMode.String()) // Test when SQL_MODE does not include ANSI_QUOTES sqlMode = NewSqlModeFromString("ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES") - require.False(t, sqlMode.AnsiQuotes()) - require.True(t, sqlMode.ModeEnabled("STRICT_TRANS_TABLES")) - require.False(t, sqlMode.ModeEnabled("ansi_quotes")) - require.False(t, sqlMode.ParserOptions().AnsiQuotes) - require.Equal(t, "ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES", sqlMode.String()) + assert.False(t, sqlMode.AnsiQuotes()) + assert.True(t, sqlMode.ModeEnabled("STRICT_TRANS_TABLES")) + assert.False(t, sqlMode.ModeEnabled("ansi_quotes")) + assert.False(t, sqlMode.ParserOptions().AnsiQuotes) + assert.Equal(t, "ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES", sqlMode.String()) } From 9b44be03b607513dc71904881a34987b43567753 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 15 Aug 2023 16:22:25 -0700 Subject: [PATCH 18/18] PR feedback --- enginetest/queries/ansi_quotes_queries.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/enginetest/queries/ansi_quotes_queries.go b/enginetest/queries/ansi_quotes_queries.go index 8fd6e107e5..224b381d67 100644 --- a/enginetest/queries/ansi_quotes_queries.go +++ b/enginetest/queries/ansi_quotes_queries.go @@ -133,6 +133,15 @@ var AnsiQuotesTests = []ScriptTest{ Query: `show create table view1;`, Expected: []sql.Row{{"view1", "CREATE VIEW `view1` AS select public_keys.\"public\", public_keys.\"count\" from public_keys", "utf8mb4", "utf8mb4_0900_bin"}}, }, + { + // TODO: MySQL returns view definitions according to the session's current + // SQL_MODE settings, but we currently don't normalize the view + // definition based on the current setting for ANSI_QUOTES. We should + // fix that, remove the test above, and unskip this test. + Skip: true, + Query: `show create table view1;`, + Expected: []sql.Row{{"view1", "CREATE VIEW `view1` AS select public_keys.`public`, public_keys.`count` from public_keys", "utf8mb4", "utf8mb4_0900_bin"}}, + }, { Query: `select "public", "count" from view1;`, Expected: []sql.Row{{"public", 1}},