feat: make sure to not expand expressions for create/alter view

Signed-off-by: Andres Taylor <[email protected]>

go/test/endtoend/vtgate/queries/misc/misc_test.go

@@ -395,7 +395,7 @@ func TestAlterTableWithView(t *testing.T) {
 	mcmp.Exec(`insert into t1(id1, id2) values (1, 1)`)
 	mcmp.AssertMatches("select * from v1", `[[INT64(1) INT64(1)]]`)
 
-	// alter table
+	// alter table add column
 	mcmp.Exec(`alter table t1 add column test bigint`)
 	mcmp.Exec(`alter view v1 as select * from t1`)
 

go/vt/vtgate/planbuilder/ddl.go

@@ -192,64 +192,63 @@ func findTableDestinationAndKeyspace(vschema plancontext.VSchema, ddlStatement s
 	return destination, keyspace, nil
 }
 
-func buildAlterView(ctx context.Context, vschema plancontext.VSchema, ddl *sqlparser.AlterView, reservedVars *sqlparser.ReservedVars, enableOnlineDDL, enableDirectDDL bool) (key.Destination, *vindexes.Keyspace, error) {
-	// For Alter View, we require that the view exist and the select query can be satisfied within the keyspace itself
-	// We should remove the keyspace name from the table name, as the database name in MySQL might be different than the keyspace name
-	destination, keyspace, err := findTableDestinationAndKeyspace(vschema, ddl)
-	if err != nil {
-		return nil, nil, err
-	}
+func buildAlterView(
+	ctx context.Context,
+	vschema plancontext.VSchema,
+	ddl *sqlparser.AlterView,
+	reservedVars *sqlparser.ReservedVars,
+	enableOnlineDDL, enableDirectDDL bool,
+) (key.Destination, *vindexes.Keyspace, error) {
+	return buildCreateViewCommon(ctx, vschema, reservedVars, enableOnlineDDL, enableDirectDDL, ddl.Select, ddl)
+}
 
-	selectPlan, err := createInstructionFor(ctx, sqlparser.String(ddl.Select), ddl.Select, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
-	if err != nil {
-		return nil, nil, err
-	}
-	selPlanKs := selectPlan.primitive.GetKeyspaceName()
-	if keyspace.Name != selPlanKs {
-		return nil, nil, vterrors.VT12001(ViewDifferentKeyspace)
-	}
-	if vschema.IsViewsEnabled() {
-		if keyspace == nil {
-			return nil, nil, vterrors.VT09005()
-		}
-		return destination, keyspace, nil
-	}
-	isRoutePlan, opCode := tryToGetRoutePlan(selectPlan.primitive)
-	if !isRoutePlan {
-		return nil, nil, vterrors.VT12001(ViewComplex)
-	}
-	if opCode != engine.Unsharded && opCode != engine.EqualUnique && opCode != engine.Scatter {
-		return nil, nil, vterrors.VT12001(ViewComplex)
-	}
-	_ = sqlparser.SafeRewrite(ddl.Select, nil, func(cursor *sqlparser.Cursor) bool {
-		switch tableName := cursor.Node().(type) {
-		case sqlparser.TableName:
-			cursor.Replace(sqlparser.TableName{
-				Name: tableName.Name,
-			})
-		}
-		return true
-	})
-	return destination, keyspace, nil
+func buildCreateView(
+	ctx context.Context,
+	vschema plancontext.VSchema,
+	ddl *sqlparser.CreateView,
+	reservedVars *sqlparser.ReservedVars,
+	enableOnlineDDL, enableDirectDDL bool,
+) (key.Destination, *vindexes.Keyspace, error) {
+	return buildCreateViewCommon(ctx, vschema, reservedVars, enableOnlineDDL, enableDirectDDL, ddl.Select, ddl)
 }
 
-func buildCreateView(ctx context.Context, vschema plancontext.VSchema, ddl *sqlparser.CreateView, reservedVars *sqlparser.ReservedVars, enableOnlineDDL, enableDirectDDL bool) (key.Destination, *vindexes.Keyspace, error) {
+func buildCreateViewCommon(
+	ctx context.Context,
+	vschema plancontext.VSchema,
+	reservedVars *sqlparser.ReservedVars,
+	enableOnlineDDL, enableDirectDDL bool,
+	ddlSelect sqlparser.SelectStatement,
+	ddl sqlparser.DDLStatement,
+) (key.Destination, *vindexes.Keyspace, error) {
 	// For Create View, we require that the keyspace exist and the select query can be satisfied within the keyspace itself
 	// We should remove the keyspace name from the table name, as the database name in MySQL might be different than the keyspace name
-	destination, keyspace, _, err := vschema.TargetDestination(ddl.ViewName.Qualifier.String())
+	destination, keyspace, err := findTableDestinationAndKeyspace(vschema, ddl)
 	if err != nil {
 		return nil, nil, err
 	}
-	ddl.ViewName.Qualifier = sqlparser.NewIdentifierCS("")
 
-	selectPlan, err := createInstructionFor(ctx, sqlparser.String(ddl.Select), ddl.Select, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
+	// because we don't trust the schema tracker to have up-to-date info, we don't want to expand any SELECT * here
+	var expressions []sqlparser.SelectExprs
+	_ = sqlparser.VisitAllSelects(ddlSelect, func(p *sqlparser.Select, idx int) error {
+		expressions = append(expressions, sqlparser.CloneSelectExprs(p.SelectExprs))
+		return nil
+	})
+	selectPlan, err := createInstructionFor(ctx, sqlparser.String(ddlSelect), ddlSelect, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
 	if err != nil {
 		return nil, nil, err
 	}
 	selPlanKs := selectPlan.primitive.GetKeyspaceName()
 	if keyspace.Name != selPlanKs {
 		return nil, nil, vterrors.VT12001(ViewDifferentKeyspace)
 	}
+
+	_ = sqlparser.VisitAllSelects(ddlSelect, func(p *sqlparser.Select, idx int) error {
+		p.SelectExprs = expressions[idx]
+		return nil
+	})
+
+	sqlparser.RemoveKeyspace(ddl)
+
 	if vschema.IsViewsEnabled() {
 		if keyspace == nil {
 			return nil, nil, vterrors.VT09005()
@@ -263,15 +262,6 @@ func buildCreateView(ctx context.Context, vschema plancontext.VSchema, ddl *sqlp
 	if opCode != engine.Unsharded && opCode != engine.EqualUnique && opCode != engine.Scatter {
 		return nil, nil, vterrors.VT12001(ViewComplex)
 	}
-	_ = sqlparser.SafeRewrite(ddl.Select, nil, func(cursor *sqlparser.Cursor) bool {
-		switch tableName := cursor.Node().(type) {
-		case sqlparser.TableName:
-			cursor.Replace(sqlparser.TableName{
-				Name: tableName.Name,
-			})
-		}
-		return true
-	})
 	return destination, keyspace, nil
 }
 

go/vt/vtgate/planbuilder/testdata/ddl_cases.json

@@ -374,7 +374,7 @@
           "Name": "user",
           "Sharded": true
         },
-        "Query": "create view tmp_view as select user_id, col1, col2 from authoritative"
+        "Query": "create view tmp_view as select * from authoritative"
       },
       "TablesUsed": [
         "user.tmp_view"

go/vt/vtgate/planbuilder/testdata/ddl_cases_no_default_keyspace.json

@@ -125,7 +125,7 @@
           "Name": "user",
           "Sharded": true
         },
-        "Query": "create view view_a as select user_id, col1, col2 from authoritative"
+        "Query": "create view view_a as select * from authoritative"
       },
       "TablesUsed": [
         "user.view_a"
@@ -144,7 +144,7 @@
           "Name": "user",
           "Sharded": true
         },
-        "Query": "create view view_a as select a.user_id, a.col1, a.col2, b.user_id, b.col1, b.col2 from authoritative as a join authoritative as b on a.user_id = b.user_id"
+        "Query": "create view view_a as select * from authoritative as a join authoritative as b on a.user_id = b.user_id"
       },
       "TablesUsed": [
         "user.view_a"
@@ -163,7 +163,7 @@
           "Name": "user",
           "Sharded": true
         },
-        "Query": "create view view_a as select user_id, col1, col2 from authoritative as a"
+        "Query": "create view view_a as select a.* from authoritative as a"
       },
       "TablesUsed": [
         "user.view_a"
@@ -201,7 +201,7 @@
           "Name": "user",
           "Sharded": true
         },
-        "Query": "create view view_a as select `user`.id, a.user_id, a.col1, a.col2, `user`.col1 from authoritative as a join `user` on a.user_id = `user`.id"
+        "Query": "create view view_a as select `user`.id, a.*, `user`.col1 from authoritative as a join `user` on a.user_id = `user`.id"
       },
       "TablesUsed": [
         "user.view_a"

go/vt/vtgate/planbuilder/testdata/view_cases.json

@@ -11,7 +11,7 @@
           "Name": "user",
           "Sharded": true
         },
-        "Query": "alter view user_extra as select * from `user`.`user`"
+        "Query": "alter view user_extra as select * from `user`"
       },
       "TablesUsed": [
         "user.user_extra"
@@ -35,7 +35,7 @@
           "Name": "user",
           "Sharded": true
         },
-        "Query": "create view view_ac as select user_id, col1, col2 from authoritative"
+        "Query": "create view view_ac as select * from authoritative"
       },
       "TablesUsed": [
         "user.view_ac"

go/vt/vttablet/onlineddl/executor.go

@@ -3069,6 +3069,7 @@ func (e *Executor) executeAlterViewOnline(ctx context.Context, onlineDDL *schema
 		// it actually easier for us to issue a CREATE OR REPLACE, because it
 		// actually creates a view...
 		stmt = &sqlparser.CreateView{
+
 			Algorithm:   viewStmt.Algorithm,
 			Definer:     viewStmt.Definer,
 			Security:    viewStmt.Security,