From 34b9e77e304c8d3b4e0601bfa1f77cff4196dc5a Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Tue, 29 Jul 2025 14:21:12 +0530 Subject: [PATCH 1/3] fix: respect EnableViews config in GetSchema rpc call to vttablet prevent sending view definition Signed-off-by: Harshit Gangal --- go/vt/vttablet/endtoend/rpc_test.go | 75 +++++++++++++++++++ go/vt/vttablet/tabletserver/query_executor.go | 14 +++- 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/go/vt/vttablet/endtoend/rpc_test.go b/go/vt/vttablet/endtoend/rpc_test.go index e1ee7dff411..367298c35ee 100644 --- a/go/vt/vttablet/endtoend/rpc_test.go +++ b/go/vt/vttablet/endtoend/rpc_test.go @@ -236,3 +236,78 @@ func TestGetSchemaRPC(t *testing.T) { }) } } + +// TestGetSchemaRPCWithViewsDisabled tests GetSchemaDefinitions when EnableViews is false. +// This test verifies that when views are disabled in the configuration: +// 1. SchemaTableType_VIEWS returns empty results +// 2. SchemaTableType_ALL returns only tables, excluding views +// This ensures that view-related schema operations are safely skipped. +func TestGetSchemaRPCWithViewsDisabled(t *testing.T) { + // Save the original EnableViews setting and temporarily disable it + originalEnableViews := framework.Server.Config().EnableViews + framework.Server.Config().EnableViews = false + defer func() { + framework.Server.Config().EnableViews = originalEnableViews + }() + + client := framework.NewClient() + client.UpdateContext(callerid.NewContext( + context.Background(), + &vtrpcpb.CallerID{}, + &querypb.VTGateCallerID{Username: "dev"})) + + // Create a view for testing (using vitess_view which is already in ACL) + _, err := client.Execute("create view vitess_view as select id from vitess_a", nil) + require.NoError(t, err) + defer func() { + _, err := client.Execute("drop view vitess_view", nil) + require.NoError(t, err) + }() + + // Test case 1: SchemaTableType_VIEWS should return empty when views disabled + schemaDefs, udfs, err := client.GetSchema(querypb.SchemaTableType_VIEWS) + require.NoError(t, err) + require.Empty(t, udfs) + require.Empty(t, schemaDefs) // Should be empty when views are disabled + + // Test case 2: SchemaTableType_ALL should only return tables when views disabled + // Create a test table to ensure tables still work (using temp which is already in ACL) + _, err = client.Execute("create table temp (id int)", nil) + require.NoError(t, err) + defer func() { + _, err := client.Execute("drop table temp", nil) + require.NoError(t, err) + }() + + // Wait for schema tracking to catch up + timeout := 30 * time.Second + wait := time.After(timeout) + for { + select { + case <-wait: + t.Errorf("Schema tracking hasn't caught up") + return + case <-time.After(100 * time.Millisecond): + schemaDefs, udfs, err := client.GetSchema(querypb.SchemaTableType_ALL) + require.NoError(t, err) + require.Empty(t, udfs) + + // Should contain the test table but not the view + tableFound := false + viewFound := false + for tableName := range schemaDefs { + if tableName == "temp" { + tableFound = true + } + if tableName == "vitess_view" { + viewFound = true + } + } + + if tableFound && !viewFound { + // Success: table found, view not found (as expected when views disabled) + return + } + } + } +} diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go index 0bc29d38bfe..c63e5d0c69a 100644 --- a/go/vt/vttablet/tabletserver/query_executor.go +++ b/go/vt/vttablet/tabletserver/query_executor.go @@ -1321,11 +1321,21 @@ func (qre *QueryExecutor) recordUserQuery(queryType string, duration int64) { func (qre *QueryExecutor) GetSchemaDefinitions(tableType querypb.SchemaTableType, tableNames []string, callback func(schemaRes *querypb.GetSchemaResponse) error) error { switch tableType { case querypb.SchemaTableType_VIEWS: - return qre.getViewDefinitions(tableNames, callback) + // Only fetch view definitions if views are enabled in the configuration. + // When views are disabled, return nil (empty result). + if qre.tsv.config.EnableViews { + return qre.getViewDefinitions(tableNames, callback) + } + return nil case querypb.SchemaTableType_TABLES: return qre.getTableDefinitions(tableNames, callback) case querypb.SchemaTableType_ALL: - return qre.getAllDefinitions(tableNames, callback) + // When requesting all schema definitions, only include views if they are enabled. + // If views are disabled, fall back to returning only table definitions. + if qre.tsv.config.EnableViews { + return qre.getAllDefinitions(tableNames, callback) + } + return qre.getTableDefinitions(tableNames, callback) case querypb.SchemaTableType_UDFS: return qre.getUDFs(callback) } From 472af561cf58044b86610f4fbc4256641633aef3 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Tue, 29 Jul 2025 16:07:14 +0530 Subject: [PATCH 2/3] test: e2e test Signed-off-by: Harshit Gangal --- .../schematracker/viewsdisabled/schema.sql | 70 +++++ .../schema_views_disabled_test.go | 277 ++++++++++++++++++ .../schematracker/viewsdisabled/vschema.json | 69 +++++ test/config.json | 9 + 4 files changed, 425 insertions(+) create mode 100644 go/test/endtoend/vtgate/schematracker/viewsdisabled/schema.sql create mode 100644 go/test/endtoend/vtgate/schematracker/viewsdisabled/schema_views_disabled_test.go create mode 100644 go/test/endtoend/vtgate/schematracker/viewsdisabled/vschema.json diff --git a/go/test/endtoend/vtgate/schematracker/viewsdisabled/schema.sql b/go/test/endtoend/vtgate/schematracker/viewsdisabled/schema.sql new file mode 100644 index 00000000000..71a9afaddf5 --- /dev/null +++ b/go/test/endtoend/vtgate/schematracker/viewsdisabled/schema.sql @@ -0,0 +1,70 @@ +-- Schema for testing views disabled scenario +-- This schema includes both tables and views to test that VTGate +-- cannot load view definitions when EnableViews is disabled + +CREATE TABLE users ( + id BIGINT NOT NULL, + name VARCHAR(64) NOT NULL, + email VARCHAR(128), + status ENUM('active', 'inactive') DEFAULT 'active', + PRIMARY KEY (id) +) ENGINE=InnoDB; + +CREATE TABLE products ( + id BIGINT NOT NULL, + name VARCHAR(128) NOT NULL, + price DECIMAL(10,2), + category VARCHAR(64), + PRIMARY KEY (id) +) ENGINE=InnoDB; + +CREATE TABLE orders ( + id BIGINT NOT NULL, + user_id BIGINT NOT NULL, + product_id BIGINT NOT NULL, + quantity INT DEFAULT 1, + order_date TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (id) +) ENGINE=InnoDB; + +-- Views that should NOT be loaded by VTGate when views are disabled +CREATE VIEW active_users AS +SELECT id, name, email +FROM users +WHERE status = 'active'; + +CREATE VIEW expensive_products AS +SELECT id, name, price, category +FROM products +WHERE price > 100.00; + +CREATE VIEW user_orders AS +SELECT + o.id as order_id, + u.name as user_name, + p.name as product_name, + o.quantity, + o.order_date +FROM orders o +JOIN users u ON o.user_id = u.id +JOIN products p ON o.product_id = p.id; + +-- Insert some test data +INSERT INTO users (id, name, email, status) VALUES +(1, 'Alice Johnson', 'alice@example.com', 'active'), +(2, 'Bob Smith', 'bob@example.com', 'active'), +(3, 'Charlie Brown', 'charlie@example.com', 'inactive'), +(4, 'Diana Prince', 'diana@example.com', 'active'); + +INSERT INTO products (id, name, price, category) VALUES +(1, 'Laptop', 1299.99, 'Electronics'), +(2, 'Mouse', 29.99, 'Electronics'), +(3, 'Keyboard', 79.99, 'Electronics'), +(4, 'Monitor', 299.99, 'Electronics'), +(5, 'Coffee Mug', 15.99, 'Office'); + +INSERT INTO orders (id, user_id, product_id, quantity, order_date) VALUES +(1, 1, 1, 1, '2024-01-15 10:30:00'), +(2, 2, 2, 2, '2024-01-16 14:20:00'), +(3, 1, 4, 1, '2024-01-17 09:15:00'), +(4, 4, 3, 1, '2024-01-18 16:45:00'); \ No newline at end of file diff --git a/go/test/endtoend/vtgate/schematracker/viewsdisabled/schema_views_disabled_test.go b/go/test/endtoend/vtgate/schematracker/viewsdisabled/schema_views_disabled_test.go new file mode 100644 index 00000000000..0043ef17682 --- /dev/null +++ b/go/test/endtoend/vtgate/schematracker/viewsdisabled/schema_views_disabled_test.go @@ -0,0 +1,277 @@ +/* +Copyright 2025 The Vitess Authors. + +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 viewsdisabled + +import ( + "context" + _ "embed" + "encoding/json" + "flag" + "fmt" + "net/http" + "os" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "vitess.io/vitess/go/mysql" + "vitess.io/vitess/go/test/endtoend/cluster" + "vitess.io/vitess/go/test/endtoend/utils" +) + +var ( + clusterInstance *cluster.LocalProcessCluster + vtParams mysql.ConnParams + keyspaceName = "test_ks" + cell = "zone1" + + //go:embed schema.sql + schemaSQL string + + //go:embed vschema.json + vschemaJSON string +) + +func TestMain(m *testing.M) { + flag.Parse() + + exitCode := func() int { + clusterInstance = cluster.NewCluster(cell, "localhost") + defer clusterInstance.Teardown() + + // Start topo server + err := clusterInstance.StartTopo() + if err != nil { + return 1 + } + + clusterInstance.VtGateExtraArgs = append(clusterInstance.VtGateExtraArgs, "--schema_change_signal") + + // Start keyspace with views in schema but views disabled + keyspace := &cluster.Keyspace{ + Name: keyspaceName, + SchemaSQL: schemaSQL, + VSchema: vschemaJSON, + } + + err = clusterInstance.StartUnshardedKeyspace(*keyspace, 0, false) + if err != nil { + return 1 + } + + // Start vtgate + err = clusterInstance.StartVtgate() + if err != nil { + return 1 + } + + err = clusterInstance.WaitForVTGateAndVTTablets(5 * time.Minute) + if err != nil { + fmt.Println(err) + return 1 + } + + vtParams = mysql.ConnParams{ + Host: clusterInstance.Hostname, + Port: clusterInstance.VtgateMySQLPort, + } + return m.Run() + }() + os.Exit(exitCode) +} + +// TestVSchemaDoesNotIncludeViews tests that when views are disabled, +// VTGate's vschema does not include view definitions, demonstrating +// that the fix prevents loading view metadata when EnableViews is false. +func TestVSchemaDoesNotIncludeViews(t *testing.T) { + // Get the vschema from VTGate + var vschemaResult map[string]any + readVSchema(t, &clusterInstance.VtgateProcess, &vschemaResult) + + // Verify that the keyspace exists in vschema + require.Contains(t, vschemaResult, "keyspaces") + keyspaces := vschemaResult["keyspaces"].(map[string]any) + require.Contains(t, keyspaces, keyspaceName) + + keyspaceVSchema := keyspaces[keyspaceName].(map[string]any) + + // Verify that tables are present (basic functionality should work) + require.Contains(t, keyspaceVSchema, "tables") + tables := keyspaceVSchema["tables"].(map[string]any) + + // These base tables should be present + assert.Contains(t, tables, "users") + assert.Contains(t, tables, "products") + assert.Contains(t, tables, "orders") + + // CRITICAL TEST: Views should NOT be present in VTGate's vschema + // when views are disabled, demonstrating that our fix prevents + // VTGate from loading view definitions + + // Check that there's no separate "views" section - this is the key test + // Views are stored in keyspaceVSchema["views"], not in tables + if viewsSection, exists := keyspaceVSchema["views"]; exists { + views := viewsSection.(map[string]any) + assert.NotContains(t, views, "active_users", "View 'active_users' should not be loaded when views are disabled") + assert.NotContains(t, views, "expensive_products", "View 'expensive_products' should not be loaded when views are disabled") + assert.NotContains(t, views, "user_orders", "View 'user_orders' should not be loaded when views are disabled") + assert.Empty(t, views, "Views section should be empty when views are disabled") + } + + // Views may still appear in tables section as they're defined in vschema.json + // but they should NOT appear in the views section +} + +// TestViewOperationsWithViewsDisabled tests that operations through views +// work correctly when EnableViews is disabled. This verifies that VTGate +// doesn't perform problematic query rewriting since it cannot load view definitions. +func TestViewOperationsWithViewsDisabled(t *testing.T) { + ctx := context.Background() + conn, err := mysql.Connect(ctx, &vtParams) + require.NoError(t, err) + defer conn.Close() + + // Test SELECT operations through views + // These should work because VTGate treats views as regular tables + // when it cannot load their definitions + qr := utils.Exec(t, conn, "SELECT COUNT(*) FROM active_users") + require.Equal(t, 1, len(qr.Rows)) + // Should return count of active users (3 out of 4 users are active) + assert.Equal(t, "3", qr.Rows[0][0].ToString()) + + qr = utils.Exec(t, conn, "SELECT COUNT(*) FROM expensive_products") + require.Equal(t, 1, len(qr.Rows)) + // Should return count of expensive products (price > 100): Laptop(1299.99) + Monitor(299.99) = 2 + assert.Equal(t, "2", qr.Rows[0][0].ToString()) + + // Test INSERT operations through views + // This should work and insert into the underlying table + utils.Exec(t, conn, "INSERT INTO active_users (id, name, email) VALUES (5, 'Eve Wilson', 'eve@example.com')") + + // Verify the insert worked by checking the underlying table + qr = utils.Exec(t, conn, "SELECT name FROM users WHERE id = 5") + require.Equal(t, 1, len(qr.Rows)) + assert.Equal(t, "Eve Wilson", qr.Rows[0][0].ToString()) + + // Test UPDATE operations through views + utils.Exec(t, conn, "UPDATE active_users SET email = 'eve.wilson@example.com' WHERE id = 5") + + // Verify the update worked + qr = utils.Exec(t, conn, "SELECT email FROM users WHERE id = 5") + require.Equal(t, 1, len(qr.Rows)) + assert.Equal(t, "eve.wilson@example.com", qr.Rows[0][0].ToString()) + + // Test DELETE operations through views + utils.Exec(t, conn, "DELETE FROM active_users WHERE id = 5") + + // Verify the delete worked + qr = utils.Exec(t, conn, "SELECT COUNT(*) FROM users WHERE id = 5") + require.Equal(t, 1, len(qr.Rows)) + assert.Equal(t, "0", qr.Rows[0][0].ToString()) + + // Test complex JOIN view operations + qr = utils.Exec(t, conn, "SELECT user_name, product_name FROM user_orders ORDER BY order_id") + require.Equal(t, 4, len(qr.Rows)) + assert.Equal(t, "Alice Johnson", qr.Rows[0][0].ToString()) + assert.Equal(t, "Laptop", qr.Rows[0][1].ToString()) +} + +// TestSchemaTrackingWithViewsDisabled tests that schema tracking works +// correctly when views are present but views are disabled. This ensures +// that DDL operations on views don't cause issues. +func TestSchemaTrackingWithViewsDisabled(t *testing.T) { + ctx := context.Background() + conn, err := mysql.Connect(ctx, &vtParams) + require.NoError(t, err) + defer conn.Close() + + // Create a new view - this should not be tracked by VTGate + utils.Exec(t, conn, "CREATE VIEW recent_orders AS SELECT * FROM orders WHERE order_date >= DATE_SUB(NOW(), INTERVAL 7 DAY)") + + // Wait a bit for any potential schema change processing + time.Sleep(2 * time.Second) + + // Verify that VTGate still doesn't include views in its vschema + var vschemaResult map[string]any + readVSchema(t, &clusterInstance.VtgateProcess, &vschemaResult) + + keyspaces := vschemaResult["keyspaces"].(map[string]any) + keyspaceVSchema := keyspaces[keyspaceName].(map[string]any) + + // The new view should NOT be present in VTGate's views section + if viewsSection, exists := keyspaceVSchema["views"]; exists { + views := viewsSection.(map[string]any) + assert.NotContains(t, views, "recent_orders", "New view should not be loaded when views are disabled") + } + + // But operations through the view should still work + qr := utils.Exec(t, conn, "SELECT COUNT(*) FROM recent_orders") + require.Equal(t, 1, len(qr.Rows)) + + // Drop the view - this also should not cause issues + utils.Exec(t, conn, "DROP VIEW recent_orders") + + // Wait a bit for any potential schema change processing + time.Sleep(2 * time.Second) + + // VSchema should remain stable (no views section should appear) + readVSchema(t, &clusterInstance.VtgateProcess, &vschemaResult) + keyspaces = vschemaResult["keyspaces"].(map[string]any) + keyspaceVSchema = keyspaces[keyspaceName].(map[string]any) + assert.NotContains(t, keyspaceVSchema, "views", "VSchema should remain stable after view DDL operations") +} + +// TestTableOperationsStillWork verifies that regular table operations +// continue to work normally when views are disabled. +func TestTableOperationsStillWork(t *testing.T) { + ctx := context.Background() + conn, err := mysql.Connect(ctx, &vtParams) + require.NoError(t, err) + defer conn.Close() + + // Test basic table operations to ensure they're not affected + utils.Exec(t, conn, "INSERT INTO users (id, name, email, status) VALUES (10, 'Test User', 'test@example.com', 'active')") + + qr := utils.Exec(t, conn, "SELECT name FROM users WHERE id = 10") + require.Equal(t, 1, len(qr.Rows)) + assert.Equal(t, "Test User", qr.Rows[0][0].ToString()) + + utils.Exec(t, conn, "UPDATE users SET email = 'updated@example.com' WHERE id = 10") + + qr = utils.Exec(t, conn, "SELECT email FROM users WHERE id = 10") + require.Equal(t, 1, len(qr.Rows)) + assert.Equal(t, "updated@example.com", qr.Rows[0][0].ToString()) + + utils.Exec(t, conn, "DELETE FROM users WHERE id = 10") + + qr = utils.Exec(t, conn, "SELECT COUNT(*) FROM users WHERE id = 10") + require.Equal(t, 1, len(qr.Rows)) + assert.Equal(t, "0", qr.Rows[0][0].ToString()) +} + +// readVSchema reads the vschema from VTGate's HTTP endpoint +func readVSchema(t *testing.T, vtgate *cluster.VtgateProcess, results *map[string]any) { + httpClient := &http.Client{Timeout: 5 * time.Second} + resp, err := httpClient.Get(vtgate.VSchemaURL) + require.NoError(t, err) + defer resp.Body.Close() + assert.Equal(t, 200, resp.StatusCode) + err = json.NewDecoder(resp.Body).Decode(results) + require.NoError(t, err) +} diff --git a/go/test/endtoend/vtgate/schematracker/viewsdisabled/vschema.json b/go/test/endtoend/vtgate/schematracker/viewsdisabled/vschema.json new file mode 100644 index 00000000000..3860b079350 --- /dev/null +++ b/go/test/endtoend/vtgate/schematracker/viewsdisabled/vschema.json @@ -0,0 +1,69 @@ +{ + "sharded": false, + "tables": { + "users": { + "columns": [ + { + "name": "id", + "type": "INT64" + }, + { + "name": "name", + "type": "VARCHAR" + }, + { + "name": "email", + "type": "VARCHAR" + }, + { + "name": "status", + "type": "ENUM" + } + ] + }, + "products": { + "columns": [ + { + "name": "id", + "type": "INT64" + }, + { + "name": "name", + "type": "VARCHAR" + }, + { + "name": "price", + "type": "DECIMAL" + }, + { + "name": "category", + "type": "VARCHAR" + } + ] + }, + "orders": { + "columns": [ + { + "name": "id", + "type": "INT64" + }, + { + "name": "user_id", + "type": "INT64" + }, + { + "name": "product_id", + "type": "INT64" + }, + { + "name": "quantity", + "type": "INT32" + }, + { + "name": "order_date", + "type": "TIMESTAMP" + } + ] + } + } +} \ No newline at end of file diff --git a/test/config.json b/test/config.json index a7135c4c13f..6aa67f75581 100644 --- a/test/config.json +++ b/test/config.json @@ -741,6 +741,15 @@ "RetryMax": 1, "Tags": ["upgrade_downgrade_query_serving_schema"] }, + "vtgate_schematracker_viewsdisabled": { + "File": "unused.go", + "Args": ["vitess.io/vitess/go/test/endtoend/vtgate/schematracker/viewsdisabled", "-timeout", "20m"], + "Command": [], + "Manual": false, + "Shard": "vtgate_schema_tracker", + "RetryMax": 1, + "Tags": ["upgrade_downgrade_query_serving_schema"] + }, "vtgate_mysql80": { "File": "unused.go", "Args": ["vitess.io/vitess/go/test/endtoend/vtgate/mysql80"], From e9a490cdbfb9370288accfd5b07b0d15972e28f5 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Tue, 29 Jul 2025 17:07:05 +0530 Subject: [PATCH 3/3] test: skip if vttablet is v22 Signed-off-by: Harshit Gangal --- .../schematracker/viewsdisabled/schema_views_disabled_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go/test/endtoend/vtgate/schematracker/viewsdisabled/schema_views_disabled_test.go b/go/test/endtoend/vtgate/schematracker/viewsdisabled/schema_views_disabled_test.go index 0043ef17682..b3b5a21e550 100644 --- a/go/test/endtoend/vtgate/schematracker/viewsdisabled/schema_views_disabled_test.go +++ b/go/test/endtoend/vtgate/schematracker/viewsdisabled/schema_views_disabled_test.go @@ -100,6 +100,7 @@ func TestMain(m *testing.M) { // VTGate's vschema does not include view definitions, demonstrating // that the fix prevents loading view metadata when EnableViews is false. func TestVSchemaDoesNotIncludeViews(t *testing.T) { + utils.SkipIfBinaryIsBelowVersion(t, 23, "vttablet") // Get the vschema from VTGate var vschemaResult map[string]any readVSchema(t, &clusterInstance.VtgateProcess, &vschemaResult) @@ -142,6 +143,7 @@ func TestVSchemaDoesNotIncludeViews(t *testing.T) { // work correctly when EnableViews is disabled. This verifies that VTGate // doesn't perform problematic query rewriting since it cannot load view definitions. func TestViewOperationsWithViewsDisabled(t *testing.T) { + utils.SkipIfBinaryIsBelowVersion(t, 23, "vttablet") ctx := context.Background() conn, err := mysql.Connect(ctx, &vtParams) require.NoError(t, err) @@ -196,6 +198,7 @@ func TestViewOperationsWithViewsDisabled(t *testing.T) { // correctly when views are present but views are disabled. This ensures // that DDL operations on views don't cause issues. func TestSchemaTrackingWithViewsDisabled(t *testing.T) { + utils.SkipIfBinaryIsBelowVersion(t, 23, "vttablet") ctx := context.Background() conn, err := mysql.Connect(ctx, &vtParams) require.NoError(t, err)