From db61f7d8997625d02bb30e2449b97663615a3a6b Mon Sep 17 00:00:00 2001
From: Harshit Gangal <harshit@planetscale.com>
Date: Wed, 13 Oct 2021 22:15:01 +0530
Subject: [PATCH] record plan warnings, added e2e test and unit test

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
---
 go/test/endtoend/vtgate/gen4/gen4_test.go | 22 +++++++++++++++
 go/vt/vtgate/executor_select_test.go      | 33 +++++++++++++++++++++++
 go/vt/vtgate/planbuilder/gen4_planner.go  |  9 ++++---
 3 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/go/test/endtoend/vtgate/gen4/gen4_test.go b/go/test/endtoend/vtgate/gen4/gen4_test.go
index de18e1aa257..bb350f7632a 100644
--- a/go/test/endtoend/vtgate/gen4/gen4_test.go
+++ b/go/test/endtoend/vtgate/gen4/gen4_test.go
@@ -196,6 +196,28 @@ func TestSubQueries(t *testing.T) {
 	assertMatches(t, conn, `select (select id from t2 order by id limit 1) from t2 order by id limit 2`, `[[INT64(1)] [INT64(1)]]`)
 }
 
+func TestPlannerWarning(t *testing.T) {
+	ctx := context.Background()
+	conn, err := mysql.Connect(ctx, &vtParams)
+	require.NoError(t, err)
+	defer conn.Close()
+
+	// straight_join query
+	_ = checkedExec(t, conn, `select 1 from t1 straight_join t2 on t1.id = t2.id`)
+	assertMatches(t, conn, `show warnings`, `[[VARCHAR("Warning") UINT16(1235) VARCHAR("straight join is converted to normal join")]]`)
+
+	// execute same query again.
+	_ = checkedExec(t, conn, `select 1 from t1 straight_join t2 on t1.id = t2.id`)
+	assertMatches(t, conn, `show warnings`, `[[VARCHAR("Warning") UINT16(1235) VARCHAR("straight join is converted to normal join")]]`)
+
+	// random query to reset the warning.
+	_ = checkedExec(t, conn, `select 1 from t1`)
+
+	// execute same query again.
+	_ = checkedExec(t, conn, `select 1 from t1 straight_join t2 on t1.id = t2.id`)
+	assertMatches(t, conn, `show warnings`, `[[VARCHAR("Warning") UINT16(1235) VARCHAR("straight join is converted to normal join")]]`)
+}
+
 func assertMatches(t *testing.T, conn *mysql.Conn, query, expected string) {
 	t.Helper()
 	qr := checkedExec(t, conn, query)
diff --git a/go/vt/vtgate/executor_select_test.go b/go/vt/vtgate/executor_select_test.go
index e471b91147c..2e33f3930cc 100644
--- a/go/vt/vtgate/executor_select_test.go
+++ b/go/vt/vtgate/executor_select_test.go
@@ -2675,3 +2675,36 @@ func TestSelectScatterFails(t *testing.T) {
 	_, err = executorExecSession(executor, "select /*vt+ ALLOW_SCATTER */ id from user", nil, sess)
 	require.NoError(t, err)
 }
+
+func TestGen4SelectStraightJoin(t *testing.T) {
+	executor, sbc1, _, _ := createExecutorEnv()
+	executor.normalize = true
+	*plannerVersion = "gen4"
+	defer func() {
+		// change it back to v3
+		*plannerVersion = "v3"
+	}()
+
+	session := NewSafeSession(&vtgatepb.Session{TargetString: "TestExecutor"})
+	query := "select u.id from user u straight_join user2 u2 on u.id = u2.id"
+	_, err := executor.Execute(context.Background(),
+		"TestGen4SelectStraightJoin",
+		session,
+		query, map[string]*querypb.BindVariable{},
+	)
+	require.NoError(t, err)
+	wantQueries := []*querypb.BoundQuery{
+		{
+			Sql:           "select u.id from `user` as u, user2 as u2 where u.id = u2.id",
+			BindVariables: map[string]*querypb.BindVariable{},
+		},
+	}
+	wantWarnings := []*querypb.QueryWarning{
+		{
+			Code:    1235,
+			Message: "straight join is converted to normal join",
+		},
+	}
+	utils.MustMatch(t, wantQueries, sbc1.Queries)
+	utils.MustMatch(t, wantWarnings, session.Warnings)
+}
diff --git a/go/vt/vtgate/planbuilder/gen4_planner.go b/go/vt/vtgate/planbuilder/gen4_planner.go
index cf8a026c91f..a804f972b22 100644
--- a/go/vt/vtgate/planbuilder/gen4_planner.go
+++ b/go/vt/vtgate/planbuilder/gen4_planner.go
@@ -78,10 +78,9 @@ func gen4planSQLCalcFoundRows(vschema ContextVSchema, sel *sqlparser.Select, que
 	if err != nil {
 		return nil, err
 	}
-	if semTable.Warning != "" {
-		// log it as planning warning.
-		vschema.PlannerWarning(semTable.Warning)
-	}
+	// record any warning as planner warning.
+	vschema.PlannerWarning(semTable.Warning)
+
 	plan, err := buildSQLCalcFoundRowsPlan(query, sel, reservedVars, vschema, planSelectGen4)
 	if err != nil {
 		return nil, err
@@ -124,6 +123,8 @@ func newBuildSelectPlan(selStmt sqlparser.SelectStatement, reservedVars *sqlpars
 	if err != nil {
 		return nil, err
 	}
+	// record any warning as planner warning.
+	vschema.PlannerWarning(semTable.Warning)
 
 	err = queryRewrite(semTable, reservedVars, selStmt)
 	if err != nil {