From 34687b76f712b7cef625799a45225e31b8cf78bb Mon Sep 17 00:00:00 2001 From: h3n4l Date: Wed, 17 Jun 2026 11:30:07 +0800 Subject: [PATCH] refactor(pg/catalog): use explicit pretty bool for view-def getters GetViewDefinition / GetMatViewDefinition took `pretty ...bool` solely to make the flag optional (Go has no default arguments). But every caller relied on the default and none ever passed a value, and the variadic silently accepts more than one bool with no compile-time arity check. Switch both to an explicit `pretty bool` and update all call sites to pass `false`. No behavior change; the signature is now precise. Co-Authored-By: Claude Opus 4.8 (1M context) --- pg/catalog/diff_view.go | 4 ++-- pg/catalog/migration.go | 2 +- pg/catalog/migration_view.go | 10 +++++----- pg/catalog/phase4_analyze_test.go | 2 +- pg/catalog/phase5_types_test.go | 2 +- pg/catalog/ruleutils.go | 16 ++++------------ 6 files changed, 14 insertions(+), 22 deletions(-) diff --git a/pg/catalog/diff_view.go b/pg/catalog/diff_view.go index 84e27255..b2d11c55 100644 --- a/pg/catalog/diff_view.go +++ b/pg/catalog/diff_view.go @@ -132,9 +132,9 @@ func getViewDef(c *Catalog, key relKey, rel *Relation) string { var def string var err error if rel.RelKind == 'm' { - def, err = c.GetMatViewDefinition(key.schema, key.name) + def, err = c.GetMatViewDefinition(key.schema, key.name, false) } else { - def, err = c.GetViewDefinition(key.schema, key.name) + def, err = c.GetViewDefinition(key.schema, key.name, false) } if err != nil { return "" diff --git a/pg/catalog/migration.go b/pg/catalog/migration.go index ea1157fd..e78a5e09 100644 --- a/pg/catalog/migration.go +++ b/pg/catalog/migration.go @@ -372,7 +372,7 @@ func wrapColumnTypeChangesWithViewOps(from, to *Catalog, diff *SchemaDiff, ops [ if toRel != nil { toOID = toRel.OID } - def, _ := to.GetViewDefinition(v.schema, v.name) + def, _ := to.GetViewDefinition(v.schema, v.name, false) extraOps = append(extraOps, MigrationOp{ Type: OpCreateView, SchemaName: v.schema, diff --git a/pg/catalog/migration_view.go b/pg/catalog/migration_view.go index 8a943b27..7e283e05 100644 --- a/pg/catalog/migration_view.go +++ b/pg/catalog/migration_view.go @@ -139,7 +139,7 @@ func generateViewDDL(from, to *Catalog, diff *SchemaDiff) []MigrationOp { // buildCreateViewOp creates a CREATE VIEW op for an added view. func buildCreateViewOp(to *Catalog, entry RelationDiffEntry) MigrationOp { qn := migrationQualifiedName(entry.SchemaName, entry.Name) - def, _ := to.GetViewDefinition(entry.SchemaName, entry.Name) + def, _ := to.GetViewDefinition(entry.SchemaName, entry.Name, false) var b strings.Builder b.WriteString("CREATE VIEW ") @@ -171,7 +171,7 @@ func buildCreateViewOp(to *Catalog, entry RelationDiffEntry) MigrationOp { // buildCreateMatViewOp creates a CREATE MATERIALIZED VIEW op. func buildCreateMatViewOp(to *Catalog, entry RelationDiffEntry) MigrationOp { qn := migrationQualifiedName(entry.SchemaName, entry.Name) - def, _ := to.GetMatViewDefinition(entry.SchemaName, entry.Name) + def, _ := to.GetMatViewDefinition(entry.SchemaName, entry.Name, false) sql := fmt.Sprintf("CREATE MATERIALIZED VIEW %s AS %s", qn, strings.TrimRight(def, " \t\n\r;")) @@ -218,7 +218,7 @@ func buildModifyViewOps(from, to *Catalog, entry RelationDiffEntry) []MigrationO } qn := migrationQualifiedName(entry.SchemaName, entry.Name) - def, _ := to.GetViewDefinition(entry.SchemaName, entry.Name) + def, _ := to.GetViewDefinition(entry.SchemaName, entry.Name, false) var b strings.Builder b.WriteString("CREATE OR REPLACE VIEW ") @@ -252,7 +252,7 @@ func buildModifyViewOps(from, to *Catalog, entry RelationDiffEntry) []MigrationO // Materialized views don't support CREATE OR REPLACE. func buildModifyMatViewOps(to *Catalog, entry RelationDiffEntry) []MigrationOp { qn := migrationQualifiedName(entry.SchemaName, entry.Name) - def, _ := to.GetMatViewDefinition(entry.SchemaName, entry.Name) + def, _ := to.GetMatViewDefinition(entry.SchemaName, entry.Name, false) dropOp := MigrationOp{ Type: OpDropView, @@ -306,7 +306,7 @@ func findDependentViews(c *Catalog, schemaName, viewName string) []viewRef { continue } // Check if this view references the target view. - def, err := c.GetViewDefinition(s.Name, rel.Name) + def, err := c.GetViewDefinition(s.Name, rel.Name, false) if err != nil { continue } diff --git a/pg/catalog/phase4_analyze_test.go b/pg/catalog/phase4_analyze_test.go index ffb1b5af..5f6eca3f 100644 --- a/pg/catalog/phase4_analyze_test.go +++ b/pg/catalog/phase4_analyze_test.go @@ -48,7 +48,7 @@ func viewDef(t *testing.T, c *Catalog, sql string) string { item = raw.Stmt } if vs, ok := item.(*nodes.ViewStmt); ok { - def, err := c.GetViewDefinition("", vs.View.Relname) + def, err := c.GetViewDefinition("", vs.View.Relname, false) if err != nil { t.Fatalf("get view def: %v", err) } diff --git a/pg/catalog/phase5_types_test.go b/pg/catalog/phase5_types_test.go index 537916a3..7b75b182 100644 --- a/pg/catalog/phase5_types_test.go +++ b/pg/catalog/phase5_types_test.go @@ -354,7 +354,7 @@ func TestPhase5_ExistingRangeStillWorks(t *testing.T) { func assertViewDefContains(t *testing.T, c *Catalog, viewName, expected string) { t.Helper() - def, err := c.GetViewDefinition("", viewName) + def, err := c.GetViewDefinition("", viewName, false) if err != nil { t.Fatalf("get view def: %v", err) } diff --git a/pg/catalog/ruleutils.go b/pg/catalog/ruleutils.go index 1f635c93..e02860c8 100644 --- a/pg/catalog/ruleutils.go +++ b/pg/catalog/ruleutils.go @@ -12,7 +12,7 @@ import ( // When pretty is true, matches pg_get_viewdef(oid, true) (PRETTYFLAG_INDENT | PRETTYFLAG_PAREN). // // pg: src/backend/utils/adt/ruleutils.c — pg_get_viewdef_worker -func (c *Catalog) GetViewDefinition(schema, name string, pretty ...bool) (string, error) { +func (c *Catalog) GetViewDefinition(schema, name string, pretty bool) (string, error) { rel := c.GetRelation(schema, name) if rel == nil { return "", errUndefinedTable(name) @@ -20,11 +20,7 @@ func (c *Catalog) GetViewDefinition(schema, name string, pretty ...bool) (string if rel.RelKind != 'v' { return "", errWrongObjectType(name, "a view") } - p := false - if len(pretty) > 0 { - p = pretty[0] - } - return c.deparseRelationQuery(rel, p) + return c.deparseRelationQuery(rel, pretty) } // GetMatViewDefinition returns the SQL definition of a materialized view, @@ -34,7 +30,7 @@ func (c *Catalog) GetViewDefinition(schema, name string, pretty ...bool) (string // When pretty is true, matches pg_get_viewdef(oid, true) (PRETTYFLAG_INDENT | PRETTYFLAG_PAREN). // // pg: src/backend/utils/adt/ruleutils.c — pg_get_viewdef_worker -func (c *Catalog) GetMatViewDefinition(schema, name string, pretty ...bool) (string, error) { +func (c *Catalog) GetMatViewDefinition(schema, name string, pretty bool) (string, error) { rel := c.GetRelation(schema, name) if rel == nil { return "", errUndefinedTable(name) @@ -42,11 +38,7 @@ func (c *Catalog) GetMatViewDefinition(schema, name string, pretty ...bool) (str if rel.RelKind != 'm' { return "", errWrongObjectType(name, "a materialized view") } - p := false - if len(pretty) > 0 { - p = pretty[0] - } - return c.deparseRelationQuery(rel, p) + return c.deparseRelationQuery(rel, pretty) } // deparseRelationQuery deparses the AnalyzedQuery of a view or matview.