diff --git a/TEMPLATES.md b/TEMPLATES.md index 920c50ba7..333a56112 100644 --- a/TEMPLATES.md +++ b/TEMPLATES.md @@ -46,9 +46,38 @@ ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`); - `users`: Column `created_at` uses `TIMESTAMP` which overflows on 2038-01-19. Consider using `DATETIME` instead. - `products`: Index `idx_category` on column `category` is redundant - covered by index `idx_category_price` on columns (`category`, `price`) +πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter +--- + +▢️ **To apply** all schema changes from this PR, comment: +``` +schemabot apply -e staging +``` + + + +
+MySQL Plan (Column-only Alter, No Table Sizes) + + +## Schema Change Plan β€” Staging + +**Database**: `testapp` | **Type**: `MySQL` | **Schema Name**: `testapp` + +*Requested by @jackjackbits at 2026-01-01 00:00:00 UTC Β· planned from [`abcdef1`](https://github.com/block/schemabot/commit/abcdef1234567890abcdef1234567890abcdef12)* + +```sql +ALTER TABLE `products` ADD COLUMN `discount_cents` bigint; +``` + +πŸ“‹ **Plan**: **1** table to alter + + --- ▢️ **To apply** all schema changes from this PR, comment: @@ -93,6 +122,9 @@ CREATE TABLE `orders` ( ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`); ``` +πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter ℹ️ Namespaces excluded from this plan by `ignore_namespaces`: `local_fixtures` @@ -543,6 +575,9 @@ CREATE TABLE `orders` ( ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`); ``` +πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter @@ -755,9 +790,44 @@ CREATE TABLE `addresses` ( ) ENGINE InnoDB, CHARSET utf8mb4, COLLATE utf8mb4_0900_ai_ci; + +ALTER TABLE `customers` ADD INDEX `idx_loyalty_tier`(`loyalty_tier`); +``` + +πŸ“Š **Table sizes**: +- `customers`: ~48.2M rows Β· ~23.4 GB across 2 shards (largest shard ~24.6M rows) + +πŸ“‹ **Plan**: **2** tables to create, **1** table to alter, **2** vschema updates + + +--- + +▢️ **To apply** all schema changes from this PR, comment: ``` +schemabot apply -e staging +``` + +
+ +
+Vitess Plan (Bytes-only Table Sizes) + + +## Schema Change Plan β€” Staging + +**Database**: `commerce` | **Type**: `Vitess` + +*Requested by @jackjackbits at 2026-01-01 00:00:00 UTC Β· planned from [`abcdef1`](https://github.com/block/schemabot/commit/abcdef1234567890abcdef1234567890abcdef12)* + +#### Keyspace: `commerce_sharded` +```sql +ALTER TABLE `addresses` ADD INDEX `idx_region`(`region`); +``` + +πŸ“Š **Table sizes**: +- `addresses`: ~48 GB across 4 shards -πŸ“‹ **Plan**: **2** tables to create, **2** vschema updates +πŸ“‹ **Plan**: **1** table to alter --- @@ -938,9 +1008,14 @@ CREATE TABLE `addresses` ( ) ENGINE InnoDB, CHARSET utf8mb4, COLLATE utf8mb4_0900_ai_ci; + +ALTER TABLE `customers` ADD INDEX `idx_loyalty_tier`(`loyalty_tier`); ``` -πŸ“‹ **Plan**: **2** tables to create, **2** vschema updates +πŸ“Š **Table sizes**: +- `customers`: ~48.2M rows Β· ~23.4 GB across 2 shards (largest shard ~24.6M rows) + +πŸ“‹ **Plan**: **2** tables to create, **1** table to alter, **2** vschema updates **Options**: ⏸️ Defer Cutover | ⏩ Skip Revert @@ -1036,6 +1111,9 @@ ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`);
+πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter --- @@ -1098,6 +1176,9 @@ ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`); +πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter --- @@ -1151,6 +1232,9 @@ ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`); +πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter ### Production @@ -1218,6 +1302,9 @@ ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`); - `users`: Column `created_at` uses `TIMESTAMP` which overflows on 2038-01-19. Consider using `DATETIME` instead. - `products`: Index `idx_category` on column `category` is redundant - covered by index `idx_category_price` on columns (`category`, `price`) +πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter --- @@ -1271,6 +1358,9 @@ CREATE TABLE `orders` ( ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`); ``` +πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter @@ -1324,6 +1414,9 @@ CREATE TABLE `orders` ( ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`); ``` +πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter @@ -1373,6 +1466,9 @@ CREATE TABLE `orders` ( ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`); ``` +πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter @@ -1627,6 +1723,9 @@ ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`); - `users`: Column `created_at` uses `TIMESTAMP` which overflows on 2038-01-19. Consider using `DATETIME` instead. - `products`: Index `idx_category` on column `category` is redundant - covered by index `idx_category_price` on columns (`category`, `price`) +πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter @@ -2343,6 +2442,9 @@ CREATE TABLE `orders` ( ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`); ``` +πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter @@ -2389,6 +2491,9 @@ CREATE TABLE `orders` ( ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`); ``` +πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter @@ -2437,6 +2542,9 @@ CREATE TABLE `orders` ( ALTER TABLE `products` ADD INDEX `idx_category_price`(`category`, `price`); ``` +πŸ“Š **Table sizes**: +- `products`: ~2.3M rows Β· ~1.1 GB + πŸ“‹ **Plan**: **2** tables to create, **1** table to alter @@ -2532,9 +2640,14 @@ CREATE TABLE `addresses` ( ) ENGINE InnoDB, CHARSET utf8mb4, COLLATE utf8mb4_0900_ai_ci; + +ALTER TABLE `customers` ADD INDEX `idx_loyalty_tier`(`loyalty_tier`); ``` -πŸ“‹ **Plan**: **2** tables to create, **2** vschema updates +πŸ“Š **Table sizes**: +- `customers`: ~48.2M rows Β· ~23.4 GB across 2 shards (largest shard ~24.6M rows) + +πŸ“‹ **Plan**: **2** tables to create, **1** table to alter, **2** vschema updates **Options**: ⏸️ Defer Cutover | ⏩ Skip Revert @@ -7385,6 +7498,9 @@ ALTER TABLE `mutes` ADD COLUMN `reason` varchar(255); ``` +πŸ“Š **Table sizes**: +- `mutes`: ~48.2M rows across 4 shards (largest shard ~13.1M rows) + πŸ“‹ **Plan**: **1** table to alter @@ -7454,6 +7570,9 @@ _Already applied β€” no change._ ALTER TABLE `mutes` ADD INDEX `created_at`(`created_at`); ``` +πŸ“Š **Table sizes**: +- `mutes`: size estimate unavailable Β· 3 shards + πŸ“‹ **Plan**: 1 DDL statement diff --git a/pkg/cmd/internal/templates/preview.go b/pkg/cmd/internal/templates/preview.go index 59cbe4e87..0b569e942 100644 --- a/pkg/cmd/internal/templates/preview.go +++ b/pkg/cmd/internal/templates/preview.go @@ -112,6 +112,7 @@ const ( // Comment template previews (GitHub PR comments) PreviewCommentPlan PreviewType = "comment_plan" // Plan comment with DDL changes + lint violations PreviewCommentPlanIgnoredNamespaces PreviewType = "comment_plan_ignored_namespaces" // Plan with namespaces withheld by ignore_namespaces + PreviewCommentPlanColumnOnlyAlter PreviewType = "comment_plan_column_only_alter" // Plan whose alter is metadata-only, so no table-size section renders PreviewCommentPlanBlocked PreviewType = "comment_plan_blocked" // Plan with a statement the engine refuses (blocked verdict) PreviewCommentPlanDirect PreviewType = "comment_plan_direct" // Locked plan with a statement routed to direct execution (direct verdict) PreviewCommentPlanCopyDiscarded PreviewType = "comment_plan_copy_discarded" // Plan whose apply would throw away an unfinished copy on the target @@ -132,6 +133,7 @@ const ( PreviewCommentMultiEnvDiff PreviewType = "comment_multi_env_diff" // Multi-env plan (different per env) PreviewCommentMultiEnvLint PreviewType = "comment_multi_env_lint" // Multi-env plan with lint violations PreviewCommentVitessPlan PreviewType = "comment_vitess_plan" // Vitess plan with keyspaces + VSchema + PreviewCommentVitessPlanBytesOnlySizes PreviewType = "comment_vitess_plan_bytes_only_sizes" // Vitess plan whose size context is storage bytes with no row counts PreviewCommentVitessPlanVSchemaRemoval PreviewType = "comment_vitess_plan_vschema_removal" // Vitess plan with unsafe VSchema removals PreviewCommentVitessApplyPlan PreviewType = "comment_vitess_apply_plan" // Locked Vitess apply-plan with options PreviewCommentMySQLMultiSchema PreviewType = "comment_mysql_multi_schema" // MySQL plan with multiple schema names diff --git a/pkg/cmd/internal/templates/preview_comment.go b/pkg/cmd/internal/templates/preview_comment.go index f76b823ea..dfab51e4a 100644 --- a/pkg/cmd/internal/templates/preview_comment.go +++ b/pkg/cmd/internal/templates/preview_comment.go @@ -40,6 +40,7 @@ func previewCommentAllOutput() { fn func() }{ {"PLAN COMMENT", func() { fmt.Print(webhooktemplates.PreviewCommentPlan()) }}, + {"PLAN COMMENT (COLUMN-ONLY ALTER, NO TABLE SIZES)", func() { fmt.Print(webhooktemplates.PreviewCommentPlanColumnOnlyAlter()) }}, {"PLAN COMMENT (IGNORED NAMESPACES)", func() { fmt.Print(webhooktemplates.PreviewCommentPlanIgnoredNamespaces()) }}, {"PLAN COMMENT (MANY LINT WARNINGS)", func() { fmt.Print(webhooktemplates.PreviewCommentPlanManyLintWarnings()) }}, {"PLAN COMMENT (ENGINE-BLOCKED CHANGE)", func() { fmt.Print(webhooktemplates.PreviewCommentPlanBlocked()) }}, @@ -144,6 +145,7 @@ func previewCommentPlanAllOutput() { fn func() }{ {"MYSQL PLAN", func() { fmt.Print(webhooktemplates.PreviewCommentPlan()) }}, + {"MYSQL PLAN (COLUMN-ONLY ALTER, NO TABLE SIZES)", func() { fmt.Print(webhooktemplates.PreviewCommentPlanColumnOnlyAlter()) }}, {"MYSQL PLAN (IGNORED NAMESPACES)", func() { fmt.Print(webhooktemplates.PreviewCommentPlanIgnoredNamespaces()) }}, {"MYSQL PLAN (MANY LINT WARNINGS)", func() { fmt.Print(webhooktemplates.PreviewCommentPlanManyLintWarnings()) }}, {"MYSQL PLAN (ENGINE-BLOCKED CHANGE)", func() { fmt.Print(webhooktemplates.PreviewCommentPlanBlocked()) }}, @@ -172,6 +174,7 @@ func previewCommentPlanAllOutput() { {"RECONCILIATION REQUIRED (IN PROGRESS)", func() { fmt.Print(webhooktemplates.PreviewCommentSchemaReconciliationInProgress()) }}, {"RECONCILIATION REQUIRED (COMPLETED)", func() { fmt.Print(webhooktemplates.PreviewCommentSchemaReconciliationCompleted()) }}, {"VITESS PLAN", func() { fmt.Print(webhooktemplates.PreviewCommentVitessPlan()) }}, + {"VITESS PLAN (BYTES-ONLY TABLE SIZES)", func() { fmt.Print(webhooktemplates.PreviewCommentVitessPlanBytesOnlySizes()) }}, {"VITESS PLAN: VSCHEMA REMOVAL (UNSAFE)", func() { fmt.Print(webhooktemplates.PreviewCommentVitessPlanVSchemaRemoval()) }}, {"POSTGRES PLAN", func() { fmt.Print(webhooktemplates.PreviewCommentPostgresPlan()) }}, {"SCHEMA CHANGE APPLY (LOCKED + OPTIONS)", func() { fmt.Print(webhooktemplates.PreviewCommentVitessApplyPlan()) }}, diff --git a/pkg/cmd/internal/templates/preview_dispatch.go b/pkg/cmd/internal/templates/preview_dispatch.go index 4ef52f689..f0abc4209 100644 --- a/pkg/cmd/internal/templates/preview_dispatch.go +++ b/pkg/cmd/internal/templates/preview_dispatch.go @@ -122,6 +122,8 @@ func PreviewCLIOutput(previewType PreviewType) { fmt.Print(webhooktemplates.PreviewCommentPlan()) case PreviewCommentPlanIgnoredNamespaces: fmt.Print(webhooktemplates.PreviewCommentPlanIgnoredNamespaces()) + case PreviewCommentPlanColumnOnlyAlter: + fmt.Print(webhooktemplates.PreviewCommentPlanColumnOnlyAlter()) case PreviewCommentPlanBlocked: fmt.Print(webhooktemplates.PreviewCommentPlanBlocked()) case PreviewCommentPlanDirect: @@ -162,6 +164,8 @@ func PreviewCLIOutput(previewType PreviewType) { fmt.Print(webhooktemplates.PreviewCommentMultiEnvPlanLint()) case PreviewCommentVitessPlan: fmt.Print(webhooktemplates.PreviewCommentVitessPlan()) + case PreviewCommentVitessPlanBytesOnlySizes: + fmt.Print(webhooktemplates.PreviewCommentVitessPlanBytesOnlySizes()) case PreviewCommentVitessPlanVSchemaRemoval: fmt.Print(webhooktemplates.PreviewCommentVitessPlanVSchemaRemoval()) case PreviewCommentVitessApplyPlan: diff --git a/pkg/webhook/plan.go b/pkg/webhook/plan.go index 3a1559fa4..b7d22b892 100644 --- a/pkg/webhook/plan.go +++ b/pkg/webhook/plan.go @@ -10,8 +10,10 @@ import ( "github.com/block/schemabot/pkg/api" "github.com/block/schemabot/pkg/apitypes" + "github.com/block/schemabot/pkg/ddl" ghclient "github.com/block/schemabot/pkg/github" "github.com/block/schemabot/pkg/metrics" + schemapkg "github.com/block/schemabot/pkg/schema" "github.com/block/schemabot/pkg/storage" "github.com/block/schemabot/pkg/ui" "github.com/block/schemabot/pkg/webhook/action" @@ -841,7 +843,30 @@ func splitExistingCopies(copies []*apitypes.ExistingCopyResponse) (discarded, ad return discarded, adopted, running } -// buildPlanCommentData converts plan results into template data. +// statementCostScalesWithSize reports whether a plan statement's execution +// cost grows with the table β€” an index build, a table copy or rebuild, or a +// full-table validation scan β€” using the real parser for the database's +// dialect. Table sizes are display-only context, so a statement that cannot +// be parsed logs a warning and renders without a size line rather than +// failing the comment. +func statementCostScalesWithSize(schemaResult *ghclient.SchemaRequestResult, stmt string) bool { + parser, err := ddl.ParserForDialect(schemapkg.DialectForDatabaseType(schemaResult.Type)) + if err != nil { + slog.Warn("no statement parser for dialect; plan comment omits the table-size line", + "repo", schemaResult.Repository, "database", schemaResult.Database, + "database_type", schemaResult.Type, "error", err) + return false + } + scales, err := parser.CostScalesWithTableSize(stmt) + if err != nil { + slog.Warn("failed to inspect plan statement for table-size-scaling cost; plan comment omits the table-size line", + "repo", schemaResult.Repository, "database", schemaResult.Database, + "database_type", schemaResult.Type, "error", err) + return false + } + return scales +} + func buildPlanCommentData(schema *ghclient.SchemaRequestResult, planResp *apitypes.PlanResponse, environment, tenant, requestedBy, agentHint string) templates.PlanCommentData { data := templates.PlanCommentData{ Database: schema.Database, @@ -899,6 +924,20 @@ func buildPlanCommentData(schema *ghclient.SchemaRequestResult, planResp *apityp } for _, t := range sc.TableChanges { ksData.Statements = append(ksData.Statements, t.DDL) + // Table sizes are shown only for statements whose cost scales + // with the table's size β€” index builds, copies/rebuilds, and + // validation scans. Metadata-only statements carrying a size line + // would be noise on the plan. + if !statementCostScalesWithSize(schema, t.DDL) { + continue + } + ksData.TableSizes = append(ksData.TableSizes, templates.TableSizeData{ + Table: t.TableName, + EstimatedRows: t.EstimatedRows, + ShardCount: t.ShardCount, + LargestShardRows: t.LargestShardRows, + EstimatedBytes: t.EstimatedBytes, + }) } // Extract VSchema changes from metadata if sc.HasVSchemaChange() { diff --git a/pkg/webhook/plan_integration_test.go b/pkg/webhook/plan_integration_test.go index 64546cf6b..a652e490b 100644 --- a/pkg/webhook/plan_integration_test.go +++ b/pkg/webhook/plan_integration_test.go @@ -19,6 +19,7 @@ import ( "testing" "time" + mysql "github.com/go-sql-driver/mysql" gh "github.com/google/go-github/v86/github" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -528,6 +529,101 @@ func TestE2EPlanConfigNotFound(t *testing.T) { } } +// A plan that adds an index to an existing table renders the table's +// approximate size in the plan comment β€” rows and bytes read from the target's +// statistics at plan time β€” so an operator sees the scale of the index build +// before applying. Only statements whose cost scales with the table carry a +// size line: metadata-only alters and tables being created are not listed. +func TestE2EPlanCommentShowsTableSizes(t *testing.T) { + dbName := "webhook_plan_table_sizes" + svc := setupE2EService(t, dbName) + dbConfig := svc.Config().Databases[dbName] + dbConfig.AllowedRepos = []string{"octocat/hello-world"} + dbConfig.AllowedDirs = []string{"schema"} + svc.Config().Databases[dbName] = dbConfig + + // Seed an existing table with enough rows that statistics report a + // meaningful estimate, and refresh statistics so the estimate is current. + dsnConfig, err := mysql.ParseDSN(e2eTargetDSN) + require.NoError(t, err) + dsnConfig.DBName = dbName + appDB, err := sql.Open("mysql", dsnConfig.FormatDSN()) + require.NoError(t, err) + t.Cleanup(func() { _ = appDB.Close() }) + _, err = appDB.ExecContext(t.Context(), "CREATE TABLE `items` (\n"+ + " `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n"+ + " `name` varchar(255) NOT NULL,\n"+ + " PRIMARY KEY (`id`)\n"+ + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci") + require.NoError(t, err) + _, err = appDB.ExecContext(t.Context(), "CREATE TABLE `notes` (\n"+ + " `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n"+ + " `body` varchar(255) NOT NULL,\n"+ + " PRIMARY KEY (`id`)\n"+ + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci") + require.NoError(t, err) + var values strings.Builder + for i := range 1200 { + if i > 0 { + values.WriteString(",") + } + fmt.Fprintf(&values, "('name-%d')", i) + } + _, err = appDB.ExecContext(t.Context(), "INSERT INTO `items` (`name`) VALUES "+values.String()) + require.NoError(t, err) + _, err = appDB.ExecContext(t.Context(), "ANALYZE TABLE `items`") + require.NoError(t, err) + + mux := http.NewServeMux() + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + + client := gh.NewClient(nil) + client.BaseURL, _ = url.Parse(server.URL + "/") + + schemabotConfig := fmt.Sprintf("database: %s\ntype: mysql\n", dbName) + schemaFiles := map[string]string{ + // One existing table gains an index (a size line), another gains only a + // column (no size line), and a new table is created (no size line). + "items.sql": "CREATE TABLE `items` (\n `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n `name` varchar(255) NOT NULL,\n PRIMARY KEY (`id`),\n KEY `idx_name` (`name`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci;", + "notes.sql": "CREATE TABLE `notes` (\n `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n `body` varchar(255) NOT NULL,\n `priority` int DEFAULT NULL,\n PRIMARY KEY (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci;", + "widgets.sql": "CREATE TABLE `widgets` (\n `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n PRIMARY KEY (`id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci;", + } + + result := setupFakeGitHubForPlan(t, mux, schemaFiles, schemabotConfig, dbName) + + logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) + installClient := ghclient.NewInstallationClient(client, logger) + factory := &fakeClientFactory{client: installClient} + + h := NewHandler(svc, factory, nil, logger) + + req := buildWebhookRequest(t, webhookPayloadOpts{ + comment: "schemabot plan -e staging", + isPR: true, + }, nil) + + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + require.Equal(t, http.StatusOK, rr.Code) + assert.Contains(t, rr.Body.String(), "plan generated successfully") + + select { + case body := <-result.comments: + assert.Contains(t, body, "## Schema Change Plan") + assert.Contains(t, body, "πŸ“Š **Table sizes**:") + assert.Contains(t, body, "- `items`: ~1.2k rows Β· ~", "the size line carries rows then a byte estimate") + assert.NotContains(t, body, "- `notes`:", "a column-only alter carries no size line") + assert.NotContains(t, body, "- `widgets`:", "a table being created is not listed in the size section") + sizesAt := strings.Index(body, "πŸ“Š **Table sizes**") + summaryAt := strings.Index(body, "πŸ“‹ **Plan**:") + assert.Less(t, sizesAt, summaryAt, "sizes render above the plan summary") + case <-time.After(10 * time.Second): + t.Fatal("timed out waiting for plan comment") + } +} + // TestE2EPlanCleansStalePlanOnlyChecksBeforeConvergingAggregates verifies the // check-refresh path of a plan command on a PR with no managed schema changes // when an earlier commit left behind a stale plan-only blocking check (for diff --git a/pkg/webhook/plan_test.go b/pkg/webhook/plan_test.go index 1b12f6da4..3644323d4 100644 --- a/pkg/webhook/plan_test.go +++ b/pkg/webhook/plan_test.go @@ -1046,3 +1046,44 @@ func TestRenderUnsafeChangesBlocked_PreservesTenantInRetryCommand(t *testing.T) assert.Contains(t, rendered, "**Tenant**: `alpha`") assert.Contains(t, rendered, "schemabot apply -e staging --tenant alpha --allow-unsafe") } + +// Per-table size estimates on the namespace-level plan changes are threaded +// into the keyspace's TableSizes for rendering, but only for statements whose +// cost scales with the table: an index add and a column widening carry size +// lines, while a metadata-only column add and a table being created (no size +// to report) are omitted. +func TestBuildPlanCommentData_TableSizes(t *testing.T) { + schema := &ghclient.SchemaRequestResult{Database: "cdb_resolute", Type: "strata"} + rows, largest, bytes := int64(48_200_000), int64(13_100_000), int64(23_400_000_000) + planResp := &apitypes.PlanResponse{ + Changes: []*apitypes.SchemaChangeResponse{{ + Namespace: "cdb_resolute_sharded", + TableChanges: []*apitypes.TableChangeResponse{ + {TableName: "widgets", DDL: "CREATE TABLE `widgets` (`id` bigint unsigned NOT NULL, PRIMARY KEY (`id`))", ChangeType: "create"}, + {TableName: "audits", DDL: "ALTER TABLE `audits` ADD COLUMN `reason` varchar(255)", ChangeType: "alter", + EstimatedRows: &rows, ShardCount: 4, LargestShardRows: &largest, EstimatedBytes: &bytes}, + {TableName: "mutes", DDL: "ALTER TABLE `mutes` ADD INDEX `created_at`(`created_at`)", ChangeType: "alter", + EstimatedRows: &rows, ShardCount: 4, LargestShardRows: &largest, EstimatedBytes: &bytes}, + {TableName: "outcomes", DDL: "ALTER TABLE `outcomes` MODIFY COLUMN `note` varchar(500)", ChangeType: "alter", + EstimatedRows: &rows, ShardCount: 4, LargestShardRows: &largest, EstimatedBytes: &bytes}, + }, + }}, + } + + data := buildPlanCommentData(schema, planResp, "staging", "", "testuser", "") + + require.Len(t, data.Changes, 1) + assert.Len(t, data.Changes[0].Statements, 4, "all statements still render as DDL") + require.Len(t, data.Changes[0].TableSizes, 2, + "the index add and the widening carry size lines; the created table and the metadata-only column add are omitted") + size := data.Changes[0].TableSizes[0] + assert.Equal(t, "mutes", size.Table) + require.NotNil(t, size.EstimatedRows) + assert.Equal(t, rows, *size.EstimatedRows) + assert.Equal(t, 4, size.ShardCount) + require.NotNil(t, size.LargestShardRows) + assert.Equal(t, largest, *size.LargestShardRows) + require.NotNil(t, size.EstimatedBytes) + assert.Equal(t, bytes, *size.EstimatedBytes) + assert.Equal(t, "outcomes", data.Changes[0].TableSizes[1].Table) +} diff --git a/pkg/webhook/templates/plan.go b/pkg/webhook/templates/plan.go index 73fa80308..e1771dbd5 100644 --- a/pkg/webhook/templates/plan.go +++ b/pkg/webhook/templates/plan.go @@ -226,6 +226,13 @@ type KeyspaceChangeData struct { VSchemaChanged bool VSchemaDiff string + // TableSizes carries plan-time size estimates for the existing tables this + // keyspace's changes touch, rendered above the DDL so an operator sees the + // scale of each table before reading the statements. Tables being created + // have no size and are omitted; an entry without a row estimate renders an + // explicit "unavailable" so absence is never silent. + TableSizes []TableSizeData + // Shards carries this keyspace's per-shard changes for a sharded plan. When // set, the DDL is rendered per shard-group ("what applies where") instead of // the single Statements block β€” so a keyspace whose shards diverge is shown @@ -233,6 +240,28 @@ type KeyspaceChangeData struct { Shards []KeyspaceShardChange } +// TableSizeData is one existing table's plan-time size estimate for display. +// Row values are approximate β€” sourced from engine statistics that may be +// stale β€” and are rendered as such. +type TableSizeData struct { + Table string + // EstimatedRows is the table's approximate row count, summed across shards + // for a sharded target. Nil renders as explicitly unavailable. + EstimatedRows *int64 + // ShardCount is the number of shards the change spans. Zero when the + // target is not sharded or the topology is unknown, which omits the shard + // clause entirely. + ShardCount int + // LargestShardRows is the approximate row count of the largest single + // shard β€” the write-blocking blast radius of a shard-at-a-time apply. + // Rendered only when the change spans more than one shard. + LargestShardRows *int64 + // EstimatedBytes is the table's approximate on-disk footprint (data plus + // indexes), summed across shards for a sharded target. Nil omits the byte + // clause. + EstimatedBytes *int64 +} + // KeyspaceShardChange is one shard's planned statements within a keyspace. type KeyspaceShardChange struct { Shard string @@ -547,6 +576,10 @@ func writePlanSummary(sb *strings.Builder, data PlanCommentData, totalStatements return } + // Size context precedes the summary: how big the tables the plan will + // copy, rebuild, or scan are, then what the plan does. + writeTableSizesSection(sb, data) + // Count statement types (Terraform-style: X to create, Y to alter, Z to drop) creates, alters, drops := countStatementTypes(data.Changes, data.DatabaseType) @@ -799,6 +832,68 @@ func writeKeyspaceChanges(sb *strings.Builder, data PlanCommentData) { } } +// writeTableSizesSection renders the plan's table-size info section: one line +// per table the plan will copy, rebuild, or scan (the comment builder +// attaches sizes only to statements whose cost scales with table size), +// across every keyspace, placed above the plan summary. A plan of only +// metadata-only statements renders no section at all. Table names carry +// their keyspace when the plan spans more than one keyspace with sizes, so a +// shared table name stays unambiguous. +func writeTableSizesSection(sb *strings.Builder, data PlanCommentData) { + keyspacesWithSizes := 0 + for _, ks := range data.Changes { + if len(ks.TableSizes) > 0 { + keyspacesWithSizes++ + } + } + if keyspacesWithSizes == 0 { + return + } + qualify := keyspacesWithSizes > 1 + sb.WriteString("πŸ“Š **Table sizes**:\n") + for _, ks := range data.Changes { + for _, ts := range ks.TableSizes { + name := ts.Table + if qualify { + name = ks.Keyspace + "." + ts.Table + } + fmt.Fprintf(sb, "- `%s`: %s\n", name, formatTableSize(ts)) + } + } + sb.WriteString("\n") +} + +// formatTableSize renders one table's size clause: the estimated rows and +// bytes, the shard span for sharded targets, and the largest single shard when +// the change spans several. An engine that reports only one of the two +// estimates (PlanetScale's metrics report bytes alone) renders the one it has. +// A table with neither is stated explicitly β€” operators must never mistake a +// failed size probe for a small table. +func formatTableSize(ts TableSizeData) string { + if ts.EstimatedRows == nil && ts.EstimatedBytes == nil { + if ts.ShardCount > 0 { + return fmt.Sprintf("size estimate unavailable Β· %d %s", ts.ShardCount, pluralize("shard", ts.ShardCount)) + } + return "size estimate unavailable" + } + var size string + switch { + case ts.EstimatedBytes == nil: + size = ui.FormatApproxRows(*ts.EstimatedRows) + " rows" + case ts.EstimatedRows == nil: + size = ui.FormatApproxBytes(*ts.EstimatedBytes) + default: + size = ui.FormatApproxRows(*ts.EstimatedRows) + " rows Β· " + ui.FormatApproxBytes(*ts.EstimatedBytes) + } + if ts.ShardCount > 0 { + size += fmt.Sprintf(" across %d %s", ts.ShardCount, pluralize("shard", ts.ShardCount)) + if ts.ShardCount > 1 && ts.LargestShardRows != nil { + size += fmt.Sprintf(" (largest shard %s rows)", ui.FormatApproxRows(*ts.LargestShardRows)) + } + } + return size +} + // writePlanDDLBlock writes a single fenced SQL block of statements, formatted // under the plan's own dialect. func writePlanDDLBlock(sb *strings.Builder, statements []string, dialect schema.Dialect) { diff --git a/pkg/webhook/templates/preview.go b/pkg/webhook/templates/preview.go index b22f774b0..71bdc77ab 100644 --- a/pkg/webhook/templates/preview.go +++ b/pkg/webhook/templates/preview.go @@ -53,6 +53,9 @@ func previewPlanData() PlanCommentData { "CREATE TABLE `orders` (\n `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n `user_id` bigint NOT NULL,\n `total_cents` bigint NOT NULL,\n `status` varchar(50) NOT NULL DEFAULT 'pending',\n PRIMARY KEY (`id`),\n INDEX `idx_user_id` (`user_id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci;", "ALTER TABLE `products` ADD INDEX `idx_category_price` (`category`, `price`);", }, + TableSizes: []TableSizeData{ + {Table: "products", EstimatedRows: previewRows(2_340_000), EstimatedBytes: previewRows(1_130_000_000)}, + }, }, }, LintViolations: sampleLintWarnings(), @@ -69,6 +72,37 @@ func PreviewCommentPlanIgnoredNamespaces() string { return RenderPlanComment(data) } +// previewRows returns a pointer to a sample row-count estimate. +// +//go:fix inline +func previewRows(n int64) *int64 { + return new(n) +} + +// PreviewCommentPlanColumnOnlyAlter renders a sample plan whose only alter +// adds a plain column β€” a metadata-only change whose cost doesn't scale with +// the table β€” so the comment carries no table-size section. +func PreviewCommentPlanColumnOnlyAlter() string { + return RenderPlanComment(PlanCommentData{ + Database: "testapp", + SchemaName: "testapp", + Environment: "staging", + HeadSHA: previewHeadSHA, + Repository: previewRepository, + RequestedBy: previewRequestedBy, + IsMySQL: true, + DatabaseType: "mysql", + Changes: []KeyspaceChangeData{ + { + Keyspace: "testapp", + Statements: []string{ + "ALTER TABLE `products` ADD COLUMN `discount_cents` bigint DEFAULT NULL;", + }, + }, + }, + }) +} + // PreviewCommentPlanBlocked renders a sample plan containing a statement the // engine deterministically refuses (execution-mode verdict "blocked"). func PreviewCommentPlanBlocked() string { @@ -928,6 +962,9 @@ func samplePlanChanges() []KeyspaceChangeData { "CREATE TABLE `orders` (\n `id` bigint unsigned NOT NULL AUTO_INCREMENT,\n `user_id` bigint NOT NULL,\n `total_cents` bigint NOT NULL,\n `status` varchar(50) NOT NULL DEFAULT 'pending',\n PRIMARY KEY (`id`),\n INDEX `idx_user_id` (`user_id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci;", "ALTER TABLE `products` ADD INDEX `idx_category_price` (`category`, `price`);", }, + TableSizes: []TableSizeData{ + {Table: "products", EstimatedRows: previewRows(2_340_000), EstimatedBytes: previewRows(1_130_000_000)}, + }, }, } } @@ -1214,6 +1251,10 @@ func sampleVitessPlanChanges() []KeyspaceChangeData { Keyspace: "commerce_sharded", Statements: []string{ "CREATE TABLE `addresses` (\n `id` bigint unsigned NOT NULL,\n `customer_id` bigint unsigned NOT NULL,\n `street` varchar(255) NOT NULL,\n `city` varchar(100) NOT NULL,\n PRIMARY KEY (`id`),\n INDEX `idx_customer_id` (`customer_id`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci;", + "ALTER TABLE `customers` ADD INDEX `idx_loyalty_tier` (`loyalty_tier`);", + }, + TableSizes: []TableSizeData{ + {Table: "customers", EstimatedRows: previewRows(48_200_000), EstimatedBytes: previewRows(23_400_000_000), ShardCount: 2, LargestShardRows: previewRows(24_600_000)}, }, VSchemaChanged: true, VSchemaDiff: `--- a/commerce_sharded.json @@ -1278,6 +1319,34 @@ func PreviewCommentVitessPlan() string { }) } +// PreviewCommentVitessPlanBytesOnlySizes renders a sample Vitess plan comment +// whose size context carries storage bytes with no row counts β€” the shape a +// PlanetScale target produces, where the branch table metrics report each +// table's bytes but nothing counts rows. +func PreviewCommentVitessPlanBytesOnlySizes() string { + return RenderPlanComment(PlanCommentData{ + Database: "commerce", + SchemaName: "commerce", + Environment: "staging", + HeadSHA: previewHeadSHA, + Repository: previewRepository, + RequestedBy: previewRequestedBy, + IsMySQL: false, + DatabaseType: "vitess", + Changes: []KeyspaceChangeData{ + { + Keyspace: "commerce_sharded", + Statements: []string{ + "ALTER TABLE `addresses` ADD INDEX `idx_region` (`region`);", + }, + TableSizes: []TableSizeData{ + {Table: "addresses", EstimatedBytes: previewRows(48_000_000_000), ShardCount: 4}, + }, + }, + }, + }) +} + // PreviewCommentVitessPlanVSchemaRemoval renders a sample Vitess plan comment // where the VSchema change removes a lookup vindex and its column-vindex // association β€” the removals surface in the Issues section as unsafe changes. diff --git a/pkg/webhook/templates/preview_sharded.go b/pkg/webhook/templates/preview_sharded.go index 6f65eb06b..68f20cd72 100644 --- a/pkg/webhook/templates/preview_sharded.go +++ b/pkg/webhook/templates/preview_sharded.go @@ -320,6 +320,9 @@ func PreviewCommentShardedPlanDivergent() string { Changes: []KeyspaceChangeData{{ Keyspace: "cdb_resolute_sharded", Statements: []string{idx}, + TableSizes: []TableSizeData{ + {Table: "mutes", EstimatedRows: previewRows(48_200_000), ShardCount: 4, LargestShardRows: previewRows(13_100_000)}, + }, Shards: []KeyspaceShardChange{ {Shard: "-40", Statements: []string{idx}}, {Shard: "80-c0", Statements: []string{idx}}, @@ -377,6 +380,12 @@ func PreviewCommentShardedPlanPartiallyApplied() string { HeadSHA: previewHeadSHA, Repository: previewRepository, RequestedBy: previewRequestedBy, Changes: []KeyspaceChangeData{{ Keyspace: "cdb_resolute_sharded", + // A nil row estimate renders as explicitly unavailable β€” the size + // probe failing must never read as "small table". The satisfied + // shard needs no change, so the change spans three shards. + TableSizes: []TableSizeData{ + {Table: "mutes", ShardCount: 3}, + }, Shards: []KeyspaceShardChange{ {Shard: "-40", Satisfied: true}, {Shard: "40-80", Statements: []string{idx}}, diff --git a/pkg/webhook/templates/table_sizes_test.go b/pkg/webhook/templates/table_sizes_test.go new file mode 100644 index 000000000..f74083d12 --- /dev/null +++ b/pkg/webhook/templates/table_sizes_test.go @@ -0,0 +1,131 @@ +package templates + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +// Table-size rendering in the plan comment: an info section above the plan +// summary shows the scale of each table gaining an index β€” rows, on-disk +// bytes, the shard span, and the largest single shard. A missing estimate is +// stated explicitly so a failed size probe never reads as a small table. + +func tableSizePlanData(sizes []TableSizeData) PlanCommentData { + return PlanCommentData{ + Database: "testapp", + Environment: "staging", + IsMySQL: true, + Changes: []KeyspaceChangeData{{ + Keyspace: "testapp", + Statements: []string{"ALTER TABLE `mutes` ADD INDEX `created_at`(`created_at`)"}, + TableSizes: sizes, + }}, + } +} + +func TestRenderPlanComment_TableSizes(t *testing.T) { + out := RenderPlanComment(tableSizePlanData([]TableSizeData{ + {Table: "mutes", EstimatedRows: previewRows(2_340_000), EstimatedBytes: previewRows(1_130_000_000)}, + })) + + assert.Contains(t, out, "πŸ“Š **Table sizes**:") + // The line ends right after the byte clause: a non-sharded target renders + // no shard clause. + assert.Contains(t, out, "- `mutes`: ~2.3M rows Β· ~1.1 GB\n") +} + +func TestRenderPlanComment_TableSizesRenderAbovePlanSummary(t *testing.T) { + data := tableSizePlanData([]TableSizeData{ + {Table: "mutes", EstimatedRows: previewRows(2_340_000), EstimatedBytes: previewRows(1_130_000_000)}, + }) + data.LintViolations = []LintViolationData{ + {Message: "Index should be invisible first", Table: "mutes", LinterName: "invisible_index_before_drop"}, + } + out := RenderPlanComment(data) + + summaryAt := strings.Index(out, "πŸ“‹ **Plan**:") + sizesAt := strings.Index(out, "πŸ“Š **Table sizes**") + lintAt := strings.Index(out, "πŸ’‘ **Lint Warnings**") + ddlAt := strings.Index(out, "```sql") + assert.Greater(t, sizesAt, ddlAt, "sizes render after the DDL, not above it") + assert.Greater(t, sizesAt, lintAt, "sizes render below the lint warnings") + assert.Less(t, sizesAt, summaryAt, "sizes render above the plan summary") +} + +func TestRenderPlanComment_TableSizesWithoutBytesOmitsByteClause(t *testing.T) { + out := RenderPlanComment(tableSizePlanData([]TableSizeData{ + {Table: "mutes", EstimatedRows: previewRows(2_340_000)}, + })) + + assert.Contains(t, out, "- `mutes`: ~2.3M rows\n") +} + +// PlanetScale's branch metrics report storage bytes with no row counts, so a +// PlanetScale change renders its byte estimate and shard span rather than +// falling to the size-unavailable line. +func TestRenderPlanComment_TableSizesBytesOnly(t *testing.T) { + out := RenderPlanComment(tableSizePlanData([]TableSizeData{ + {Table: "mutes", EstimatedBytes: previewRows(23_400_000_000), ShardCount: 4}, + })) + + assert.Contains(t, out, "- `mutes`: ~23.4 GB across 4 shards\n") +} + +func TestRenderPlanComment_TableSizesSharded(t *testing.T) { + out := RenderPlanComment(tableSizePlanData([]TableSizeData{ + {Table: "mutes", EstimatedRows: previewRows(48_200_000), EstimatedBytes: previewRows(23_400_000_000), ShardCount: 4, LargestShardRows: previewRows(13_100_000)}, + })) + + assert.Contains(t, out, "- `mutes`: ~48.2M rows Β· ~23.4 GB across 4 shards (largest shard ~13.1M rows)\n") +} + +func TestRenderPlanComment_TableSizesSingleShardOmitsLargest(t *testing.T) { + out := RenderPlanComment(tableSizePlanData([]TableSizeData{ + {Table: "mutes", EstimatedRows: previewRows(15_249), ShardCount: 1, LargestShardRows: previewRows(15_249)}, + })) + + assert.Contains(t, out, "- `mutes`: ~15.2k rows across 1 shard\n") + assert.NotContains(t, out, "largest shard", "a single-shard span has no distinct largest shard") +} + +func TestRenderPlanComment_TableSizeUnavailableIsExplicit(t *testing.T) { + out := RenderPlanComment(tableSizePlanData([]TableSizeData{ + {Table: "mutes", ShardCount: 3}, + {Table: "orders"}, + })) + + assert.Contains(t, out, "- `mutes`: size estimate unavailable Β· 3 shards\n") + assert.Contains(t, out, "- `orders`: size estimate unavailable\n") +} + +func TestRenderPlanComment_TableSizesQualifiedAcrossKeyspaces(t *testing.T) { + data := PlanCommentData{ + Database: "testapp", + Environment: "staging", + DatabaseType: "vitess", + Changes: []KeyspaceChangeData{ + { + Keyspace: "commerce", + Statements: []string{"ALTER TABLE `mutes` ADD INDEX `created_at`(`created_at`)"}, + TableSizes: []TableSizeData{{Table: "mutes", EstimatedRows: previewRows(2_340_000), EstimatedBytes: previewRows(1_130_000_000)}}, + }, + { + Keyspace: "commerce_sharded", + Statements: []string{"ALTER TABLE `customers` ADD COLUMN `tier` varchar(20)"}, + TableSizes: []TableSizeData{{Table: "customers", EstimatedRows: previewRows(48_200_000), EstimatedBytes: previewRows(23_400_000_000), ShardCount: 2, LargestShardRows: previewRows(24_600_000)}}, + }, + }, + } + out := RenderPlanComment(data) + + assert.Contains(t, out, "- `commerce.mutes`: ~2.3M rows Β· ~1.1 GB\n") + assert.Contains(t, out, "- `commerce_sharded.customers`: ~48.2M rows Β· ~23.4 GB across 2 shards (largest shard ~24.6M rows)\n") +} + +func TestRenderPlanComment_NoTableSizesOmitsSection(t *testing.T) { + out := RenderPlanComment(tableSizePlanData(nil)) + + assert.False(t, strings.Contains(out, "Table sizes"), "a plan without size data renders no size section") +}