diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ee743656..7a54bddd 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,6 +6,14 @@ on: pull_request: branches: [ main ] +permissions: + # Least-privilege default, as in release.yaml. This workflow only checks + # out the tree and runs tests, so it never needs to write. That matters + # more here than elsewhere: it runs on pull_request, so the test code it + # executes comes from the pull request itself, and a token that can write + # would be handed to code the repository has not merged yet. + contents: read + jobs: test: runs-on: ubuntu-latest @@ -46,6 +54,9 @@ jobs: - name: Run query, diff, and repair unit tests run: go test -count=1 -v ./db/queries ./internal/consistency/diff ./internal/consistency/repair + - name: Run schema-structure comparison unit tests + run: go test -count=1 -v ./internal/consistency/schema + - name: Run mtree missing-tree fail-fast test run: go test -count=1 -v ./tests/integration -run 'TestMtreeDiffFailsFastWhenTreeNotBuilt' @@ -71,6 +82,15 @@ jobs: - name: Run schema-diff tests run: go test -count=1 -v ./tests/integration -run 'TestSchemaDiff_' + - name: Run schema-diff --compare=structure tests + run: go test -count=1 -v ./tests/integration -run 'TestSchemaDiffStructure_' + + - name: Run scope resolution tests + run: go test -count=1 -v ./tests/integration -run 'TestSchemaProvider_' + + - name: Run table-diff schema-mismatch diagnosis tests + run: go test -count=1 -v ./tests/integration -run 'TestTableDiffSchemaMismatch_' + - name: Run spock-diff comparison unit tests run: go test -count=1 -v ./internal/consistency/diff -run 'TestCompareSubscriptions' diff --git a/cmd/ace/main.go b/cmd/ace/main.go index bad9eaaf..ecf1a368 100644 --- a/cmd/ace/main.go +++ b/cmd/ace/main.go @@ -13,6 +13,7 @@ package main import ( "context" + "errors" "os" "path/filepath" "strings" @@ -69,6 +70,15 @@ func main() { err := app.Run(context.Background(), os.Args) if err != nil { logger.Error("%v", err) + + // Default exit code is 1. A command can carry a more specific + // code by wrapping its error in pkg/common.ExitCodeError. + code := 1 + var withCode interface{ ExitCode() int } + if errors.As(err, &withCode) { + code = withCode.ExitCode() + } + os.Exit(code) } } diff --git a/db/queries/queries.go b/db/queries/queries.go index bad88eda..4cae0e0a 100644 --- a/db/queries/queries.go +++ b/db/queries/queries.go @@ -671,11 +671,9 @@ func BlockHashSQL(schema, table string, primaryKeyCols []string, mode string, in endPlaceholders[i] = fmt.Sprintf("$%d", paramIndex) paramIndex++ } - // Upper bound is always EXCLUSIVE: a block's range_end is the next - // block's range_start (from LEAD in the build offsets and from split - // points), so a closed "<=" would hash boundary rows into two adjacent - // leaves. XOR parent hashing then cancels the duplicate siblings, letting - // divergent data produce matching root hashes and hiding real conflicts. + // Upper bound is exclusive: range_end is the next block's range_start + // (from LEAD in the build offsets and from split points), so each row + // belongs to exactly one leaf. operator := "<" var upperExpr string if len(primaryKeyCols) == 1 { @@ -821,7 +819,7 @@ func GetColumnTypes(ctx context.Context, db DBQuerier, schema, table string) (ma return types, nil } -// TODO: Need to add Spock privilege checks!! +// TODO: add Spock privilege checks. func CheckUserPrivileges(ctx context.Context, db DBQuerier, username, schema, table string) (*types.UserPrivileges, error) { sql, err := RenderSQL(SQLTemplates.CheckUserPrivileges, nil) if err != nil { @@ -1344,6 +1342,558 @@ func GetPkeyColumnTypes(ctx context.Context, db DBQuerier, schema, table string, return types, nil } +// ColumnDescriptor is one column's structural properties for schema +// structure comparison. It excludes attnum: column order is not part of +// structural identity, and callers must not rely on row order for anything +// but display. +type ColumnDescriptor struct { + Table string + Name string + TypeOID uint32 // this node's own type OID; local-only lookup key for domain/range/composite/enum descriptors, see GetColumnDescriptors + TypeMod int32 + TypeText string + // TypeNamespace/TypeName/TypeKind are the portable identity of the + // column's type (namespace.typname plus typtype); structural comparison + // keys on these fields. + TypeNamespace string + TypeName string + TypeKind string // 'b' base | 'd' domain | 'e' enum | 'r' range | 'c' composite + NotNull bool + Identity string // '' | 'a' (always) | 'd' (by default) + Generated string // '' | 's' (stored) + Options string // attoptions, sorted and comma-joined so apply order cannot differ; opaque to this layer + CollNamespace string + CollName string + CollProvider string + // CollVersion is the version the catalog recorded for this collation, + // not the version of the collation library in use now; see + // GetColumnDescriptors' SQL and GetDatabaseLocale. + CollVersion string + DefaultExpr string +} + +// GetColumnDescriptors reads the structural properties of every live column +// of the given tables in one round trip. See the GetColumnDescriptors SQL +// template for exactly which catalog fields are read and why. +func GetColumnDescriptors(ctx context.Context, db DBQuerier, schema string, tables []string) (map[string][]ColumnDescriptor, error) { + sql, err := RenderSQL(SQLTemplates.GetColumnDescriptors, nil) + if err != nil { + return nil, fmt.Errorf("failed to render GetColumnDescriptors SQL: %w", err) + } + + rows, err := db.Query(ctx, sql, schema, tables) + if err != nil { + return nil, fmt.Errorf("query to get column descriptors for schema %q failed: %w", schema, err) + } + defer rows.Close() + + result := make(map[string][]ColumnDescriptor) + for rows.Next() { + var c ColumnDescriptor + if err := rows.Scan(&c.Table, &c.Name, &c.TypeOID, &c.TypeMod, &c.TypeText, + &c.TypeNamespace, &c.TypeName, &c.TypeKind, + &c.NotNull, &c.Identity, &c.Generated, &c.Options, + &c.CollNamespace, &c.CollName, &c.CollProvider, &c.CollVersion, + &c.DefaultExpr); err != nil { + return nil, fmt.Errorf("failed to scan column descriptor: %w", err) + } + result[c.Table] = append(result[c.Table], c) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating over column descriptors: %w", err) + } + return result, nil +} + +// ReplicaIdentityDescriptor is the table's row-identity mode together with +// the key the mode resolves to. KeyColumns/KeyOpclasses describe the primary +// key when ReplicaIdentity is "d" and the designated index when it is "i"; +// both are empty for "f" (the whole row is the identity) and "n" (there is +// none), which ReplicaIdentity itself already says. +type ReplicaIdentityDescriptor struct { + Table string + ReplicaIdentity string // "d" default | "n" nothing | "f" full | "i" specific index + KeyColumns []string + KeyOpclasses []string + // KeyLength is the designated index's indnkeyatts. It must equal + // len(KeyColumns): the query joins each key column to pg_attribute and + // pg_opclass, and a join that fails would drop a column from the key + // silently, turning a real key difference into an apparent match. + KeyLength int +} + +// GetReplicaIdentityKey reads each table's replica identity mode and, when +// it names an explicit index, that index's key columns and operator +// classes, in index-column order. +func GetReplicaIdentityKey(ctx context.Context, db DBQuerier, schema string, tables []string) (map[string]ReplicaIdentityDescriptor, error) { + sql, err := RenderSQL(SQLTemplates.GetReplicaIdentityKey, nil) + if err != nil { + return nil, fmt.Errorf("failed to render GetReplicaIdentityKey SQL: %w", err) + } + + rows, err := db.Query(ctx, sql, schema, tables) + if err != nil { + return nil, fmt.Errorf("query to get replica identity keys for schema %q failed: %w", schema, err) + } + defer rows.Close() + + result := make(map[string]ReplicaIdentityDescriptor) + for rows.Next() { + var d ReplicaIdentityDescriptor + if err := rows.Scan(&d.Table, &d.ReplicaIdentity, &d.KeyColumns, &d.KeyOpclasses, &d.KeyLength); err != nil { + return nil, fmt.Errorf("failed to scan replica identity descriptor: %w", err) + } + // A short key is a wrong key, and a wrong key that still compares + // equal to the other node's is worse than an error. See KeyLength. + if len(d.KeyColumns) != d.KeyLength || len(d.KeyOpclasses) != d.KeyLength { + return nil, fmt.Errorf( + "replica identity key for table %q resolved to %d column(s) and %d operator class(es), but the index declares %d key column(s)", + d.Table, len(d.KeyColumns), len(d.KeyOpclasses), d.KeyLength) + } + result[d.Table] = d + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating over replica identity descriptors: %w", err) + } + return result, nil +} + +// ConstraintDescriptor is one PRIMARY KEY, UNIQUE, CHECK, FOREIGN KEY or +// EXCLUDE constraint. It deliberately has no Name field: PostgreSQL invents +// names for unnamed constraints, so identical constraints on two nodes can +// carry different names with no structural difference at all. Comparison +// must go by Definition. +type ConstraintDescriptor struct { + Table string + Type string // "p" | "u" | "c" | "f" | "x" + Definition string + Deferrable bool + Validated bool +} + +// GetConstraintDescriptors reads every PRIMARY KEY, UNIQUE, CHECK, FOREIGN +// KEY and EXCLUDE constraint on the given tables. +func GetConstraintDescriptors(ctx context.Context, db DBQuerier, schema string, tables []string) (map[string][]ConstraintDescriptor, error) { + sql, err := RenderSQL(SQLTemplates.GetConstraintDescriptors, nil) + if err != nil { + return nil, fmt.Errorf("failed to render GetConstraintDescriptors SQL: %w", err) + } + + rows, err := db.Query(ctx, sql, schema, tables) + if err != nil { + return nil, fmt.Errorf("query to get constraint descriptors for schema %q failed: %w", schema, err) + } + defer rows.Close() + + result := make(map[string][]ConstraintDescriptor) + for rows.Next() { + var c ConstraintDescriptor + if err := rows.Scan(&c.Table, &c.Type, &c.Definition, &c.Deferrable, &c.Validated); err != nil { + return nil, fmt.Errorf("failed to scan constraint descriptor: %w", err) + } + result[c.Table] = append(result[c.Table], c) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating over constraint descriptors: %w", err) + } + return result, nil +} + +// PartitionDescriptor says whether a table is itself a partition (and of +// what bound) and/or is itself partitioned (and by what key). Empty strings +// mean "not applicable", not "unknown". +type PartitionDescriptor struct { + Table string + PartitionBound string + PartitionKey string +} + +// GetPartitionDescriptors reads partition bound and partition key +// information for the given tables. +func GetPartitionDescriptors(ctx context.Context, db DBQuerier, schema string, tables []string) (map[string]PartitionDescriptor, error) { + sql, err := RenderSQL(SQLTemplates.GetPartitionDescriptors, nil) + if err != nil { + return nil, fmt.Errorf("failed to render GetPartitionDescriptors SQL: %w", err) + } + + rows, err := db.Query(ctx, sql, schema, tables) + if err != nil { + return nil, fmt.Errorf("query to get partition descriptors for schema %q failed: %w", schema, err) + } + defer rows.Close() + + result := make(map[string]PartitionDescriptor) + for rows.Next() { + var d PartitionDescriptor + if err := rows.Scan(&d.Table, &d.PartitionBound, &d.PartitionKey); err != nil { + return nil, fmt.Errorf("failed to scan partition descriptor: %w", err) + } + result[d.Table] = d + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating over partition descriptors: %w", err) + } + return result, nil +} + +// DomainDescriptor is what a domain (pg_type.typtype = 'd') constrains, +// resolved one level of typbasetype deep. BaseTypeNamespace/BaseTypeName/ +// BaseTypeMod carry the same portable (namespace, name, typmod) shape as a +// column's own type, so the same comparison logic applies to both. OID +// appears only as the map key (see GetDomainDescriptors); it is local to +// this node. +type DomainDescriptor struct { + Namespace string + Name string + BaseTypeNamespace string + BaseTypeName string + // BaseTypeKind is the base type's typtype, part of its identity: see + // GetDomainDescriptors' base_kind. + BaseTypeKind string + BaseTypeMod int32 + // BaseTypeText is the base type as format_type prints it, modifier + // included, for display (e.g. "character varying(20)"); nothing + // compares it (see GetDomainDescriptors). + BaseTypeText string + NotNull bool + Default string + Checks []string // CHECK definitions, sorted by definition text (see GetDomainDescriptors) +} + +// GetDomainDescriptors resolves every domain named by oids, keyed by that +// OID. oids must come from this node's own GetColumnDescriptors result +// (or a recursive domain-of-domain lookup on this node); the map keys are +// local to this node — DomainDescriptor's fields are what get compared +// across nodes. +func GetDomainDescriptors(ctx context.Context, db DBQuerier, oids []uint32) (map[uint32]DomainDescriptor, error) { + result := make(map[uint32]DomainDescriptor) + if len(oids) == 0 { + return result, nil + } + + sql, err := RenderSQL(SQLTemplates.GetDomainDescriptors, nil) + if err != nil { + return nil, fmt.Errorf("failed to render GetDomainDescriptors SQL: %w", err) + } + + rows, err := db.Query(ctx, sql, oids) + if err != nil { + return nil, fmt.Errorf("query to get domain descriptors failed: %w", err) + } + defer rows.Close() + + for rows.Next() { + var oid uint32 + var d DomainDescriptor + if err := rows.Scan(&oid, &d.Namespace, &d.Name, &d.BaseTypeNamespace, &d.BaseTypeName, + &d.BaseTypeKind, &d.BaseTypeMod, &d.BaseTypeText, &d.NotNull, &d.Default, + &d.Checks); err != nil { + return nil, fmt.Errorf("failed to scan domain descriptor: %w", err) + } + result[oid] = d + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating over domain descriptors: %w", err) + } + return result, nil +} + +// RangeDescriptor is what makes a range type (pg_type.typtype = 'r') mean +// what it means: its element type and the collation/opclass/functions that +// order and canonicalise it. Canonical/SubtypeDiff are already portable +// text (see GetRangeDescriptors — printed via ::regprocedure::text), not +// OIDs. +type RangeDescriptor struct { + Namespace string + Name string + SubtypeNamespace string + SubtypeName string + // SubtypeKind is the subtype's typtype, part of its identity: see + // GetDomainDescriptors' base_kind. + SubtypeKind string + // Collation and Opclass are schema-qualified, or empty when the range + // has none. + Collation string + Opclass string + Canonical string + SubtypeDiff string +} + +// GetRangeDescriptors resolves every range type named by oids, keyed by +// that OID (this node's own — see GetDomainDescriptors' same caveat). +func GetRangeDescriptors(ctx context.Context, db DBQuerier, oids []uint32) (map[uint32]RangeDescriptor, error) { + result := make(map[uint32]RangeDescriptor) + if len(oids) == 0 { + return result, nil + } + + sql, err := RenderSQL(SQLTemplates.GetRangeDescriptors, nil) + if err != nil { + return nil, fmt.Errorf("failed to render GetRangeDescriptors SQL: %w", err) + } + + rows, err := db.Query(ctx, sql, oids) + if err != nil { + return nil, fmt.Errorf("query to get range descriptors failed: %w", err) + } + defer rows.Close() + + for rows.Next() { + var oid uint32 + var d RangeDescriptor + if err := rows.Scan(&oid, &d.Namespace, &d.Name, &d.SubtypeNamespace, &d.SubtypeName, + &d.SubtypeKind, &d.Collation, &d.Opclass, &d.Canonical, &d.SubtypeDiff); err != nil { + return nil, fmt.Errorf("failed to scan range descriptor: %w", err) + } + result[oid] = d + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating over range descriptors: %w", err) + } + return result, nil +} + +// CompositeAttribute is one field of a composite type (pg_type.typtype = +// 'c'), in attnum order: attribute order is part of a composite type's +// structural identity (see GetCompositeAttributes). +type CompositeAttribute struct { + // AttNum is the attribute's own pg_attribute.attnum, not its position + // in Attributes: DROP ATTRIBUTE leaves the surviving attributes' + // attnums alone, so this is the ordinal that stays comparable across + // nodes. See GetCompositeAttributes. + AttNum int16 + Name string + TypeNamespace string + TypeName string + // TypeKind is the attribute type's typtype, part of its identity: see + // GetDomainDescriptors' base_kind. + TypeKind string + TypeMod int32 + // TypeText is the attribute's type as format_type prints it. Display + // only, never compared (see BaseTypeText). + TypeText string + // Collation is schema-qualified, or empty when the attribute has none. + Collation string +} + +// CompositeDescriptor is one composite type's own portable identity plus +// its attributes, in declaration order. +type CompositeDescriptor struct { + Namespace string + Name string + Attributes []CompositeAttribute +} + +// GetCompositeAttributes resolves every composite type named by oids, +// keyed by that OID (this node's own), each with its attributes in +// declaration order. +func GetCompositeAttributes(ctx context.Context, db DBQuerier, oids []uint32) (map[uint32]CompositeDescriptor, error) { + result := make(map[uint32]CompositeDescriptor) + if len(oids) == 0 { + return result, nil + } + + sql, err := RenderSQL(SQLTemplates.GetCompositeAttributes, nil) + if err != nil { + return nil, fmt.Errorf("failed to render GetCompositeAttributes SQL: %w", err) + } + + rows, err := db.Query(ctx, sql, oids) + if err != nil { + return nil, fmt.Errorf("query to get composite attributes failed: %w", err) + } + defer rows.Close() + + for rows.Next() { + var oid uint32 + var namespace, name string + var a CompositeAttribute + if err := rows.Scan(&oid, &namespace, &name, &a.AttNum, &a.Name, + &a.TypeNamespace, &a.TypeName, &a.TypeKind, &a.TypeMod, &a.TypeText, + &a.Collation); err != nil { + return nil, fmt.Errorf("failed to scan composite attribute: %w", err) + } + d := result[oid] + d.Namespace, d.Name = namespace, name + d.Attributes = append(d.Attributes, a) + result[oid] = d + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating over composite attributes: %w", err) + } + return result, nil +} + +// EnumDescriptor is one enum type's own portable identity plus its labels, +// in enumsortorder. +type EnumDescriptor struct { + Namespace string + Name string + Labels []string +} + +// GetEnumLabels resolves every enum type named by oids, keyed by that OID +// (this node's own), each with its labels in enumsortorder — order is the +// entire point of an enum. +func GetEnumLabels(ctx context.Context, db DBQuerier, oids []uint32) (map[uint32]EnumDescriptor, error) { + result := make(map[uint32]EnumDescriptor) + if len(oids) == 0 { + return result, nil + } + + sql, err := RenderSQL(SQLTemplates.GetEnumLabels, nil) + if err != nil { + return nil, fmt.Errorf("failed to render GetEnumLabels SQL: %w", err) + } + + rows, err := db.Query(ctx, sql, oids) + if err != nil { + return nil, fmt.Errorf("query to get enum labels failed: %w", err) + } + defer rows.Close() + + for rows.Next() { + var oid uint32 + var namespace, name, label string + if err := rows.Scan(&oid, &namespace, &name, &label); err != nil { + return nil, fmt.Errorf("failed to scan enum label: %w", err) + } + d := result[oid] + d.Namespace, d.Name = namespace, name + d.Labels = append(d.Labels, label) + result[oid] = d + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating over enum labels: %w", err) + } + return result, nil +} + +// TypeReference is one type's kind plus the OIDs of the types it is built +// out of. Every OID here is local to the node it was read from. +type TypeReference struct { + OID uint32 + Kind string // typtype: 'b' base | 'd' domain | 'e' enum | 'r' range | 'm' multirange | 'c' composite | 'p' pseudo + // Refs holds each referenced type, with zeroes dropped: an array's + // element type, a domain's base type, a range's subtype, a + // multirange's range, and a composite's attribute types. + Refs []uint32 +} + +// GetTypeReferences reads the kind of every type in oids and the types each +// of them refers to, for a caller resolving the full set of types a schema +// depends on. See the GetTypeReferences SQL template for which catalog +// edges are followed and why a single pass over a column's own typtype is +// not enough. +func GetTypeReferences(ctx context.Context, db DBQuerier, oids []uint32) (map[uint32]TypeReference, error) { + result := make(map[uint32]TypeReference) + if len(oids) == 0 { + return result, nil + } + + sql, err := RenderSQL(SQLTemplates.GetTypeReferences, nil) + if err != nil { + return nil, fmt.Errorf("failed to render GetTypeReferences SQL: %w", err) + } + + rows, err := db.Query(ctx, sql, oids) + if err != nil { + return nil, fmt.Errorf("query to get type references failed: %w", err) + } + defer rows.Close() + + for rows.Next() { + var ( + ref TypeReference + element, base, rangeSubtype, multirangeRange uint32 + attributes []uint32 + ) + if err := rows.Scan(&ref.OID, &ref.Kind, &element, &base, + &rangeSubtype, &multirangeRange, &attributes); err != nil { + return nil, fmt.Errorf("failed to scan type reference: %w", err) + } + for _, candidate := range append([]uint32{element, base, rangeSubtype, multirangeRange}, attributes...) { + if candidate != 0 { + ref.Refs = append(ref.Refs, candidate) + } + } + result[ref.OID] = ref + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating over type references: %w", err) + } + return result, nil +} + +// DatabaseLocale is the collation configuration of one database. +type DatabaseLocale struct { + Name string // datname, for a report that has to say which database + Collate string // datcollate + Ctype string // datctype + Provider string // datlocprovider: 'c' libc | 'i' icu | 'b' builtin; empty before PostgreSQL 15 + Locale string // datlocale, or daticulocale before PostgreSQL 17; empty when unset +} + +// GetDatabaseLocale reads the connected database's collation settings. +// +// A column that does not name a collation of its own inherits these, and +// inherits them invisibly: pg_attribute records the "default" collation on +// every node whatever the database was created with. Comparing this is the +// only way to notice that two nodes holding the same rows disagree about +// how those rows sort. See the GetDatabaseLocale SQL template. +func GetDatabaseLocale(ctx context.Context, db DBQuerier) (DatabaseLocale, error) { + var locale DatabaseLocale + + sql, err := RenderSQL(SQLTemplates.GetDatabaseLocale, nil) + if err != nil { + return locale, fmt.Errorf("failed to render GetDatabaseLocale SQL: %w", err) + } + + row := db.QueryRow(ctx, sql) + if err := row.Scan(&locale.Name, &locale.Collate, &locale.Ctype, + &locale.Provider, &locale.Locale); err != nil { + return locale, fmt.Errorf("failed to scan database locale: %w", err) + } + return locale, nil +} + +// QuoteIdentifiers renders every distinct name in names the way this node's +// own PostgreSQL would write it back via quote_ident(), so identifiers +// round-trip unambiguously. +// +// The returned map has one entry per distinct input name. A name that does +// not come back is absent; callers should treat that as "print the raw +// name" for display. +func QuoteIdentifiers(ctx context.Context, db DBQuerier, names []string) (map[string]string, error) { + result := make(map[string]string, len(names)) + if len(names) == 0 { + return result, nil + } + + sql, err := RenderSQL(SQLTemplates.QuoteIdentifiers, nil) + if err != nil { + return nil, fmt.Errorf("failed to render QuoteIdentifiers SQL: %w", err) + } + + rows, err := db.Query(ctx, sql, names) + if err != nil { + return nil, fmt.Errorf("query to quote identifiers failed: %w", err) + } + defer rows.Close() + + for rows.Next() { + var raw, quoted string + if err := rows.Scan(&raw, "ed); err != nil { + return nil, fmt.Errorf("failed to scan quoted identifier: %w", err) + } + result[raw] = quoted + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating over quoted identifiers: %w", err) + } + return result, nil +} + func GetPkeyType(ctx context.Context, db DBQuerier, schema, table, pkey string) (string, error) { sql, err := RenderSQL(SQLTemplates.GetPkeyType, nil) if err != nil { @@ -1455,7 +2005,6 @@ func UpdateLeafHashesBatch(ctx context.Context, db DBQuerier, mtreeTable string, return nil } - // MarkLeavesDirtyByPositions flags the given leaf blocks for rehash. Used to // refresh leaves whose stored hash is stale relative to the live table data // (e.g. mismatches resolved as false positives during a diff). @@ -2164,7 +2713,6 @@ func UpdateBlockRangeStartComposite(ctx context.Context, db DBQuerier, mtreeTabl return nil } - func GetMinValComposite(ctx context.Context, db DBQuerier, schema, table string, pkeyCols []string) ([]interface{}, error) { cols := make([]string, len(pkeyCols)) for i, c := range pkeyCols { @@ -2986,9 +3534,10 @@ func DropCDCMetadataTable(ctx context.Context, db DBQuerier) error { return nil } -// pubCommitLSN is empty for legacy metadata rows that pre-date the -// pub_commit_lsn column; callers must treat empty as "invariant -// uncheckable" and skip the publication-commit guard with a warning. +// GetCDCMetadata reads cdc metadata for a publication. pubCommitLSN is +// empty for legacy rows that pre-date that column; callers must treat +// empty as uncheckable and skip the publication-commit guard with a +// warning. func GetCDCMetadata(ctx context.Context, db DBQuerier, publicationName string) (slotName, startLSN string, tables []string, pubCommitLSN string, err error) { sql, err := RenderSQL(SQLTemplates.GetCDCMetadata, nil) if err != nil { @@ -3001,9 +3550,9 @@ func GetCDCMetadata(ctx context.Context, db DBQuerier, publicationName string) ( return slotName, startLSN, tables, pubCommitLSN, nil } -// Init path only. Ongoing flushes use UpdateCDCMetadata, which deliberately -// leaves pub_commit_lsn untouched so the listen.go guard always compares -// against the LSN captured at the matching init. +// InitCDCMetadata sets cdc metadata at init time, including pub_commit_lsn. +// Ongoing flushes use UpdateCDCMetadata, which leaves pub_commit_lsn +// untouched so it always reflects the LSN captured at init. func InitCDCMetadata(ctx context.Context, db DBQuerier, publicationName, slotName, startLSN, pubCommitLSN string, tables []string) error { sql, err := RenderSQL(SQLTemplates.InitCDCMetadata, nil) if err != nil { @@ -3019,10 +3568,11 @@ func InitCDCMetadata(ctx context.Context, db DBQuerier, publicationName, slotNam return nil } -// Called mid-Phase-A, after CREATE PUBLICATION, so the captured value is -// strictly less than Phase A's commit LSN. The slot created in Phase B -// has consistent_point >= that commit LSN, hence consistent_point > -// captured value: a safe lower bound for any valid replication start LSN. +// CurrentWalInsertLSN returns the current WAL insert LSN. Called mid Phase +// A, after CREATE PUBLICATION, so the value is strictly less than Phase +// A's commit LSN; since Phase B's slot has consistent_point >= that commit +// LSN, the returned value is a safe lower bound for any valid replication +// start LSN. func CurrentWalInsertLSN(ctx context.Context, db DBQuerier) (string, error) { sql, err := RenderSQL(SQLTemplates.CurrentWalInsertLSN, nil) if err != nil { diff --git a/db/queries/templates.go b/db/queries/templates.go index 7c741ebc..a9d232bc 100644 --- a/db/queries/templates.go +++ b/db/queries/templates.go @@ -19,7 +19,7 @@ import ( ) // aceTemplateFuncs provides the {{aceSchema}} function to SQL templates. -// The function is evaluated at render time (after config is loaded), not at parse time. +// It runs at render time, after config is loaded. var aceTemplateFuncs = template.FuncMap{ "aceSchema": func() string { return pgx.Identifier{config.Get().MTree.Schema}.Sanitize() }, } @@ -45,6 +45,21 @@ type Templates struct { GetPkeyColumnTypes *template.Template GetRelationTree *template.Template + // Structure-comparison descriptors. Each reads one aspect of table + // structure for every table named in $2, scoped to schema $1. Each + // query is small, independently readable, and testable. + GetColumnDescriptors *template.Template + GetReplicaIdentityKey *template.Template + GetConstraintDescriptors *template.Template + GetPartitionDescriptors *template.Template + GetDomainDescriptors *template.Template + GetRangeDescriptors *template.Template + GetCompositeAttributes *template.Template + GetEnumLabels *template.Template + GetTypeReferences *template.Template + GetDatabaseLocale *template.Template + QuoteIdentifiers *template.Template + CreateMetadataTable *template.Template GetPkeyOffsets *template.Template CreateSimpleMtreeTable *template.Template @@ -154,7 +169,7 @@ type Templates struct { } var SQLTemplates = Templates{ - // A template isn't needed for this query; just keeping the struct uniform + // A template isn't needed here; kept for struct uniformity. CreateMetadataTable: template.Must(template.New("createMetadataTable").Funcs(aceTemplateFuncs).Parse(` CREATE TABLE IF NOT EXISTS {{aceSchema}}.ace_mtree_metadata ( schema_name text, @@ -227,10 +242,9 @@ var SQLTemplates = Templates{ last_updated = EXCLUDED.last_updated `)), - // On conflict every column including pub_commit_lsn is refreshed: - // reaching this query path means a re-init, which created a fresh - // publication with a new commit LSN, and listen.go's guard must - // compare against that current LSN — not a stale prior one. + // On conflict every column including pub_commit_lsn is refreshed, + // since reaching this path means a re-init produced a fresh + // publication with a new commit LSN that later reads must see. InitCDCMetadata: template.Must(template.New("initCdcMetadata").Funcs(aceTemplateFuncs).Parse(` INSERT INTO {{aceSchema}}.ace_cdc_metadata ( @@ -279,15 +293,12 @@ var SQLTemplates = Templates{ DROP TABLE IF EXISTS {{aceSchema}}.ace_cdc_metadata `)), - // pub_commit_lsn is extracted via to_jsonb(row) ->> 'pub_commit_lsn' - // instead of a direct column reference so the query parses and runs - // against pre-migration ace_cdc_metadata tables that lack the column. - // On legacy 3-column rows the JSON object has no pub_commit_lsn key, - // ->> returns NULL, and COALESCE produces the empty string the - // listen.go guard treats as "invariant uncheckable, warn and skip". - // The additive ALTER TABLE in CreateCDCMetadataTable still backfills - // the column on the next MtreeInit so post-init reads use the real - // column path. + // pub_commit_lsn is extracted via to_jsonb(row) ->> 'pub_commit_lsn' so + // the query still works on pre-migration ace_cdc_metadata tables + // missing that column: ->> then returns NULL and COALESCE yields an + // empty string, treated as "invariant uncheckable, warn and skip". + // CreateCDCMetadataTable's additive ALTER TABLE backfills the column + // on the next MtreeInit so later reads use it directly. GetCDCMetadata: template.Must(template.New("getCDCMetadata").Funcs(aceTemplateFuncs).Parse(` SELECT m.slot_name, @@ -741,6 +752,419 @@ var SQLTemplates = Templates{ AND a.attname = ANY($3::text[]) AND a.attnum > 0 AND NOT a.attisdropped; `)), + // GetColumnDescriptors reads every property of every live, non-dropped + // column of the given tables that structure comparison cares about: + // type, nullability, identity/generated-ness, any per-column options, + // the collation and its version, and the default expression. Ordered + // by column name, since column order carries no structural meaning. + // + // A column's type identity is carried as (type_namespace, type_name, + // atttypmod), which stays meaningful across independently initialized + // clusters even though object OIDs differ between them. atttypid is + // selected only so CollectSnapshot can use it, on this node's own + // connection, to fetch domain/range/composite/enum descriptors for + // the types actually in play. + // + // type_kind is pg_type.typtype: 'b' base, 'd' domain, 'e' enum, + // 'r' range, 'c' composite. It lets the comparison layer decide + // whether a type-name mismatch can be reasoned about via the + // built-in narrowing tables or must be treated as a plain difference. + GetColumnDescriptors: template.Must(template.New("getColumnDescriptors").Parse(` + SELECT + c.relname, + a.attname, + a.atttypid, + a.atttypmod, + pg_catalog.format_type(a.atttypid, a.atttypmod) AS type_text, + tn.nspname AS type_namespace, + t.typname AS type_name, + t.typtype::text AS type_kind, + a.attnotnull, + a.attidentity::text, + a.attgenerated::text, + -- attoptions keeps the order the options were applied in, so the + -- same options set in a different order would compare unequal. + -- Sorted here as GetDomainDescriptors sorts a domain's CHECKs. + -- Entries are keyword=value from a fixed set, so none holds a + -- comma. + COALESCE(( + SELECT string_agg(opt, ',' ORDER BY opt) + FROM pg_catalog.unnest(a.attoptions) AS opt + ), '') AS options, + -- A collation's identity is (namespace, name), for the same + -- reason a type's is: two schemas can each hold a collation + -- named "en_US" that resolve differently. + COALESCE(cn.nspname, '') AS collnamespace, + COALESCE(co.collname, '') AS collname, + COALESCE(co.collprovider::text, '') AS collprovider, + -- collversion is what the catalog recorded when the collation + -- was created or last REFRESHed - NOT the version of the + -- collation library running now. An unrefreshed glibc upgrade + -- leaves this string matching on both nodes while the two nodes + -- actually sort differently, so GetDatabaseLocale is what + -- catches the common case; this field only catches a node whose + -- catalog was refreshed against a different library. + COALESCE(co.collversion, '') AS collversion, + COALESCE(pg_catalog.pg_get_expr(ad.adbin, ad.adrelid), '') AS default_expr + FROM pg_catalog.pg_class c + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + JOIN pg_catalog.pg_attribute a ON a.attrelid = c.oid + JOIN pg_catalog.pg_type t ON t.oid = a.atttypid + JOIN pg_catalog.pg_namespace tn ON tn.oid = t.typnamespace + LEFT JOIN pg_catalog.pg_attrdef ad ON ad.adrelid = a.attrelid AND ad.adnum = a.attnum + LEFT JOIN pg_catalog.pg_collation co ON co.oid = a.attcollation + LEFT JOIN pg_catalog.pg_namespace cn ON cn.oid = co.collnamespace + WHERE n.nspname = $1 + AND c.relname = ANY($2::text[]) + AND a.attnum > 0 + AND NOT a.attisdropped + -- COLLATE "C" on every text ordering in this file. Row order is an + -- input to comparison wherever it is preserved rather than re-sorted + -- (array_agg of a domain's CHECKs, a composite's attributes), and a + -- database-default collation differing between two nodes would + -- otherwise reorder identical catalogs. Pinning the collation costs + -- nothing and removes the question. + ORDER BY c.relname COLLATE "C", a.attname COLLATE "C"; + `)), + // GetDomainDescriptors resolves what a domain (typtype='d') actually + // constrains, for every domain OID in $1: one level of typbasetype, + // so a domain-over-domain's further narrowing is only caught when + // that inner domain is itself directly used by some compared column. + // The base type is named portably (namespace, name, typmod), like a + // column's own type. Constraints are aggregated as an array, one row + // per domain, sorted by definition text, since CHECK (VALUE ...) + // constraints on a domain get invented names like table constraints + // do. + // + // $1 is an oid[] gathered from this node's own GetColumnDescriptors + // result and stays local to this node; only the resolved names below + // are compared across nodes. + GetDomainDescriptors: template.Must(template.New("getDomainDescriptors").Parse(` + SELECT + t.oid, + n.nspname, + t.typname, + bn.nspname AS base_namespace, + bt.typname AS base_name, + -- typtype belongs to the base type's identity: dropping an enum + -- and recreating the name as a domain leaves (namespace, name) + -- untouched, so without this the substitution is invisible. + bt.typtype::text AS base_kind, + t.typtypmod, + -- Display only, never compared: the base type as a person writes + -- it, modifier included ("character varying(20)"), so a report + -- about a domain narrowed from varchar(20) to varchar(10) shows + -- the length rather than printing "varchar" on both sides. The + -- comparison itself keys on (base_namespace, base_name, + -- typtypmod) above, exactly as a column's type does. + pg_catalog.format_type(t.typbasetype, t.typtypmod) AS base_text, + t.typnotnull, + COALESCE(pg_catalog.pg_get_expr(t.typdefaultbin, 0), t.typdefault, '') AS default_expr, + COALESCE(chk.defs, '{}') AS check_defs + FROM pg_catalog.pg_type t + JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace + JOIN pg_catalog.pg_type bt ON bt.oid = t.typbasetype + JOIN pg_catalog.pg_namespace bn ON bn.oid = bt.typnamespace + LEFT JOIN LATERAL ( + SELECT array_agg(pg_catalog.pg_get_constraintdef(ct.oid, true) ORDER BY pg_catalog.pg_get_constraintdef(ct.oid, true) COLLATE "C") AS defs + FROM pg_catalog.pg_constraint ct + WHERE ct.contypid = t.oid AND ct.contype = 'c' + ) chk ON true + WHERE t.oid = ANY($1::oid[]) AND t.typtype = 'd'; + `)), + // GetRangeDescriptors resolves a range type's (typtype='r') subtype and + // the functions/collation/opclass that define its ordering, since any + // of these changes what "the same range" means. Functions are printed + // via ::regprocedure::text for a portable, schema-qualified signature + // (e.g. "public.my_canon(daterange)"). + GetRangeDescriptors: template.Must(template.New("getRangeDescriptors").Parse(` + SELECT + t.oid, + n.nspname, + t.typname, + sn.nspname AS subtype_namespace, + st.typname AS subtype_name, + -- See base_kind in GetDomainDescriptors. + st.typtype::text AS subtype_kind, + -- Collation and operator class are named (namespace, name) for + -- the same reason types are: an unqualified opcname is only + -- unique within one namespace and access method. + CASE WHEN co.oid IS NULL THEN '' + ELSE con.nspname || '.' || co.collname + END AS collation, + CASE WHEN oc.oid IS NULL THEN '' + ELSE ocn.nspname || '.' || oc.opcname + END AS opclass, + CASE WHEN r.rngcanonical = 0 THEN '' ELSE r.rngcanonical::pg_catalog.regprocedure::text END AS canonical, + CASE WHEN r.rngsubdiff = 0 THEN '' ELSE r.rngsubdiff::pg_catalog.regprocedure::text END AS subtype_diff + FROM pg_catalog.pg_type t + JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace + JOIN pg_catalog.pg_range r ON r.rngtypid = t.oid + JOIN pg_catalog.pg_type st ON st.oid = r.rngsubtype + JOIN pg_catalog.pg_namespace sn ON sn.oid = st.typnamespace + LEFT JOIN pg_catalog.pg_collation co ON co.oid = r.rngcollation + LEFT JOIN pg_catalog.pg_namespace con ON con.oid = co.collnamespace + LEFT JOIN pg_catalog.pg_opclass oc ON oc.oid = r.rngsubopc + LEFT JOIN pg_catalog.pg_namespace ocn ON ocn.oid = oc.opcnamespace + WHERE t.oid = ANY($1::oid[]) AND t.typtype = 'r'; + `)), + // GetCompositeAttributes reads a composite type's (typtype='c') own + // attributes, one row per attribute, ordered by attnum: a composite + // type's attnum order is its wire/row-literal layout, fixed at + // CREATE TYPE time. + GetCompositeAttributes: template.Must(template.New("getCompositeAttributes").Parse(` + SELECT + t.oid, + n.nspname, + t.typname, + -- attnum, not a dense 1..n counter: DROP ATTRIBUTE leaves gaps, + -- and an ordinal that renumbers after a gap makes every later + -- attribute look changed when only one was dropped. + a.attnum, + a.attname, + an.nspname AS attr_type_namespace, + at.typname AS attr_type_name, + -- See base_kind in GetDomainDescriptors. + at.typtype::text AS attr_type_kind, + a.atttypmod, + -- Display only, never compared (see base_text above). + pg_catalog.format_type(a.atttypid, a.atttypmod) AS attr_type_text, + CASE WHEN co.oid IS NULL THEN '' + ELSE cn.nspname || '.' || co.collname + END AS collation + FROM pg_catalog.pg_type t + JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace + JOIN pg_catalog.pg_attribute a ON a.attrelid = t.typrelid + JOIN pg_catalog.pg_type at ON at.oid = a.atttypid + JOIN pg_catalog.pg_namespace an ON an.oid = at.typnamespace + LEFT JOIN pg_catalog.pg_collation co ON co.oid = a.attcollation + LEFT JOIN pg_catalog.pg_namespace cn ON cn.oid = co.collnamespace + WHERE t.oid = ANY($1::oid[]) AND t.typtype = 'c' + AND a.attnum > 0 AND NOT a.attisdropped + ORDER BY t.oid, a.attnum; + `)), + // GetEnumLabels reads an enum type's (typtype='e') labels in + // enumsortorder, since order is the defining property of an enum. + GetEnumLabels: template.Must(template.New("getEnumLabels").Parse(` + SELECT + t.oid, + n.nspname, + t.typname, + e.enumlabel + FROM pg_catalog.pg_type t + JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace + JOIN pg_catalog.pg_enum e ON e.enumtypid = t.oid + WHERE t.oid = ANY($1::oid[]) AND t.typtype = 'e' + ORDER BY t.oid, e.enumsortorder; + `)), + // GetTypeReferences reports, for each type OID in $1, its typtype and + // every other type it is built out of. The caller walks this to a fixed + // point, so a type is compared however deeply it is buried. + // + // Looking only at a column's own typtype misses most of the interesting + // cases: an array type is itself a base type ('b'), so a "status[]" + // column hides the enum entirely, and a composite's attribute or a + // domain's base type can be a user-defined type that no column mentions + // directly. Every edge that can carry a user-defined type is reported: + // + // typelem array -> element type (varlena arrays only, so that + // point/line, which also set typelem, are not + // mistaken for arrays) + // typbasetype domain -> base type, including domain over domain + // rngsubtype range -> subtype + // rngtypid multirange -> its range (typtype 'm', PostgreSQL 14+) + // typrelid composite -> each live attribute's type + // + // A zero OID means "no such edge". $1 and the OIDs returned are this + // node's own and never leave it; only the descriptors resolved from them + // are compared across nodes. + GetTypeReferences: template.Must(template.New("getTypeReferences").Parse(` + SELECT + t.oid, + t.typtype::text, + CASE WHEN t.typlen = -1 AND t.typelem <> 0 + THEN t.typelem ELSE 0 + END AS element_oid, + t.typbasetype AS base_oid, + COALESCE(( + SELECT r.rngsubtype FROM pg_catalog.pg_range r + WHERE r.rngtypid = t.oid + ), 0) AS range_subtype_oid, + COALESCE(( + SELECT r.rngtypid FROM pg_catalog.pg_range r + WHERE r.rngmultitypid = t.oid + ), 0) AS multirange_range_oid, + COALESCE(( + SELECT array_agg(a.atttypid ORDER BY a.attnum) + FROM pg_catalog.pg_attribute a + WHERE a.attrelid = t.typrelid + AND a.attnum > 0 + AND NOT a.attisdropped + ), '{}'::oid[]) AS attribute_type_oids + FROM pg_catalog.pg_type t + WHERE t.oid = ANY($1::oid[]); + `)), + // GetDatabaseLocale reads the collation settings of the database this + // connection is attached to. + // + // This is the collation fact that matters most for two nodes meant to + // hold the same rows, and the one a per-column check cannot see: a text + // column that does not name a collation resolves to the database + // default, which appears in pg_attribute as the "default" collation on + // every node regardless of what LC_COLLATE the database was actually + // created with. Two nodes, one initdb'd en_US.UTF-8 and one C, therefore + // agree column by column while sorting differently - and a unique index + // that disagrees about which strings are duplicates is a data-loss + // hazard, not a cosmetic one. + // + // The provider and locale columns are read through to_jsonb rather than + // named directly, because their names move: datlocprovider arrived in + // PostgreSQL 15, and the ICU locale is daticulocale in 15 and 16 but + // datlocale from 17 on. Naming a column that does not exist fails at + // parse time even on the branch that would not have executed, so the + // version differences cannot be handled with a CASE; ->> on a row + // converted to jsonb simply yields NULL for a key that is not there. + GetDatabaseLocale: template.Must(template.New("getDatabaseLocale").Parse(` + SELECT + d.datname, + d.datcollate, + d.datctype, + COALESCE(pg_catalog.to_jsonb(d) ->> 'datlocprovider', '') AS locale_provider, + COALESCE( + pg_catalog.to_jsonb(d) ->> 'datlocale', + pg_catalog.to_jsonb(d) ->> 'daticulocale', + '' + ) AS locale + FROM pg_catalog.pg_database d + WHERE d.datname = pg_catalog.current_database(); + `)), + // QuoteIdentifiers renders every distinct identifier in $1 the way + // PostgreSQL itself would need to write it back to mean the same thing + // unambiguously, using quote_ident() — the same function pg_dump and + // this file's own deparse queries rely on, since correct quoting also + // requires case folding and a reserved-word list that varies across + // major versions. This keeps identifiers that collide when joined by + // a bare "." (table "a.b" column "c" vs. table "a" column "b.c") from + // colliding in a person-facing report either. + QuoteIdentifiers: template.Must(template.New("quoteIdentifiers").Parse(` + SELECT DISTINCT + raw, + pg_catalog.quote_ident(raw) AS quoted + FROM pg_catalog.unnest($1::text[]) AS raw; + `)), + // GetReplicaIdentityKey reads, per table, the replica identity mode + // (relreplident) and the key columns and operator classes of the index + // that mode actually designates, in index-column order, since (a,b) and + // (b,a) are not the same key. + // + // Which index that is depends on the mode: 'i' means the index flagged + // indisreplident, and 'd' - the default, and the common case - means the + // primary key. Resolving only the 'i' case would leave key_columns empty + // for almost every real table, so the comparison layer would compare "" + // against "" and report agreement no matter how the two nodes' primary + // keys differed. Modes 'f' (whole row) and 'n' (nothing) designate no + // index and come back empty; relreplident itself carries that. + // + // Exactly one index can match, so the aggregate is unambiguous: 'd' + // looks only at indisprimary and 'i' only at indisreplident, and + // PostgreSQL sets indisreplident on at most one index per table. The + // ORDER BY/LIMIT is belt and braces, preferring an explicitly designated + // index if a future PostgreSQL ever allows both flags at once. + // + // Expression index columns (indkey entry 0) have no pg_attribute row and + // would be dropped silently by the join below. PostgreSQL rejects + // expression indexes for both PRIMARY KEY and REPLICA IDENTITY USING + // INDEX, so no such column can reach here; key_length is returned so the + // caller can still verify that nothing was dropped. + GetReplicaIdentityKey: template.Must(template.New("getReplicaIdentityKey").Parse(` + SELECT + c.relname, + c.relreplident::text, + COALESCE(key_cols.key_columns, '{}') AS key_columns, + COALESCE(key_cols.key_opclasses, '{}') AS key_opclasses, + COALESCE(key_cols.key_length, 0) AS key_length + FROM pg_catalog.pg_class c + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + LEFT JOIN LATERAL ( + SELECT + array_agg(a.attname ORDER BY ik.ord) AS key_columns, + -- Named (namespace, name), the same way GetRangeDescriptors + -- names a range's operator class: an unqualified opcname is + -- only unique within one namespace and access method, so two + -- nodes keying a column on same-named operator classes from + -- different schemas - one ordering text by collation, one by + -- byte pattern - would otherwise compare equal. + array_agg(ocn.nspname || '.' || oc.opcname ORDER BY ik.ord) + AS key_opclasses, + i.indnkeyatts AS key_length + FROM pg_catalog.pg_index i + -- indkey holds indnatts entries, but only the first indnkeyatts + -- of them are key columns; the rest are INCLUDE payload, which + -- is not part of the row identity. They were previously dropped + -- only as a side effect of indclass being shorter than indkey - + -- an out-of-bounds array access doing the filtering. Say it. + JOIN LATERAL unnest(i.indkey) WITH ORDINALITY AS ik(attnum, ord) + ON ik.ord <= i.indnkeyatts + JOIN pg_catalog.pg_attribute a + ON a.attrelid = i.indrelid AND a.attnum = ik.attnum + -- int2vector/oidvector are 0-indexed by long-standing PostgreSQL + -- convention, and WITH ORDINALITY starts at 1, hence "ord - 1". + JOIN pg_catalog.pg_opclass oc ON oc.oid = i.indclass[(ik.ord - 1)::int] + JOIN pg_catalog.pg_namespace ocn ON ocn.oid = oc.opcnamespace + WHERE i.indrelid = c.oid + AND ( + i.indisreplident + OR (c.relreplident = 'd' AND i.indisprimary) + ) + GROUP BY i.indexrelid, i.indisreplident, i.indnkeyatts + ORDER BY i.indisreplident DESC + LIMIT 1 + ) key_cols ON true + WHERE n.nspname = $1 + AND c.relname = ANY($2::text[]); + `)), + // GetConstraintDescriptors reads PRIMARY KEY, UNIQUE, CHECK, FOREIGN KEY + // and EXCLUDE constraints. Omits the constraint's own name (conname): + // PostgreSQL invents names for unnamed constraints, so the same + // constraint can be named differently on two nodes with no structural + // difference. Comparison goes by condef, so rows are ordered by + // condef too. + GetConstraintDescriptors: template.Must(template.New("getConstraintDescriptors").Parse(` + SELECT + c.relname, + ct.contype::text, + pg_catalog.pg_get_constraintdef(ct.oid, true) AS condef, + ct.condeferrable, + ct.convalidated + FROM pg_catalog.pg_constraint ct + JOIN pg_catalog.pg_class c ON c.oid = ct.conrelid + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = $1 + AND c.relname = ANY($2::text[]) + AND ct.contype IN ('p', 'u', 'c', 'f', 'x') + -- The ordering expression is spelled out rather than referring to the + -- "condef" output alias: an alias is only visible to ORDER BY when it + -- stands alone, and adding COLLATE makes it an expression, where it + -- is not. + ORDER BY c.relname COLLATE "C", pg_catalog.pg_get_constraintdef(ct.oid, true) COLLATE "C"; + `)), + // GetPartitionDescriptors reads, per table, its own partition bound (if + // it is itself a partition of something) and its partitioning key (if + // it is itself partitioned). A table can be both, neither, or one of + // the two; empty strings mean "not applicable", not "unknown". + GetPartitionDescriptors: template.Must(template.New("getPartitionDescriptors").Parse(` + SELECT + c.relname, + COALESCE(pg_catalog.pg_get_expr(c.relpartbound, c.oid), '') AS partition_bound, + CASE WHEN c.relkind = 'p' + THEN COALESCE(pg_catalog.pg_get_partkeydef(c.oid), '') + ELSE '' + END AS partition_key + FROM pg_catalog.pg_class c + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = $1 + AND c.relname = ANY($2::text[]); + `)), GetPkeyOffsets: template.Must(template.New("pkeyOffsets").Parse(` WITH sampled_data AS ( SELECT diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 4818184e..10835e89 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -5,6 +5,26 @@ All notable changes to ACE will be captured in this document. This project follo ## [v2.1.1] ### Added +- **`schema-diff --compare=structure` checks table definitions directly, + without reading any data.** This new mode checks column type, `NOT NULL`, + identity, generated, storage options, default, and collation; the replica + identity key and its operator classes; `PRIMARY KEY`, `UNIQUE`, `CHECK`, + `FOREIGN KEY`, and `EXCLUDE` constraints; partition bounds; and the full + definition of every domain, range, composite, and enum type used by a + compared column, not just its name. Types are matched by name and + definition, never by OID, because two nodes set up on their own can give + the same user-defined type a different OID. Each difference found gets one + of five ranks (`cosmetic`, `equivalent-differing`, `narrowed`, + `incompatible`, `absent`), and the process exits with the code of the + single worst rank found (`0`, `16`, `32`, `48`, or `64`). `--skip-tables` + and `--skip-file` work the same way as they do for the default data diff. + `--output=json` prints a structured report instead of text, but only when + `--output` is given by hand on the command line, since `json` is also + `--output`'s default value for every other mode. Not yet supported: + `--schedule` and `--output=html`. Not checked by this mode: + non-constraint indexes, triggers, rules, sequences, views, materialized + views, storage parameters, column order, comments, and ACLs. See the + `schema-diff` command docs for the full list of findings and exit codes. - **Foreign tables, views, and partitioned tables with foreign partitions are refused with a clear message.** `table-diff`, `table-repair`, and `mtree` previously failed on these with "no primary key found", or in the diff --git a/docs/commands/diff/schema-diff.md b/docs/commands/diff/schema-diff.md index 4e318368..a49f9321 100644 --- a/docs/commands/diff/schema-diff.md +++ b/docs/commands/diff/schema-diff.md @@ -1,7 +1,9 @@ # schema-diff -Compares schemas across nodes. By default, runs `table-diff` on every table. +Compares schemas across nodes. By default, runs `table-diff` on every table. Alternatively, `--ddl-only` compares only object presence (tables, views, functions, indexes). +A third mode, `--compare=structure`, checks the actual definition of each common +table instead of its data or its mere presence. **Usage** @@ -20,17 +22,18 @@ Alternatively, `--ddl-only` compares only object presence (tables, views, functi |------|-------|-------------|---------| | `--dbname` | `-d` | Database name | | | `--nodes` | `-n` | Nodes to include (comma or `all`) | `all` | -| `--skip-tables` | `-T` | Comma list of tables to exclude | | -| `--skip-file` | `-s` | File with list of tables to exclude | | +| `--compare ` | | What to compare: `data` (default, per-table data diff) or `structure` (compares table definitions instead of data; see below). | +| `--skip-tables` | `-T` | Comma list of tables to exclude. Applies to `--compare=structure` too. | | +| `--skip-file` | `-s` | File with list of tables to exclude. Applies to `--compare=structure` too. | | | `--block-size ` | `-b` | Rows per block when diffing tables. Default `100000`. | | `--concurrency-factor ` | `-c` | CPU ratio for concurrency (0.0–4.0). Default `0.5`. | | `--compare-unit-size ` | `-u` | Recursive split size for mismatched blocks. Default `10000`. | -| `--output ` | `-o` | Per-table diff report format. Default `json`. | +| `--output ` | `-o` | Per-table diff report format. Default `json`. With `--compare=structure`, only `json` is accepted, and it only takes effect when given by hand (see below). | | `--override-block-size` | `-B` | Allow block sizes outside `ace.yaml` guardrails. | | `--ddl-only` | `-L` | Compare object sets only (no per-table diff) | `false` | | `--quiet` | `-q` | Suppress output | `false` | | `--debug` | `-v` | Debug logging | `false` | -| `--schedule` | `-S` | Run the schema diff repeatedly on a timer (requires `--every`). Not compatible with `--ddl-only`. | +| `--schedule` | `-S` | Run the schema diff repeatedly on a timer (requires `--every`). Not compatible with `--ddl-only` or `--compare=structure`. | | `--every ` | `-e` | Go duration string (for example, `24h`). Used with `--schedule`. | **Example** @@ -43,10 +46,70 @@ When `--ddl-only` is **not** set, every qualifying table invokes `table-diff` us ### Scheduling runs -Use `--schedule --every=` to keep a schema comparison running on a loop. This mode is only supported when ACE can run per-table diffs (omit `--ddl-only`): +Use `--schedule --every=` to keep a schema comparison running on a loop. This mode is only supported when ACE can run per-table diffs (omit `--ddl-only` and `--compare=structure`): ```sh ./ace schema-diff --schedule --every=24h --dbname=mydatabase my-cluster public ``` ACE performs the first comparison immediately, then waits for the given interval before repeating. Stop the process to end the loop. + +### Structure mode (`--compare=structure`) + +```sh +./ace schema-diff --compare=structure --dbname=mydatabase my-cluster public +``` + +This mode does not read table data. It reads the real table definition on +each node — columns, replica identity key, constraints, partition bounds, and +every domain, range, composite, and enum type a column uses — and compares +them directly. It is a fast check. Run it before the data diff (which is much +slower), or use it alone to check that the schema DDL is still the same on +every node. + +**What is compared:** column type, `NOT NULL`, identity, generated, storage +options, default, and collation; the replica identity key and its operator +classes; `PRIMARY KEY`, `UNIQUE`, `CHECK`, `FOREIGN KEY`, and `EXCLUDE` +constraints; partition bound and partition key; and the full definition of +every domain, range, composite, and enum type used by a compared column, not +just its name. It also compares the two databases' collation settings, +because a mismatch there changes how the same text sorts and compares on +each node. + +**What is not compared:** non-constraint indexes, triggers, rules, sequences, +views, materialized views, storage parameters, column order, comments, and +ACLs. A skipped view is named in a log line, not left out silently. + +Types are matched by name and definition, never by OID. Two nodes created by +separate `initdb` runs give different OIDs to the same user-defined type, so +matching by OID would either miss a real difference or report one that does +not exist. + +**Findings and exit code.** Each difference found gets one of five ranks. The +process exits with the code of the single worst rank found in the whole run: + +| Exit code | Rank | Meaning | +|---|---|---| +| `0` | (none) | The schemas are identical, or both agree that the schema is empty. | +| `16` | `cosmetic` | Does not change the shape of the data. This mode does not produce this rank today. | +| `32` | `equivalent-differing` | The same values fit on both sides, but are stored or handled differently — for example, a different collation on the same type. | +| `48` | `narrowed` | One side accepts a strict subset of the values the other side accepts — for example, `int4` vs `int8`, or a stricter `CHECK`. The report names the narrow side. | +| `64` | `incompatible` | Neither side's set of values contains the other's, or the object exists on only one node. | + +A table missing on some nodes is reported first, on its own, before the +per-table findings. It also counts toward the `64` exit code. + +**Getting a JSON report.** Pass `--output=json` on the command line to get a +structured report (schema name, node names, missing tables, and each finding) +instead of the plain-text report shown above. The flag must be given by hand: +running the command with no `--output` at all still prints text, even though +`json` is `--output`'s own default value for every other schema-diff mode. If +this mode did not check for that, every run would print JSON by default, even +one that never named `--output`. + +**Not yet supported with this mode:** + +- `--schedule` — rejected with a clear error. Use a loop around the command + instead, or use `--compare=data`, which does support scheduling. +- `--output=html` — rejected when given by hand, since this mode has no + per-table diff files to turn into HTML, only a list of findings. diff --git a/internal/cli/cli.go b/internal/cli/cli.go index e91e58d7..9254bafc 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -44,8 +44,8 @@ var defaultConfigYAML string var defaultPgServiceConf string func SetupCLI(version string) *cli.Command { - // Use -V (not the urfave default -v) for version, so -v stays reserved for - // the debug/verbose flag on subcommands (matching go tooling conventions). + // Use -V for version, so -v stays reserved for the debug/verbose flag on + // subcommands, matching go tooling conventions. cli.VersionFlag = &cli.BoolFlag{ Name: "version", Aliases: []string{"V"}, @@ -284,6 +284,11 @@ func SetupCLI(version string) *cli.Command { Usage: "Compare only schema objects (tables, functions, etc.), not table data", Value: false, }, + &cli.StringFlag{ + Name: "compare", + Usage: "What to compare: \"data\" (default, per-table data diff) or \"structure\" (symmetric column/key/constraint comparison, exits nonzero with a severity-coded status when something differs)", + Value: diff.CompareData, + }, &cli.BoolFlag{ Name: "schedule", Aliases: []string{"S"}, @@ -875,10 +880,9 @@ func resolveClusterArg(cmd, missingUsage, argsUsage string, required int, args [ cluster := config.DefaultCluster() if cluster == "" { if required > 0 { - // The positional(s) supplied were consumed as the required - // entity (e.g. ), leaving the cluster unset. Spell that - // out so a lone argument isn't silently misread as the cluster - // and reported back as a bare "cluster name is required". + // The positional(s) supplied were consumed as the required entity + // (e.g. ), leaving the cluster unset. Spell that out so a + // lone argument isn't misread as the cluster. return "", nil, fmt.Errorf( "cluster name is required: %q was read as the %s argument (usage: %s %s); "+ "pass the cluster as the first argument or set default_cluster in ace.yaml", @@ -1311,17 +1315,22 @@ func SchemaDiffCLI(cmd *cli.Command) error { task.SkipFile = cmd.String("skip-file") task.Quiet = cmd.Bool("quiet") task.DDLOnly = cmd.Bool("ddl-only") + task.Compare = cmd.String("compare") task.Ctx = context.Background() if scheduleEnabled && task.DDLOnly { return fmt.Errorf("scheduling is only supported when --ddl-only is false") } + if scheduleEnabled && task.Compare == diff.CompareStructure { + return fmt.Errorf("scheduling is not yet supported with --compare=structure") + } task.BlockSize = int(blockSizeInt) task.ConcurrencyFactor = cmd.Float64("concurrency-factor") task.MaxConnections = cmd.Int("max-connections") task.CompareUnitSize = cmd.Int("compare-unit-size") task.Output = cmd.String("output") + task.OutputExplicit = cmd.IsSet("output") task.OverrideBlockSize = cmd.Bool("override-block-size") if err := task.Validate(); err != nil { @@ -1474,8 +1483,8 @@ func StartSchedulerCLI(_ context.Context, cmd *cli.Command) error { signal.Notify(sighupCh, syscall.SIGHUP) defer signal.Stop(sighupCh) - // Start the API server once. It does not need to restart on reload because - // it handles on-demand requests rather than reading scheduled job config. + // Start the API server once: it serves on-demand requests rather than + // reading scheduled job config, so a reload never needs to restart it. var apiServer *server.APIServer if runAPI { if ok, apiErr := canStartAPIServer(cfg); ok { @@ -1509,7 +1518,7 @@ func StartSchedulerCLI(_ context.Context, cmd *cli.Command) error { return schedulerReloadLoop(runCtx, sighupCh, apiServer) } -// schedulerReloadLoop is the heart of the SIGHUP feature. +// schedulerReloadLoop implements SIGHUP-triggered config reload. // // Design: // 1. Build jobs from the current config and start the gocron scheduler. @@ -1569,10 +1578,9 @@ func schedulerReloadLoop( return nil case err := <-schedDone: - // The scheduler exited on its own – without being told to via - // schedCancel. This is unexpected (RunJobs normally blocks until - // its context is canceled). Treat a real error as fatal; a nil - // or Canceled result means it exited cleanly and we just stop. + // The scheduler exited on its own, not via schedCancel — unexpected, + // since RunJobs normally blocks until canceled. Treat a real error + // as fatal; nil or Canceled means it exited cleanly. schedCancel() if err != nil && !errors.Is(err, context.Canceled) { return err diff --git a/internal/consistency/diff/diff_summary.go b/internal/consistency/diff/diff_summary.go index bca6a0ed..a21b9e2f 100644 --- a/internal/consistency/diff/diff_summary.go +++ b/internal/consistency/diff/diff_summary.go @@ -20,9 +20,9 @@ import ( // MissingTableInfo records a table that was not found on every node. type MissingTableInfo struct { - Table string // schema-qualified table name - PresentOn []string // node names where the table exists - MissingFrom []string // node names where the table does not exist + Table string `json:"table"` // schema-qualified table name + PresentOn []string `json:"present_on"` // node names where the table exists + MissingFrom []string `json:"missing_from"` // node names where the table does not exist } // FailedTableInfo records a table whose diff failed along with the reason. diff --git a/internal/consistency/diff/schema_diff.go b/internal/consistency/diff/schema_diff.go index a60dc040..16bf3f80 100644 --- a/internal/consistency/diff/schema_diff.go +++ b/internal/consistency/diff/schema_diff.go @@ -15,6 +15,7 @@ import ( "bufio" "context" "encoding/json" + "errors" "fmt" "maps" "os" @@ -26,6 +27,8 @@ import ( "github.com/google/uuid" "github.com/jackc/pgx/v5/pgxpool" "github.com/pgedge/ace/db/queries" + "github.com/pgedge/ace/internal/consistency/schema" + "github.com/pgedge/ace/internal/consistency/scope" "github.com/pgedge/ace/internal/infra/db" utils "github.com/pgedge/ace/pkg/common" "github.com/pgedge/ace/pkg/config" @@ -34,18 +37,41 @@ import ( "github.com/pgedge/ace/pkg/types" ) +// CompareData and CompareStructure are the two values SchemaDiffCmd.Compare +// accepts. CompareData is the default, preserving schema-diff's existing +// per-table data-diff behaviour. +const ( + CompareData = "data" + CompareStructure = "structure" +) + type SchemaDiffCmd struct { types.Task - ClusterName string - DBName string - SchemaName string - Nodes string - Quiet bool - SkipTables string - SkipFile string - DDLOnly bool - skipTablesList []string + ClusterName string + DBName string + SchemaName string + Nodes string + Quiet bool + SkipTables string + SkipFile string + DDLOnly bool + // Compare selects what schema-diff compares: CompareData (the default, + // per-table data diff) or CompareStructure (structural comparison via + // internal/consistency/schema). + Compare string + + skipTablesList []string + // checksRun records that RunChecks already ran for this task, so + // SchemaTableDiff does not repeat the scope-resolution query and its + // logging. + // + // It memoises tableList/missingTables, which describe the cluster as it + // was when RunChecks ran, so it is scoped to one execution: every entry + // point that starts a run clears it first. Leaving it set across runs + // would silently compare a stale table list — the schema-diff scheduler + // hands the same command back for each fire. + checksRun bool tableList []string missingTables []MissingTableInfo nodeList []string @@ -57,6 +83,13 @@ type SchemaDiffCmd struct { BlockSize int CompareUnitSize int Output string + // OutputExplicit records whether the user actually passed --output, as + // opposed to it carrying the flag's own default. --compare=structure + // needs this distinction because "json" is that default: without it, + // every run - including one that never mentioned --output - would look + // like a request for the JSON rendering, silently changing what an + // interactive run prints. See schemaStructureDiff. + OutputExplicit bool TableFilter string OverrideBlockSize bool Ctx context.Context @@ -157,6 +190,23 @@ func (c *SchemaDiffCmd) parseSkipList() error { return nil } +// resolveCompareMode settles c.Compare before anything else runs. +// --ddl-only keeps selecting the existing table/view/function/index +// name-only diff (schemaObjectDiff). --compare=structure requests the +// symmetric, per-table structural comparison from +// internal/consistency/schema. If both are given, --compare=structure +// wins as the more specific request. +func (c *SchemaDiffCmd) resolveCompareMode() error { + if c.Compare == "" { + c.Compare = CompareData + return nil + } + if c.Compare != CompareData && c.Compare != CompareStructure { + return fmt.Errorf("invalid --compare value %q: must be %q or %q", c.Compare, CompareData, CompareStructure) + } + return nil +} + func (c *SchemaDiffCmd) Validate() error { if c.ClusterName == "" { return fmt.Errorf("cluster name is required") @@ -164,6 +214,12 @@ func (c *SchemaDiffCmd) Validate() error { if c.SchemaName == "" { return fmt.Errorf("schema name is required") } + if err := c.resolveCompareMode(); err != nil { + return err + } + if c.Compare == CompareStructure && c.OutputExplicit && !strings.EqualFold(c.Output, "json") { + return fmt.Errorf("--output=%s is not supported with --compare=structure: structure mode has no per-table diff files to render, only findings - use --output=json or omit --output", c.Output) + } nodeList, err := utils.ParseNodes(c.Nodes) if err != nil { @@ -211,6 +267,7 @@ func (c *SchemaDiffCmd) RunChecks(skipValidation bool) error { // Query tables from every node and build a union. nodeNames := make([]string, 0, len(c.clusterNodes)) tablePresence := make(map[string]map[string]bool) // table -> {nodeName: true} + quotedOf := make(map[string]string) // raw identifier -> quote_ident() form for _, nodeInfo := range c.clusterNodes { nodeName := nodeInfo["Name"].(string) @@ -240,10 +297,28 @@ func (c *SchemaDiffCmd) RunChecks(skipValidation bool) error { return fmt.Errorf("schema %s not found on node %s", c.SchemaName, nodeName) } - tables, err := queries.GetTablesInSchema(c.Ctx, pool, c.SchemaName) + resolved, err := scope.SchemaProvider{SchemaName: c.SchemaName}.Resolve(c.Ctx, pool) if err != nil { pool.Close() - return fmt.Errorf("could not get tables in schema on node %s: %w", nodeName, err) + return fmt.Errorf("could not resolve tables in schema on node %s: %w", nodeName, err) + } + tables := make([]string, 0, len(resolved.Tables)) + for _, qn := range resolved.Tables { + tables = append(tables, qn.Table) + } + + // Quote this node's own names while its connection is still open, + // so a table reported as missing is spelled the way the structural + // findings below spell one: schema."odd.name", not the ambiguous + // schema.odd.name. Every node renders a given name identically, so + // merging each node's answers is safe, and a name that cannot be + // quoted is printed raw rather than failing the run. + if quoted, qerr := queries.QuoteIdentifiers(c.Ctx, pool, append([]string{c.SchemaName}, tables...)); qerr == nil { + for raw, q := range quoted { + quotedOf[raw] = q + } + } else if !c.Quiet { + logger.Info("could not quote identifiers for display on node %s (names will be printed unquoted): %v", nodeName, qerr) } foreign, ferr := queries.GetForeignTablesInSchema(c.Ctx, pool, c.SchemaName) @@ -272,6 +347,28 @@ func (c *SchemaDiffCmd) RunChecks(skipValidation bool) error { } } + // For --compare=structure, a table named by --skip-tables/--skip-file + // must be left out of missing-table reporting too, not only out of the + // per-table comparison. Otherwise a table the user explicitly excluded + // would still show up as "missing on some nodes" and still force + // schema.ExitIncompatible in schemaStructureDiff. This check must run + // here, before schema.Qualify turns the raw table name into its + // quoted, schema-qualified display form: c.skipTablesList holds the + // raw, unqualified form (see parseSkipList), and matching against the + // display form would miss any name that needs quoting. + // + // compare=data keeps its old behavior: a table missing on some nodes is + // still reported there even if --skip-tables named it, since that mode + // only uses the skip list to leave a table out of the per-table data + // diff (see the skip check around the tableList loop below), not to + // hide the fact that its presence is asymmetric across nodes. + skipForMissingReport := make(map[string]bool, len(c.skipTablesList)) + if c.Compare == CompareStructure { + for _, t := range c.skipTablesList { + skipForMissingReport[t] = true + } + } + // Partition into common (all nodes) vs partial (some nodes). var commonTables []string var missingTables []MissingTableInfo @@ -279,6 +376,9 @@ func (c *SchemaDiffCmd) RunChecks(skipValidation bool) error { if len(presence) == len(nodeNames) { commonTables = append(commonTables, table) } else { + if skipForMissingReport[table] { + continue + } var presentOn, missingFrom []string for _, n := range nodeNames { if presence[n] { @@ -288,7 +388,7 @@ func (c *SchemaDiffCmd) RunChecks(skipValidation bool) error { } } missingTables = append(missingTables, MissingTableInfo{ - Table: fmt.Sprintf("%s.%s", c.SchemaName, table), + Table: schema.Qualify(quotedOf, c.SchemaName, table), PresentOn: presentOn, MissingFrom: missingFrom, }) @@ -302,10 +402,14 @@ func (c *SchemaDiffCmd) RunChecks(skipValidation bool) error { c.tableList = commonTables c.missingTables = missingTables - if len(c.tableList) == 0 && len(c.missingTables) == 0 { + // An empty schema is a finding for --compare=data but not for + // --compare=structure: nodes agreeing on an empty schema should exit 0, + // not the same code an unreachable node gets. + if len(c.tableList) == 0 && len(c.missingTables) == 0 && c.Compare != CompareStructure { return fmt.Errorf("no tables found in schema %s", c.SchemaName) } + c.checksRun = true return nil } @@ -409,10 +513,257 @@ func (task *SchemaDiffCmd) schemaObjectDiff() error { return nil } +// schemaStructureDiff is --compare=structure's implementation. It compares +// every common table's actual structure - columns, replica identity, +// constraints - using the same internal/consistency/schema.CollectSnapshot +// / Compare pair that table-diff's preflight uses to explain a mismatch, +// so both call sites share one comparison layer. +// +// Every pair of participating nodes is compared (up to three, per +// Validate's node-count limit): with three nodes, "A matches B" does not +// imply "B matches C". +// +// Tables present on only some nodes were already found by RunChecks +// (task.missingTables) and are reported separately here: CollectSnapshot +// has no way to be told a table does not exist on a node, so running it on +// one would surface every column as individually absent instead of one +// clear line naming the table. +// +// --skip-tables/--skip-file apply here exactly as they do to the default +// per-table data diff: a listed table is excluded from comparison (and from +// the exit code) entirely. task.tableList itself is left untouched - +// RunChecks built it once for the whole run - so this function derives its +// own filtered compareTables instead. A table missing on some nodes is +// excluded the same way: RunChecks already leaves a skipped table out of +// task.missingTables (for this mode only), so it does not appear in the +// report and does not force schema.ExitIncompatible. +// +// --output=json switches what is printed from the prose report to a +// StructureDiffReport, for a script that wants the findings structured +// rather than parsed out of text. This only happens when --output was +// actually given (task.OutputExplicit): "json" is that flag's own default +// value, so without this guard every run - including one that never +// mentioned --output - would silently switch its default console output. +type StructureDiffReport struct { + Schema string `json:"schema"` + Nodes []string `json:"nodes"` + MissingTables []MissingTableInfo `json:"missing_tables,omitempty"` + Comparisons []StructureComparisonReport `json:"comparisons"` + // ExitCode is the same code the process itself exits with (one of + // schema.ExitIdentical .. schema.ExitIncompatible), repeated here so a + // script reading only stdout does not also have to inspect the process's + // exit status. + ExitCode int `json:"exit_code"` +} + +// StructureComparisonReport is one node pair's findings within a +// StructureDiffReport. An empty Divergences means this pair's structure +// matched exactly, over whatever tables ended up in scope. +type StructureComparisonReport struct { + NodeA string `json:"node_a"` + NodeB string `json:"node_b"` + Divergences []schema.Divergence `json:"divergences"` +} + +// schemaStructureDiff runs the --compare=structure mode: it reads one +// structural snapshot per node, compares the nodes, prints what differs, and +// reports the worst rank it found through the process's exit code. +// +// Every node is compared with every other node, not with one chosen +// reference, because which node is right is not something this command can +// decide. Findings are counted per distinct object and property rather than +// per node pair, so one drifted column is one finding however many pairs saw +// it. +// +// Tables named by --skip-tables/--skip-file are dropped here, after +// RunChecks has already settled which tables every node has; missing tables +// are reported from what RunChecks recorded. The return is nil when nothing +// differs, and otherwise a utils.ExitCodeError carrying that worst rank's +// code, which is what turns a structural difference into an exit status a +// script can act on. +func (task *SchemaDiffCmd) schemaStructureDiff() error { + type nodeConn struct { + name string + pool *pgxpool.Pool + } + + var conns []nodeConn + defer func() { + for _, c := range conns { + c.pool.Close() + } + }() + + for _, nodeInfo := range task.clusterNodes { + nodeName, _ := nodeInfo["Name"].(string) + if !utils.Contains(task.nodeList, nodeName) { + continue + } + + nodeWithDBInfo := make(map[string]any) + maps.Copy(nodeWithDBInfo, nodeInfo) + utils.ApplyDatabaseCredentials(nodeWithDBInfo, task.database) + if portVal, ok := nodeWithDBInfo["Port"]; ok { + if portFloat, isFloat := portVal.(float64); isFloat { + nodeWithDBInfo["Port"] = strconv.Itoa(int(portFloat)) + } + } + + pool, err := auth.GetClusterNodeConnection(task.Ctx, nodeWithDBInfo, auth.ConnectionOptions{PoolSize: task.MaxConnections}) + if err != nil { + return fmt.Errorf("could not connect to node %s: %w", nodeName, err) + } + conns = append(conns, nodeConn{name: nodeName, pool: pool}) + } + + if len(conns) < 2 { + return fmt.Errorf("schema-diff --compare=structure needs at least two reachable nodes") + } + + compareTables := task.tableList + if len(task.skipTablesList) > 0 { + skip := make(map[string]bool, len(task.skipTablesList)) + for _, t := range task.skipTablesList { + skip[t] = true + } + var skipped []string + compareTables = make([]string, 0, len(task.tableList)) + for _, t := range task.tableList { + if skip[t] { + skipped = append(skipped, t) + continue + } + compareTables = append(compareTables, t) + } + if len(skipped) > 0 && !task.Quiet { + logger.Info("Skipping %d table(s) excluded by --skip-tables/--skip-file in schema %s: %s", + len(skipped), task.SchemaName, strings.Join(skipped, ", ")) + } + } + + snapshots := make(map[string]schema.Snapshot, len(conns)) + for _, c := range conns { + snap, err := schema.CollectSnapshot(task.Ctx, c.pool, c.name, task.SchemaName, compareTables) + if err != nil { + return fmt.Errorf("collecting structure snapshot on node %s: %w", c.name, err) + } + snapshots[c.name] = snap + } + + worst := schema.ExitIdentical + // Findings are counted per distinct (object, property), not summed over + // node pairs: with three nodes, one column that drifted on node 3 is + // found twice, by n1-vs-n3 and n2-vs-n3, and reporting "2 divergences" + // for one drifted column reads as two problems. + distinctFindings := make(map[string]bool) + var report strings.Builder + comparisons := make([]StructureComparisonReport, 0, len(conns)*(len(conns)-1)/2) + + // Tables missing from some nodes are reported first, since a schema + // present on only one node should not open with "(no structural + // differences)". + if len(task.missingTables) > 0 { + report.WriteString("=== tables missing on some nodes ===\n") + for _, mt := range task.missingTables { + fmt.Fprintf(&report, " - %s: present on %s, missing from %s\n", + mt.Table, strings.Join(mt.PresentOn, ", "), strings.Join(mt.MissingFrom, ", ")) + } + if schema.ExitIncompatible > worst { + worst = schema.ExitIncompatible + } + } + + for i := 0; i < len(conns); i++ { + for j := i + 1; j < len(conns); j++ { + a, b := conns[i].name, conns[j].name + divs := schema.Compare(task.SchemaName, compareTables, snapshots[a], snapshots[b]) + comparisons = append(comparisons, StructureComparisonReport{NodeA: a, NodeB: b, Divergences: divs}) + for _, d := range divs { + // FindingKey, not a key built here: a table's constraint + // findings share one Object+Kind+Property and would + // otherwise collapse into a single count. + distinctFindings[d.FindingKey()] = true + } + if code := schema.WorstExitCode(divs); code > worst { + worst = code + } + + fmt.Fprintf(&report, "=== %s vs %s ===\n", a, b) + switch { + case len(compareTables) == 0 && len(task.tableList) > 0: + // Every common table was named by --skip-tables/--skip-file. + report.WriteString(" (every common table was excluded by --skip-tables/--skip-file)\n") + case len(compareTables) == 0 && len(task.missingTables) == 0: + // The schema exists on both nodes and is empty on both. That + // is agreement, not a failure to compare, and it exits 0. + report.WriteString(" (schema is empty on every node)\n") + case len(compareTables) == 0: + // No common tables exist to compare. + report.WriteString(" (no tables in common to compare)\n") + case len(divs) == 0: + fmt.Fprintf(&report, " (no structural differences across %d table(s))\n", len(compareTables)) + default: + report.WriteString(schema.FormatDivergences(divs)) + report.WriteString("\n") + } + } + } + + if task.OutputExplicit && strings.EqualFold(task.Output, "json") { + nodeNames := make([]string, len(conns)) + for i, c := range conns { + nodeNames[i] = c.name + } + encoded, err := json.MarshalIndent(StructureDiffReport{ + Schema: task.SchemaName, + Nodes: nodeNames, + MissingTables: task.missingTables, + Comparisons: comparisons, + ExitCode: worst, + }, "", " ") + if err != nil { + return fmt.Errorf("could not marshal structure diff report to json: %w", err) + } + fmt.Println(string(encoded)) + } else { + fmt.Print(report.String()) + } + + if worst == schema.ExitIdentical { + if !task.Quiet { + logger.Info("schema structure diff: schema %s is identical across %d node(s)", task.SchemaName, len(conns)) + } + return nil + } + + summary := fmt.Sprintf("schema structure diff: %d divergence(s) found in schema %s across %d compared table(s)", + len(distinctFindings), task.SchemaName, len(compareTables)) + switch { + case len(compareTables) == 0 && len(task.tableList) > 0: + summary = fmt.Sprintf("schema structure diff: every common table in schema %s was excluded by --skip-tables/--skip-file, so nothing could be compared", + task.SchemaName) + case len(compareTables) == 0: + summary = fmt.Sprintf("schema structure diff: no table in schema %s exists on every node, so nothing could be compared", + task.SchemaName) + } + if len(task.missingTables) > 0 { + summary += fmt.Sprintf("; %d table(s) missing on some node(s)", len(task.missingTables)) + } + + return &utils.ExitCodeError{Code: worst, Err: errors.New(summary)} +} + func (task *SchemaDiffCmd) SchemaTableDiff() (err error) { - if err := task.RunChecks(false); err != nil { - return err + // The caller may already have run the checks - internal/cli does, to + // validate before committing to a run - so they are not repeated here. + // Whoever ran them, the resolved scope belongs to this run only, and is + // released with it: see checksRun. + if !task.checksRun { + if err := task.RunChecks(false); err != nil { + return err + } } + defer func() { task.checksRun = false }() startTime := time.Now() @@ -439,6 +790,7 @@ func (task *SchemaDiffCmd) SchemaTableDiff() (err error) { ctx := map[string]any{ "schema": task.SchemaName, + "compare": task.Compare, "ddl_only": task.DDLOnly, "table_filter": task.TableFilter, "tables_total": len(task.tableList), @@ -472,8 +824,13 @@ func (task *SchemaDiffCmd) SchemaTableDiff() (err error) { task.Task.FinishedAt = finishedAt task.Task.TimeTaken = finishedAt.Sub(startTime).Seconds() + // A found difference is a result, not a failure: --compare=structure + // reports it via ExitCodeError so the task store can distinguish + // "the schemas differ" from "the run broke". + var divergence *utils.ExitCodeError status := taskstore.StatusFailed - if err == nil { + switch { + case err == nil, errors.As(err, &divergence): status = taskstore.StatusCompleted } task.Task.TaskStatus = status @@ -484,6 +841,7 @@ func (task *SchemaDiffCmd) SchemaTableDiff() (err error) { "tables_diffed": tablesProcessed, "tables_failed": tablesFailed, "tables_skipped": len(skippedTables), + "compare": task.Compare, "ddl_only": task.DDLOnly, } if len(failedTables) > 0 { @@ -493,7 +851,13 @@ func (task *SchemaDiffCmd) SchemaTableDiff() (err error) { } ctx["failed_tables"] = names } - if err != nil { + switch { + case divergence != nil: + // Keyed apart from "error" so a reader can tell "the + // schemas differ" from "the run broke". + ctx["divergence"] = divergence.Error() + ctx["exit_code"] = divergence.Code + case err != nil: ctx["error"] = err.Error() } @@ -520,6 +884,10 @@ func (task *SchemaDiffCmd) SchemaTableDiff() (err error) { } }() + if task.Compare == CompareStructure { + return task.schemaStructureDiff() + } + if task.DDLOnly { return task.schemaObjectDiff() } @@ -604,11 +972,13 @@ func (task *SchemaDiffCmd) CloneForSchedule(ctx context.Context) *SchemaDiffCmd clone.SkipFile = task.SkipFile clone.Quiet = task.Quiet clone.DDLOnly = task.DDLOnly + clone.Compare = task.Compare clone.BlockSize = task.BlockSize clone.ConcurrencyFactor = task.ConcurrencyFactor clone.MaxConnections = task.MaxConnections clone.CompareUnitSize = task.CompareUnitSize clone.Output = task.Output + clone.OutputExplicit = task.OutputExplicit clone.TableFilter = task.TableFilter clone.OverrideBlockSize = task.OverrideBlockSize clone.SkipDBUpdate = task.SkipDBUpdate diff --git a/internal/consistency/diff/schema_diff_test.go b/internal/consistency/diff/schema_diff_test.go index f454a2ab..1997b4fd 100644 --- a/internal/consistency/diff/schema_diff_test.go +++ b/internal/consistency/diff/schema_diff_test.go @@ -12,10 +12,13 @@ package diff import ( + "encoding/json" "os" "path/filepath" "strings" "testing" + + "github.com/pgedge/ace/internal/consistency/schema" ) // writeSkipFile is a helper that writes lines to a temp file and returns its path. @@ -259,3 +262,185 @@ func TestParseSkipList_EmptyTableAfterSchema(t *testing.T) { t.Errorf("error = %q, want it to mention missing table name", err.Error()) } } + +// TestValidate_StructureModeRejectsExplicitHTMLOutput verifies that +// --output=html is rejected for --compare=structure when the user actually +// passed --output: structure mode has no per-table diff files to render as +// html, only findings. +func TestValidate_StructureModeRejectsExplicitHTMLOutput(t *testing.T) { + cmd := &SchemaDiffCmd{ + ClusterName: "c1", + SchemaName: "public", + Nodes: "n1,n2", + Compare: CompareStructure, + Output: "html", + OutputExplicit: true, + } + err := cmd.Validate() + if err == nil { + t.Fatal("expected an error for --output=html with --compare=structure, got nil") + } + if !strings.Contains(err.Error(), "not supported with --compare=structure") { + t.Errorf("error = %q, want it to mention --compare=structure", err.Error()) + } +} + +// TestValidate_StructureModeRejectsAnyExplicitNonJSONOutput verifies the +// guard is not special-cased to "html" alone: any explicit value other than +// "json" is rejected, matching the docs' claim that structure mode only +// accepts json. +func TestValidate_StructureModeRejectsAnyExplicitNonJSONOutput(t *testing.T) { + cmd := &SchemaDiffCmd{ + ClusterName: "c1", + SchemaName: "public", + Nodes: "n1,n2", + Compare: CompareStructure, + Output: "xml", + OutputExplicit: true, + } + err := cmd.Validate() + if err == nil { + t.Fatal("expected an error for --output=xml with --compare=structure, got nil") + } + if !strings.Contains(err.Error(), "not supported with --compare=structure") { + t.Errorf("error = %q, want it to mention --compare=structure", err.Error()) + } +} + +// TestValidate_StructureModeAllowsDefaultOutputValue verifies the guard only +// fires when the user actually passed --output: the flag's own default value +// reaching Output with OutputExplicit left false must not trip it, or every +// --compare=structure run that never mentions --output would fail. +func TestValidate_StructureModeAllowsDefaultOutputValue(t *testing.T) { + cmd := &SchemaDiffCmd{ + ClusterName: "c1", + SchemaName: "public", + Nodes: "n1,n2", + Compare: CompareStructure, + Output: "html", // the flag's default value, not user-chosen here + // OutputExplicit intentionally left false. + } + if err := cmd.Validate(); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestValidate_StructureModeAllowsExplicitJSONOutput verifies the one +// explicit value structure mode does support. +func TestValidate_StructureModeAllowsExplicitJSONOutput(t *testing.T) { + cmd := &SchemaDiffCmd{ + ClusterName: "c1", + SchemaName: "public", + Nodes: "n1,n2", + Compare: CompareStructure, + Output: "json", + OutputExplicit: true, + } + if err := cmd.Validate(); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestValidate_DataModeIgnoresOutputCompareGuard verifies the guard is +// specific to --compare=structure: the default per-table data diff has +// always accepted --output=html and must keep doing so. +func TestValidate_DataModeIgnoresOutputCompareGuard(t *testing.T) { + cmd := &SchemaDiffCmd{ + ClusterName: "c1", + SchemaName: "public", + Nodes: "n1,n2", + Compare: CompareData, + Output: "html", + OutputExplicit: true, + } + if err := cmd.Validate(); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +// TestStructureDiffReport_JSONFieldNames pins the --output=json wire format: +// a script parsing this depends on these exact key names, so a rename here +// would be a breaking change that should show up as a failing test, not +// silently ship. +func TestStructureDiffReport_JSONFieldNames(t *testing.T) { + report := StructureDiffReport{ + Schema: "public", + Nodes: []string{"n1", "n2"}, + MissingTables: []MissingTableInfo{ + {Table: "public.foo", PresentOn: []string{"n1"}, MissingFrom: []string{"n2"}}, + }, + Comparisons: []StructureComparisonReport{ + { + NodeA: "n1", NodeB: "n2", + Divergences: []schema.Divergence{ + { + Object: "public.t.x", Kind: "column", Property: "type", + NodeA: "n1", NodeB: "n2", ValueOnA: "integer", ValueOnB: "bigint", + Rank: schema.RankNarrowed, NarrowSide: "n1", + }, + }, + }, + }, + ExitCode: schema.ExitNarrowed, + } + + encoded, err := json.Marshal(report) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + var decoded map[string]any + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + for _, key := range []string{"schema", "nodes", "missing_tables", "comparisons", "exit_code"} { + if _, ok := decoded[key]; !ok { + t.Errorf("encoded report is missing top-level key %q: %s", key, encoded) + } + } + + comparisons, _ := decoded["comparisons"].([]any) + if len(comparisons) != 1 { + t.Fatalf("comparisons = %v, want 1 entry", comparisons) + } + comparison, _ := comparisons[0].(map[string]any) + for _, key := range []string{"node_a", "node_b", "divergences"} { + if _, ok := comparison[key]; !ok { + t.Errorf("comparison entry is missing key %q: %s", key, encoded) + } + } + + divs, _ := comparison["divergences"].([]any) + if len(divs) != 1 { + t.Fatalf("divergences = %v, want 1 entry", divs) + } + div, _ := divs[0].(map[string]any) + for _, key := range []string{ + "object", "kind", "property", "node_a", "node_b", + "value_on_a", "value_on_b", "rank", "narrow_side", + } { + if _, ok := div[key]; !ok { + t.Errorf("divergence entry is missing key %q: %s", key, encoded) + } + } +} + +// TestStructureDiffReport_OmitsEmptyMissingTables verifies missing_tables is +// left out entirely (not printed as null or []) when nothing was missing - +// the common case, and the one that should read as clean JSON. +func TestStructureDiffReport_OmitsEmptyMissingTables(t *testing.T) { + report := StructureDiffReport{ + Schema: "public", + Nodes: []string{"n1", "n2"}, + Comparisons: []StructureComparisonReport{{NodeA: "n1", NodeB: "n2"}}, + ExitCode: schema.ExitIdentical, + } + encoded, err := json.Marshal(report) + if err != nil { + t.Fatalf("marshal: %v", err) + } + if strings.Contains(string(encoded), "missing_tables") { + t.Errorf("expected missing_tables to be omitted when empty, got: %s", encoded) + } +} diff --git a/internal/consistency/diff/spock_diff.go b/internal/consistency/diff/spock_diff.go index 1fd933b9..da25269c 100644 --- a/internal/consistency/diff/spock_diff.go +++ b/internal/consistency/diff/spock_diff.go @@ -25,6 +25,7 @@ import ( "github.com/google/uuid" "github.com/jackc/pgx/v5/pgxpool" "github.com/pgedge/ace/db/queries" + "github.com/pgedge/ace/internal/consistency/topology" "github.com/pgedge/ace/internal/infra/db" utils "github.com/pgedge/ace/pkg/common" "github.com/pgedge/ace/pkg/logger" @@ -32,13 +33,13 @@ import ( "github.com/pgedge/ace/pkg/types" ) -// SpockNodeConfig aggregates all spock configuration for a single node. -type SpockNodeConfig struct { - NodeName string `json:"node_name"` - Subscriptions []types.SpockSubscription `json:"subscriptions"` - RepSetInfo []types.SpockRepSetInfo `json:"rep_set_info"` - Hints []string `json:"hints"` -} +// SpockNodeConfig aggregates spock configuration for a single node. +// +// It aliases topology.NodeConfig, which holds the single implementation of +// reading a node's Spock topology (subscriptions, replication sets, hints) +// for reuse by anything needing node replication relationships, while +// keeping this package's public surface and tests unchanged. +type SpockNodeConfig = topology.NodeConfig // SpockDiffTask defines the task for comparing spock metadata across nodes. type SpockDiffTask struct { @@ -328,60 +329,12 @@ func (t *SpockDiffTask) ExecuteTask() (err error) { for _, nodeName := range nodeNames { pool := pools[nodeName] - config := SpockNodeConfig{NodeName: nodeName, Hints: []string{}} logger.Debug("Fetching Spock config for node: %s", nodeName) - // Fetch node and subscription info - nodeInfos, err := queries.GetSpockNodeAndSubInfo(t.Ctx, pool) - if err != nil { - return fmt.Errorf("querying spock.node and spock.subscription on node %s failed: %w", nodeName, err) - } - - if len(nodeInfos) > 0 { - config.NodeName = nodeInfos[0].NodeName - for _, ni := range nodeInfos { - sub := types.SpockSubscription{} - if ni.SubName != "" { - sub.SubName = ni.SubName - sub.ProviderNode = ni.SubOriginName - sub.SubEnabled = ni.SubEnabled - sub.ReplicationSets = ni.SubReplicationSets - if ni.SubOriginName == "" { - hint := fmt.Sprintf("Subscription '%s' has an unresolved origin node; its reciprocal peer cannot be determined and it may be reported below as a missing subscription.", sub.SubName) - if !utils.Contains(config.Hints, hint) { - config.Hints = append(config.Hints, hint) - } - } - if len(ni.SubReplicationSets) == 0 { - hint := fmt.Sprintf("Subscription '%s' has no replication sets.", sub.SubName) - if !utils.Contains(config.Hints, hint) { - config.Hints = append(config.Hints, hint) - } - } - } - config.Subscriptions = append(config.Subscriptions, sub) - } - } else { - config.Hints = append(config.Hints, "Hint: No subscriptions have been created on this node.") - } - - // Fetch replication set info - repRows, err := queries.GetSpockRepSetInfo(t.Ctx, pool) + config, err := topology.FetchSpockNodeConfig(t.Ctx, pool, nodeName) if err != nil { - return fmt.Errorf("querying spock.tables on node %s failed: %w", nodeName, err) - } - - config.RepSetInfo = repRows - - var tablesInRepSets []string - for _, rs := range repRows { - if rs.SetName != "" { - tablesInRepSets = append(tablesInRepSets, rs.RelName...) - } - } - if len(repRows) > 0 && len(tablesInRepSets) == 0 { - config.Hints = append(config.Hints, "Hint: Tables not in replication set might not have primary keys, or you need to run repset-add-table.") + return err } allNodeConfigs[nodeName] = config @@ -515,10 +468,10 @@ func compareSubscriptions(c1, c2 SpockNodeConfig) types.SubscriptionDiff { n1Name := c1.NodeName n2Name := c2.NodeName - // A healthy pair requires n1 to subscribe from n2 and n2 from n1. Match on the - // provider node identity, not the subscription name (which users may override). - subsFromOnN1 := subscriptionsByProvider(c1.Subscriptions) - subsFromOnN2 := subscriptionsByProvider(c2.Subscriptions) + // A healthy pair requires n1 to subscribe from n2 and n2 from n1, matched + // by provider node identity since users may rename subscriptions. + subsFromOnN1 := topology.SubscriptionsByProvider(c1.Subscriptions) + subsFromOnN2 := topology.SubscriptionsByProvider(c2.Subscriptions) s1, n1SubsFromN2 := subsFromOnN1[n2Name] // subscription on n1 receiving from n2 s2, n2SubsFromN1 := subsFromOnN2[n1Name] // subscription on n2 receiving from n1 @@ -531,8 +484,8 @@ func compareSubscriptions(c1, c2 SpockNodeConfig) types.SubscriptionDiff { } if n1SubsFromN2 && n2SubsFromN1 { - // Compare order-insensitively without mutating the originals: these slices - // are shared with the SpockConfigs JSON output, which keeps DB order. + // Copy before sorting: these slices are shared with the SpockConfigs + // JSON output, which preserves DB order. sets1 := append([]string(nil), s1.ReplicationSets...) sets2 := append([]string(nil), s2.ReplicationSets...) sort.Strings(sets1) @@ -551,17 +504,6 @@ func compareSubscriptions(c1, c2 SpockNodeConfig) types.SubscriptionDiff { return diff } -// subscriptionsByProvider indexes subscriptions by the node they replicate from. -func subscriptionsByProvider(subs []types.SpockSubscription) map[string]types.SpockSubscription { - byProvider := make(map[string]types.SpockSubscription, len(subs)) - for _, s := range subs { - if s.ProviderNode != "" { - byProvider[s.ProviderNode] = s - } - } - return byProvider -} - func compareReplicationSets(c1, c2 SpockNodeConfig) types.ReplicationSetDiff { diff := types.ReplicationSetDiff{} diff --git a/internal/consistency/diff/table_diff.go b/internal/consistency/diff/table_diff.go index 457d4c6c..e35e2ea8 100644 --- a/internal/consistency/diff/table_diff.go +++ b/internal/consistency/diff/table_diff.go @@ -34,6 +34,7 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/pgedge/ace/db/queries" + "github.com/pgedge/ace/internal/consistency/schema" auth "github.com/pgedge/ace/internal/infra/db" utils "github.com/pgedge/ace/pkg/common" "github.com/pgedge/ace/pkg/config" @@ -108,14 +109,13 @@ type TableDiffTask struct { DiffResult types.DiffOutput diffMutex sync.Mutex - firstError error - firstErrorMu sync.Mutex - errorRecorded atomic.Bool + firstError error + firstErrorMu sync.Mutex + errorRecorded atomic.Bool // pairDiffRows enforces max_diff_rows per node pair, keyed by pairKey -> - // *atomic.Int64. A single shared counter would make the cap a budget split - // across all C(n,2) pairs on clusters with more than two nodes, truncating - // the report and requiring multiple repair passes. + // *atomic.Int64, so one pair's divergence cannot exhaust the row budget + // on clusters with more than two nodes. pairDiffRows sync.Map diffLimitTriggered atomic.Bool @@ -181,16 +181,15 @@ func (t *TableDiffTask) pairCounter(pairKey string) *atomic.Int64 { } // shouldStopPair returns true if enumeration for this node pair should cease, -// either because the pair reached max_diff_rows or because a node error has been -// recorded (circuit breaker that prevents OOM when a node starts failing). The -// row limit is per pair so one pair's divergence cannot exhaust another pair's -// budget on clusters with more than two nodes. +// either because the pair reached max_diff_rows or because a node error has +// been recorded (a circuit breaker that prevents OOM when a node starts +// failing). The row limit is per pair so one pair's divergence cannot +// exhaust another pair's budget on clusters with more than two nodes. // -// The cap is best-effort, not exact: this gate is checked before the diffMutex -// is taken, so several concurrent comparisons for the same pair can each pass it -// just under the limit and then append their batches, leaving the pair a few -// rows over max_diff_rows. That is acceptable for a report-size bound and -// matches the prior global-counter behaviour. +// The cap is best-effort: this gate is checked before diffMutex is taken, +// so concurrent comparisons for the same pair can each pass it just under +// the limit, leaving the pair a few rows over max_diff_rows. That is +// acceptable for a report-size bound. func (t *TableDiffTask) shouldStopPair(pairKey string) bool { if t.hasError() { return true @@ -201,7 +200,7 @@ func (t *TableDiffTask) shouldStopPair(pairKey string) bool { return t.pairCounter(pairKey).Load() >= t.MaxDiffRows } -// It's imperative for the caller to hold the diffMutex while calling this function +// incrementPairDiffRowsLocked requires the caller to hold diffMutex. func (t *TableDiffTask) incrementPairDiffRowsLocked(pairKey string, delta int) bool { if delta <= 0 || t.MaxDiffRows <= 0 { return false @@ -243,9 +242,8 @@ func (t *TableDiffTask) loadNodeOriginNames() error { // flatNodeOriginNames merges all per-node origin maps into a single map for // lookup purposes (e.g. resolving --against-origin). If the same roident -// appears on multiple nodes, the last one wins — this is acceptable because -// resolveAgainstOrigin is only used with spock (where roidents are global) -// or for user-facing name resolution where any match suffices. +// appears on multiple nodes, the last one wins: roidents are global under +// spock, and any match suffices for user-facing name resolution. func (t *TableDiffTask) flatNodeOriginNames() map[string]string { flat := make(map[string]string) for _, nodeMap := range t.NodeOriginNames { @@ -791,6 +789,8 @@ func (t *TableDiffTask) RunChecks(skipValidation bool) (err error) { } var cols, key []string + var refHostname string + var refNodeInfo map[string]any hostMap := make(map[string]string) schema := t.Schema @@ -856,10 +856,12 @@ func (t *TableDiffTask) RunChecks(skipValidation bool) (err error) { if len(cols) == 0 && len(key) == 0 { cols = currCols key = currKey + refHostname = hostname + refNodeInfo = nodeInfo } if !reflect.DeepEqual(currCols, cols) || !reflect.DeepEqual(currKey, key) { - return fmt.Errorf("table schemas don't match between nodes") + return t.diagnoseSchemaMismatch(schema, table, refNodeInfo, refHostname, cols, key, hostname, conn, currCols, currKey) } cols = currCols @@ -947,6 +949,53 @@ func (t *TableDiffTask) RunChecks(skipValidation bool) (err error) { return nil } +// diagnoseSchemaMismatch runs after RunChecks' cheap column/key-name +// comparison finds a mismatch. It names the actual difference by reusing +// internal/consistency/schema's structural comparison, the same one +// schema-diff itself uses. +// +// It re-reads both nodes' full structural descriptors in a fresh +// REPEATABLE READ transaction each, reconnecting to the reference node +// since RunChecks already closed that connection. This cost is paid only +// on the error path. +func (t *TableDiffTask) diagnoseSchemaMismatch( + schemaName, table string, + refNodeInfo map[string]any, refHostname string, refCols, refKey []string, + curHostname string, curConn *pgxpool.Pool, curCols, curKey []string, +) error { + fallback := fmt.Errorf("table schemas don't match between nodes %s and %s: columns %v vs %v, key %v vs %v", + refHostname, curHostname, refCols, curCols, refKey, curKey) + + refConn, err := auth.GetClusterNodeConnection(t.Ctx, refNodeInfo, t.connOpts()) + if err != nil { + return fmt.Errorf("%w (could not reconnect to %s for a detailed diff: %v)", fallback, refHostname, err) + } + defer refConn.Close() + + refSnap, err := schema.CollectSnapshot(t.Ctx, refConn, refHostname, schemaName, []string{table}) + if err != nil { + return fmt.Errorf("%w (could not collect a detailed diff from %s: %v)", fallback, refHostname, err) + } + curSnap, err := schema.CollectSnapshot(t.Ctx, curConn, curHostname, schemaName, []string{table}) + if err != nil { + return fmt.Errorf("%w (could not collect a detailed diff from %s: %v)", fallback, curHostname, err) + } + + divergences := schema.Compare(schemaName, []string{table}, refSnap, curSnap) + if len(divergences) == 0 { + // The name-only check disagreed but the structural comparison found + // nothing reportable; fall back to what table-diff itself saw. + return fallback + } + + // FormatDivergences, not a local copy of it: it decodes the packed + // values ("id, tenant", not "2:id6:tenant") and names the narrow side. + var b strings.Builder + fmt.Fprintf(&b, "table '%s.%s' schema differs between nodes %s and %s:\n", schemaName, table, refHostname, curHostname) + b.WriteString(schema.FormatDivergences(divergences)) + return errors.New(b.String()) +} + func (t *TableDiffTask) cleanupFilteredView() { if t.TableFilter == "" || t.FilteredViewName == "" || !t.FilteredViewCreated { return @@ -1405,7 +1454,6 @@ func (t *TableDiffTask) ExecuteTask() (err error) { newInitialRange := Range{Start: nil, End: firstOriginalStart} ranges = append([]Range{newInitialRange}, ranges...) } - // } logger.Debug("Created %d initial ranges to compare", len(ranges)) logger.Debug("Ranges: %v", ranges) @@ -1550,17 +1598,17 @@ func (t *TableDiffTask) ExecuteTask() (err error) { } diffWg.Add(1) go func(task RecursiveDiffTask) { - // Wait for a semaphore slot — blocks here if maxConcurrent goroutines - // are already doing diff work. This prevents OOM from unbounded fan-out. - // Uses select so that context cancellation unblocks the wait and avoids - // hanging diffWg.Wait() indefinitely. + // Wait for a semaphore slot; blocks if maxConcurrent goroutines + // are already doing diff work, preventing OOM from unbounded + // fan-out. select lets context cancellation unblock the wait + // so diffWg.Wait() cannot hang. select { case t.diffSem <- struct{}{}: // Got a slot — release it when this goroutine finishes. defer func() { <-t.diffSem }() case <-ctx.Done(): - // Context was cancelled while waiting for a slot. Decrement the - // WaitGroup so the caller's diffWg.Wait() can return. + // Context cancelled while waiting for a slot; decrement so + // diffWg.Wait() can return. diffWg.Done() diffBar.Increment() return @@ -1758,9 +1806,8 @@ func (t *TableDiffTask) generateSubRanges( if parentRange.End != nil { endVal := parentRange.End if len(t.Key) == 1 { - // In hashRange, the end is exclusive (<), but here for counting and splitting - // we use inclusive (<=) to match fetchRows. This is acceptable because - // we are splitting a mismatched range, and slight overlap is okay. + // Uses inclusive (<=) to match fetchRows; slight overlap from + // splitting a mismatched range is acceptable. conditions = append(conditions, fmt.Sprintf("%s <= $%d", quotedKeyCols[0], paramIdx)) args = append(args, endVal) paramIdx++ @@ -2002,8 +2049,7 @@ func (t *TableDiffTask) recursiveDiff( if len(subRanges) == 0 { logger.Debug("[%s vs %s] Range %v-%v could not be split further (generateSubRangesViaNtile returned empty). Treating as unit.", node1Name, node2Name, currentRange.Start, currentRange.End) - // Fallback: treat current range as the smallest unit and compare. - // Call self with current range but force small enough size + // Fallback: treat the range as the smallest unit and compare it. task.CurrentEstimatedBlockSize = finalCompareUnitSize newWg := &sync.WaitGroup{} newWg.Add(1) @@ -2075,17 +2121,17 @@ func (t *TableDiffTask) recursiveDiff( wg.Add(1) go func(sr Range, newEstimatedBlockSize int) { - // Wait for a semaphore slot — blocks here if maxConcurrent goroutines - // are already doing diff work. This prevents OOM from unbounded fan-out. - // Uses select so that context cancellation unblocks the wait and avoids - // hanging wg.Wait() indefinitely. + // Wait for a semaphore slot; blocks if maxConcurrent goroutines + // are already doing diff work, preventing OOM from unbounded + // fan-out. select lets context cancellation unblock the wait + // so wg.Wait() cannot hang. select { case t.diffSem <- struct{}{}: // Got a slot — release it when this goroutine finishes. defer func() { <-t.diffSem }() case <-ctx.Done(): - // Context was cancelled while waiting for a slot. Decrement the - // WaitGroup so the caller's wg.Wait() can return. + // Context cancelled while waiting for a slot; decrement so + // wg.Wait() can return. wg.Done() return } diff --git a/internal/consistency/schema/collect.go b/internal/consistency/schema/collect.go new file mode 100644 index 00000000..8b19eb61 --- /dev/null +++ b/internal/consistency/schema/collect.go @@ -0,0 +1,645 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package schema + +import ( + "context" + "fmt" + "sort" + "strconv" + "strings" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgxpool" + "github.com/pgedge/ace/db/queries" +) + +// CollectSnapshot reads every structural property this package knows how to +// compare, for the given tables, on one node. +// +// Everything is read inside a single REPEATABLE READ, read-only transaction: +// open it, read within it, done. What CollectSnapshot returns is what was +// true on this node at the moment the transaction started. +// +// schemaName is a single PostgreSQL namespace; tables must already be +// unqualified names within it. Multi-schema scopes are not handled yet. +func CollectSnapshot(ctx context.Context, pool *pgxpool.Pool, nodeName, schemaName string, tables []string) (Snapshot, error) { + snap := Snapshot{ + Node: nodeName, + Objects: make(map[ObjectID]Object), + TableColumns: make(map[ObjectID][]string), + } + if len(tables) == 0 { + return snap, nil + } + + tx, err := pool.BeginTx(ctx, pgx.TxOptions{ + IsoLevel: pgx.RepeatableRead, + AccessMode: pgx.ReadOnly, + }) + if err != nil { + return Snapshot{}, fmt.Errorf("could not open a REPEATABLE READ snapshot on %s: %w", nodeName, err) + } + // A read-only transaction that is never committed is always safe to roll + // back, including after a successful read: there is nothing to persist. + defer func() { _ = tx.Rollback(ctx) }() + + if err := pinDeparseSettings(ctx, tx, nodeName); err != nil { + return Snapshot{}, err + } + + columnsByTable, err := queries.GetColumnDescriptors(ctx, tx, schemaName, tables) + if err != nil { + return Snapshot{}, fmt.Errorf("reading column descriptors on %s: %w", nodeName, err) + } + keysByTable, err := queries.GetReplicaIdentityKey(ctx, tx, schemaName, tables) + if err != nil { + return Snapshot{}, fmt.Errorf("reading replica identity keys on %s: %w", nodeName, err) + } + constraintsByTable, err := queries.GetConstraintDescriptors(ctx, tx, schemaName, tables) + if err != nil { + return Snapshot{}, fmt.Errorf("reading constraint descriptors on %s: %w", nodeName, err) + } + partitionsByTable, err := queries.GetPartitionDescriptors(ctx, tx, schemaName, tables) + if err != nil { + return Snapshot{}, fmt.Errorf("reading partition descriptors on %s: %w", nodeName, err) + } + + domains, ranges, composites, enums, err := fetchReferencedTypeDescriptors(ctx, tx, nodeName, columnsByTable) + if err != nil { + return Snapshot{}, err + } + + // The database's own collation settings, which every column that does + // not name a collation silently inherits. Collected here rather than + // per column because that inheritance is invisible in pg_attribute - + // see GetDatabaseLocale. + locale, err := queries.GetDatabaseLocale(ctx, tx) + if err != nil { + return Snapshot{}, fmt.Errorf("reading database locale on %s: %w", nodeName, err) + } + + // Every identifier this snapshot will print is quoted in one round trip, + // up front. See QuoteIdentifiers' doc comment. + quotedOf, err := queries.QuoteIdentifiers(ctx, tx, identifiersToQuote(schemaName, tables, columnsByTable, domains, ranges, composites, enums)) + if err != nil { + return Snapshot{}, fmt.Errorf("quoting identifiers for display on %s: %w", nodeName, err) + } + + buildReferencedTypeObjects(quotedOf, domains, ranges, composites, enums, snap.Objects) + + localeObj := Object{Kind: "database", Name: locale.Name, Properties: []Property{ + {Name: "lc_collate", Value: locale.Collate}, + {Name: "lc_ctype", Value: locale.Ctype}, + {Name: "locale_provider", Value: locale.Provider}, + {Name: "locale", Value: locale.Locale}, + }} + sortProperties(localeObj.Properties) + snap.Objects[DatabaseID()] = localeObj + + for _, table := range tables { + // A table the catalog did not return was dropped between the caller + // resolving its scope and this snapshot. Writing no Object lets + // Compare report it once, as absent on the table, rather than once + // per column. partitionsByTable is the existence test: + // GetPartitionDescriptors filters on schema and name only, so it + // holds one row per relation that exists. + if _, exists := partitionsByTable[table]; !exists { + continue + } + + tableID := TableID(schemaName, table) + qualified := qualify(quotedOf, schemaName, table) + + // Table-level object: partition information only, written even when + // that is empty so a table that exists is never taken for a missing + // one. + tableObj := Object{Kind: "table", Name: qualified} + if p, ok := partitionsByTable[table]; ok { + tableObj.Properties = append(tableObj.Properties, + Property{Name: "partition_bound", Value: p.PartitionBound}, + Property{Name: "partition_key", Value: p.PartitionKey}, + ) + } + sortProperties(tableObj.Properties) + snap.Objects[tableID] = tableObj + + for _, c := range columnsByTable[table] { + colName := qualify(quotedOf, schemaName, table, c.Name) + snap.TableColumns[tableID] = append(snap.TableColumns[tableID], c.Name) + // A collation is identified the same way a type is, by + // (namespace, name); the provider and recorded version follow. + // A column with no collation of its own inherits the database's, + // which is compared once as the "database" Object rather than + // repeated here, since pg_attribute cannot show it. + collation := "" + if c.CollName != "" { + collation = fmt.Sprintf("%s.%s/%s/%s", + c.CollNamespace, c.CollName, c.CollProvider, c.CollVersion) + } + colObj := Object{ + Kind: "column", + Name: colName, + Properties: []Property{ + {Name: "type", Value: c.TypeText}, + // Type identity is (type_namespace, type_name, type_mod), + // portable across nodes and PostgreSQL versions — never + // the type's OID. See GetColumnDescriptors' doc comment. + {Name: "type_namespace", Value: c.TypeNamespace}, + {Name: "type_name", Value: c.TypeName}, + {Name: "type_kind", Value: c.TypeKind}, + {Name: "type_mod", Value: strconv.FormatInt(int64(c.TypeMod), 10)}, + {Name: "notnull", Value: strconv.FormatBool(c.NotNull)}, + {Name: "identity", Value: c.Identity}, + {Name: "generated", Value: c.Generated}, + {Name: "options", Value: c.Options}, + {Name: "collation", Value: collation}, + {Name: "default", Value: c.DefaultExpr}, + }, + } + sortProperties(colObj.Properties) + snap.Objects[ColumnID(schemaName, table, c.Name)] = colObj + } + + keyObj := Object{Kind: "key", Name: qualified} + if k, ok := keysByTable[table]; ok { + keyObj.Properties = []Property{ + {Name: "replica_identity", Value: k.ReplicaIdentity}, + // joinList, not a comma: a column named "a,b" would + // otherwise make the key (a,b) and the key ("a,b") + // indistinguishable. + {Name: "key_columns", Value: joinList(k.KeyColumns)}, + {Name: "key_opclasses", Value: joinList(k.KeyOpclasses)}, + } + } + sortProperties(keyObj.Properties) + snap.Objects[KeyID(schemaName, table)] = keyObj + + // Constraints are a set, not a keyed collection: PostgreSQL invents + // names for unnamed constraints, so there is no stable per-name slot + // to compare across nodes. All of a table's constraints live as + // same-named "constraint" properties on one Object, ordered by + // definition text via sortProperties. + constraintObj := Object{Kind: "constraint", Name: qualified} + for _, c := range constraintsByTable[table] { + constraintObj.Properties = append(constraintObj.Properties, Property{ + Name: "constraint", + Value: fmt.Sprintf("%s|deferrable=%t|validated=%t|%s", c.Type, c.Deferrable, c.Validated, c.Definition), + }) + } + sortProperties(constraintObj.Properties) + snap.Objects[ConstraintID(schemaName, table)] = constraintObj + } + + return snap, nil +} + +// fetchReferencedTypeDescriptors finds every domain, range, composite and +// enum type the compared columns depend on, however indirectly, and reads +// each one's full descriptor. +// +// This lets Compare notice that two columns declaring "the same" domain by +// name actually disagree about what it constrains (a different CHECK, a +// different base type) — comparing by name and definition, not by OID, +// keeps this stable across independently initdb'd nodes. +// +// Discovery walks the type graph to a fixed point rather than looking only +// at each column's own typtype, because most user-defined types are not +// reached in one step: an array type is a base type in its own right, so a +// "status[]" column names no enum at all, and an enum can sit inside a +// composite's attribute or under a chain of domains without any column +// mentioning it. See resolveTypeClosure and GetTypeReferences. +// +// The OIDs gathered here are this node's own, valid only for further +// catalog lookups on this connection — never sent for cross-node +// comparison. Only the portable fields the queries return (namespace/name/ +// kind/typmod strings, sorted constraint text, ordered labels) reach the +// Snapshot. Building Objects from these descriptors is +// buildReferencedTypeObjects' job; fetching is kept separate because every +// identifier they mention must be quoted before any Object is built (see +// identifiersToQuote). +func fetchReferencedTypeDescriptors(ctx context.Context, tx pgx.Tx, nodeName string, columnsByTable map[string][]queries.ColumnDescriptor) ( + domains map[uint32]queries.DomainDescriptor, + ranges map[uint32]queries.RangeDescriptor, + composites map[uint32]queries.CompositeDescriptor, + enums map[uint32]queries.EnumDescriptor, + err error, +) { + seed := make([]uint32, 0, len(columnsByTable)) + seen := make(map[uint32]bool) + for _, cols := range columnsByTable { + for _, c := range cols { + if seen[c.TypeOID] { + continue + } + seen[c.TypeOID] = true + seed = append(seed, c.TypeOID) + } + } + + kinds, err := resolveTypeClosure(ctx, tx, nodeName, seed) + if err != nil { + return nil, nil, nil, nil, err + } + + var domainOIDs, rangeOIDs, compositeOIDs, enumOIDs []uint32 + for _, oid := range sortedOIDs(kinds) { + switch kinds[oid] { + case "d": + domainOIDs = append(domainOIDs, oid) + case "r": + rangeOIDs = append(rangeOIDs, oid) + case "c": + compositeOIDs = append(compositeOIDs, oid) + case "e": + enumOIDs = append(enumOIDs, oid) + } + } + + domains, err = queries.GetDomainDescriptors(ctx, tx, domainOIDs) + if err != nil { + return nil, nil, nil, nil, fmt.Errorf("reading domain descriptors on %s: %w", nodeName, err) + } + ranges, err = queries.GetRangeDescriptors(ctx, tx, rangeOIDs) + if err != nil { + return nil, nil, nil, nil, fmt.Errorf("reading range descriptors on %s: %w", nodeName, err) + } + composites, err = queries.GetCompositeAttributes(ctx, tx, compositeOIDs) + if err != nil { + return nil, nil, nil, nil, fmt.Errorf("reading composite attributes on %s: %w", nodeName, err) + } + enums, err = queries.GetEnumLabels(ctx, tx, enumOIDs) + if err != nil { + return nil, nil, nil, nil, fmt.Errorf("reading enum labels on %s: %w", nodeName, err) + } + return domains, ranges, composites, enums, nil +} + +// resolveTypeClosure expands seed into every type reachable from it, +// returning each reachable type's typtype keyed by this node's OID. +// +// The walk is breadth-first and asks about a given OID exactly once, which +// is what makes it terminate: an OID is recorded as requested before the +// round that asks about it, so a type PostgreSQL declines to report (it +// should report all of them) cannot be re-queued forever. Since every OID +// is visited once, the number of rounds is bounded by the depth of the type +// graph — three or four in practice, an array of a domain over a composite +// being about as deep as real schemas go. +func resolveTypeClosure(ctx context.Context, tx pgx.Tx, nodeName string, seed []uint32) (map[uint32]string, error) { + kinds := make(map[uint32]string, len(seed)) + requested := make(map[uint32]bool, len(seed)) + + pending := make([]uint32, 0, len(seed)) + for _, oid := range seed { + if oid != 0 && !requested[oid] { + requested[oid] = true + pending = append(pending, oid) + } + } + + for len(pending) > 0 { + refs, err := queries.GetTypeReferences(ctx, tx, pending) + if err != nil { + return nil, fmt.Errorf("resolving referenced types on %s: %w", nodeName, err) + } + + var next []uint32 + for _, ref := range refs { + kinds[ref.OID] = ref.Kind + for _, dep := range ref.Refs { + if dep == 0 || requested[dep] { + continue + } + requested[dep] = true + next = append(next, dep) + } + } + // refs is a map, so next comes out in Go's map order; sorting keeps + // the query parameters, and anything a log or test reads from them, + // stable from run to run. + sort.Slice(next, func(i, j int) bool { return next[i] < next[j] }) + pending = next + } + + return kinds, nil +} + +// sortedOIDs returns m's keys in ascending order, so every batch of OIDs +// this file hands to a query is built the same way on both nodes. +func sortedOIDs(m map[uint32]string) []uint32 { + out := make([]uint32, 0, len(m)) + for oid := range m { + out = append(out, oid) + } + sort.Slice(out, func(i, j int) bool { return out[i] < out[j] }) + return out +} + +// buildReferencedTypeObjects turns fetchReferencedTypeDescriptors' result +// into one Object per distinct type, added to objects. +func buildReferencedTypeObjects( + quotedOf map[string]string, + domains map[uint32]queries.DomainDescriptor, + ranges map[uint32]queries.RangeDescriptor, + composites map[uint32]queries.CompositeDescriptor, + enums map[uint32]queries.EnumDescriptor, + objects map[ObjectID]Object, +) { + for _, d := range domains { + qualified := qualify(quotedOf, d.Namespace, d.Name) + props := []Property{ + // Namespace, name and kind are kept as separate properties so + // compareDomainObject can feed them straight into classifyType. + {Name: "basetype_namespace", Value: d.BaseTypeNamespace}, + {Name: "basetype_name", Value: d.BaseTypeName}, + {Name: "basetype_kind", Value: d.BaseTypeKind}, + {Name: "basetypmod", Value: strconv.FormatInt(int64(d.BaseTypeMod), 10)}, + // Display only; the three fields above are what gets compared. + {Name: "basetype_text", Value: d.BaseTypeText}, + {Name: "notnull", Value: strconv.FormatBool(d.NotNull)}, + {Name: "default", Value: d.Default}, + } + for _, chk := range d.Checks { + props = append(props, Property{Name: "check", Value: chk}) + } + sortProperties(props) + objects[TypeID("domain", d.Namespace, d.Name)] = Object{Kind: "domain", Name: qualified, Properties: props} + } + + for _, r := range ranges { + qualified := qualify(quotedOf, r.Namespace, r.Name) + props := []Property{ + {Name: "subtype", Value: qualify(quotedOf, r.SubtypeNamespace, r.SubtypeName)}, + {Name: "subtype_kind", Value: r.SubtypeKind}, + {Name: "collation", Value: r.Collation}, + {Name: "opclass", Value: r.Opclass}, + {Name: "canonical", Value: r.Canonical}, + {Name: "subtype_diff", Value: r.SubtypeDiff}, + } + sortProperties(props) + objects[TypeID("range", r.Namespace, r.Name)] = Object{Kind: "range", Name: qualified, Properties: props} + } + + for _, d := range composites { + qualified := qualify(quotedOf, d.Namespace, d.Name) + var props []Property + for _, a := range d.Attributes { + props = append(props, + Property{ + // The attribute's own attnum, not its position in the + // slice: dropping one attribute must not renumber the + // ones after it, or a single DROP ATTRIBUTE on one node + // makes every later attribute look changed too. + Name: fmt.Sprintf("attr:%04d:%s", a.AttNum, a.Name), + // Packed (see packAttr) so compareCompositeObject can + // split it apart and classify the type the same way a + // column's type is classified (e.g. int4 -> int8 as + // RankNarrowed, not a blanket incompatible). + Value: packAttr(a.TypeNamespace, a.TypeName, a.TypeKind, a.TypeMod, a.Collation), + }, + // Display only; paired with the property above by attnum and + // attribute name. + Property{ + Name: fmt.Sprintf("attrtext:%04d:%s", a.AttNum, a.Name), + Value: attributeText(a.TypeText, a.Collation), + }, + ) + } + // Not sorted: attribute order is part of a composite type's + // structural identity, so the "%04d" attnum keeps this Object's + // property order attnum-stable. + objects[TypeID("composite", d.Namespace, d.Name)] = Object{Kind: "composite", Name: qualified, Properties: props} + } + + for _, d := range enums { + objects[TypeID("enum", d.Namespace, d.Name)] = Object{ + Kind: "enum", Name: qualify(quotedOf, d.Namespace, d.Name), + // joinList, not a comma: a label may contain one, and + // ('a,b','c') must not compare equal to ('a','b,c'). + Properties: []Property{{Name: "labels", Value: joinList(d.Labels)}}, + } + } +} + +// identifiersToQuote collects every distinct identifier this snapshot will +// print, for one QuoteIdentifiers round trip. Missing one here is a quoting +// bug, not a correctness bug: qualify falls back to the raw, unescaped name. +func identifiersToQuote( + schemaName string, + tables []string, + columnsByTable map[string][]queries.ColumnDescriptor, + domains map[uint32]queries.DomainDescriptor, + ranges map[uint32]queries.RangeDescriptor, + composites map[uint32]queries.CompositeDescriptor, + enums map[uint32]queries.EnumDescriptor, +) []string { + seen := map[string]bool{schemaName: true} + for _, table := range tables { + seen[table] = true + for _, c := range columnsByTable[table] { + seen[c.Name] = true + } + } + for _, d := range domains { + seen[d.Namespace] = true + seen[d.Name] = true + } + for _, r := range ranges { + seen[r.Namespace] = true + seen[r.Name] = true + seen[r.SubtypeNamespace] = true + seen[r.SubtypeName] = true + } + for _, d := range composites { + seen[d.Namespace] = true + seen[d.Name] = true + for _, a := range d.Attributes { + seen[a.Name] = true + } + } + for _, d := range enums { + seen[d.Namespace] = true + seen[d.Name] = true + } + + names := make([]string, 0, len(seen)) + for name := range seen { + names = append(names, name) + } + return names +} + +// deparseSettings are pinned for the life of every snapshot transaction. +// +// Much of what this package collects is text rendered by the reading +// session (pg_get_constraintdef, pg_get_expr, format_type), and that +// rendering depends on session settings — a differing DateStyle alone can +// turn a byte-identical schema into several spurious divergences, purely in +// how date literals are spelled. Pinning these settings, the same way +// pg_dump does before deparsing, makes the collected text a function of the +// catalog alone. +// +// search_path is pinned too, so any name a deparse routine chooses to +// qualify is decided the same way on both nodes — every query in this +// transaction schema-qualifies what it reads, against pg_catalog +// explicitly. +var deparseSettings = []string{ + "SET LOCAL DateStyle = 'ISO, YMD'", + "SET LOCAL IntervalStyle = 'postgres'", + "SET LOCAL TimeZone = 'UTC'", + "SET LOCAL bytea_output = 'hex'", + // money's output function formats through lc_monetary, so a default or + // CHECK holding a money constant deparses differently under a different + // monetary locale. + "SET LOCAL lc_monetary = 'C'", + "SET LOCAL extra_float_digits = 3", + "SET LOCAL standard_conforming_strings = on", + "SET LOCAL search_path = pg_catalog", +} + +// pinDeparseSettings applies deparseSettings inside the snapshot +// transaction. SET LOCAL is allowed in a READ ONLY transaction and reverts +// when the transaction ends, so the connection is left unchanged. +func pinDeparseSettings(ctx context.Context, tx pgx.Tx, nodeName string) error { + for _, stmt := range deparseSettings { + if _, err := tx.Exec(ctx, stmt); err != nil { + return fmt.Errorf("pinning deparse settings on %s (%s): %w", nodeName, stmt, err) + } + } + return nil +} + +// Qualify is qualify, exported for callers outside this package that have +// an identifier to print and a quotedOf map from queries.QuoteIdentifiers - +// internal/consistency/diff naming a table that exists on only some nodes, +// for one. It is the same function so that one schema-diff report spells a +// given name exactly one way. +func Qualify(quotedOf map[string]string, parts ...string) string { + return qualify(quotedOf, parts...) +} + +// qualify joins identifiers for a person to read, each rendered through +// quotedOf (from queries.QuoteIdentifiers) exactly as PostgreSQL itself +// would write it back — quoted and case-preserved wherever an unquoted +// spelling would mean something else. This keeps a dot inside a quoted +// identifier from being misread as a component separator. See ObjectID's +// doc comment for the comparison-key half of this problem. +// +// A part missing from quotedOf falls back to the raw, unquoted part rather +// than failing the whole snapshot over a display string. +func qualify(quotedOf map[string]string, parts ...string) string { + out := make([]string, len(parts)) + for i, p := range parts { + if q, ok := quotedOf[p]; ok { + out[i] = q + } else { + out[i] = p + } + } + return strings.Join(out, ".") +} + +// joinList joins values that are compared as one string but must stay +// individually distinguishable — enum labels, key column names. A comma +// cannot do this: it is a legal character in both, so ("a,b", "c") and +// ("a", "b,c") would join to the same string and compare equal. The same +// reasoning as ObjectID's, for values rather than identifiers. +// +// Each value is written as its own byte length, then ":", then its bytes — +// a netstring, not a delimiter. An enum label is a string literal, not an +// identifier, so nothing stops a user from writing one that contains +// whatever byte a delimiter-based join would pick to keep values apart; +// counting bytes up front needs no such byte to be off limits, so the join +// is unambiguous for every value, not just the realistic ones. +func joinList(values []string) string { + var b strings.Builder + for _, v := range values { + b.WriteString(strconv.Itoa(len(v))) + b.WriteByte(':') + b.WriteString(v) + } + return b.String() +} + +// splitList reverses joinList. ok is false for a string that is not validly +// encoded — corrupt input, or a value that never went through joinList. +func splitList(joined string) (values []string, ok bool) { + for len(joined) > 0 { + sep := strings.IndexByte(joined, ':') + if sep < 0 { + return nil, false + } + n, err := strconv.Atoi(joined[:sep]) + if err != nil || n < 0 { + return nil, false + } + rest := joined[sep+1:] + if len(rest) < n { + return nil, false + } + values = append(values, rest[:n]) + joined = rest[n:] + } + return values, true +} + +// packAttr packs one composite attribute's type identity into a single +// string, the same way joinList packs a list — as length-prefixed fields, +// not delimiter-joined ones, so a collation or type name containing +// whatever character a delimiter would have used still round-trips. +func packAttr(typeNamespace, typeName, typeKind string, typeMod int32, collation string) string { + return joinList([]string{ + typeNamespace, + typeName, + typeKind, + strconv.FormatInt(int64(typeMod), 10), + collation, + }) +} + +// unpackAttr reverses packAttr. ok is false for a value that did not come +// from packAttr. +func unpackAttr(packed string) (typeNamespace, typeName, typeKind string, typeMod int32, collation string, ok bool) { + fields, ok := splitList(packed) + if !ok || len(fields) != 5 { + return "", "", "", 0, "", false + } + mod, err := strconv.ParseInt(fields[3], 10, 32) + if err != nil { + return "", "", "", 0, "", false + } + return fields[0], fields[1], fields[2], int32(mod), fields[4], true +} + +// attributeText renders one composite attribute for a person: its type as +// PostgreSQL itself would print it, plus the collation when the attribute +// carries one. +func attributeText(typeText, collation string) string { + if collation == "" { + return typeText + } + return typeText + " collate " + collation +} + +// sortProperties orders a table's properties deterministically by (Name, +// Value). For columns, keys and the table object, Names are unique, so this +// is effectively a sort by Name. For constraints, every entry shares the +// Name "constraint", so this falls back to sorting by Value — i.e. by +// constraint definition, not by PostgreSQL's invented name. +func sortProperties(props []Property) { + sort.Slice(props, func(i, j int) bool { + if props[i].Name != props[j].Name { + return props[i].Name < props[j].Name + } + return props[i].Value < props[j].Value + }) +} diff --git a/internal/consistency/schema/collect_test.go b/internal/consistency/schema/collect_test.go new file mode 100644 index 00000000..0c7d76a6 --- /dev/null +++ b/internal/consistency/schema/collect_test.go @@ -0,0 +1,40 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package schema + +import "testing" + +// TestSortProperties_OrdersByNameThenValue checks that entries with +// distinct Names sort by Name, and entries sharing a Name (constraints, all +// named "constraint") fall back to sorting by Value, i.e. by definition +// text. +func TestSortProperties_OrdersByNameThenValue(t *testing.T) { + props := []Property{ + {Name: "type", Value: "text"}, + {Name: "constraint", Value: "CHECK (b > 0)"}, + {Name: "constraint", Value: "CHECK (a > 0)"}, + {Name: "notnull", Value: "true"}, + } + sortProperties(props) + + want := []Property{ + {Name: "constraint", Value: "CHECK (a > 0)"}, + {Name: "constraint", Value: "CHECK (b > 0)"}, + {Name: "notnull", Value: "true"}, + {Name: "type", Value: "text"}, + } + for i := range want { + if props[i] != want[i] { + t.Fatalf("position %d: got %+v, want %+v", i, props[i], want[i]) + } + } +} diff --git a/internal/consistency/schema/compare.go b/internal/consistency/schema/compare.go new file mode 100644 index 00000000..4ec358bd --- /dev/null +++ b/internal/consistency/schema/compare.go @@ -0,0 +1,936 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package schema + +import ( + "fmt" + "sort" + "strconv" + "strings" +) + +// Divergence is one difference found between two nodes' structure. +// NodeA/NodeB are the two snapshots Compare was given, in the order given, +// and ValueOnA/ValueOnB follow that same order; comparison is symmetric. +// Turning this into directional "n1 -> n2" reporting, once a replication +// topology is known, is internal/consistency/drift's job. +type Divergence struct { + Object string `json:"object"` // "public.orders" or "public.orders.discount" + Kind string `json:"kind"` // "table" | "column" | "key" | "constraint" + Property string `json:"property,omitempty"` // "type", "notnull", ...; empty when the whole object is what differs + + NodeA string `json:"node_a"` + NodeB string `json:"node_b"` + ValueOnA string `json:"value_on_a"` + ValueOnB string `json:"value_on_b"` + + Rank string `json:"rank"` // one of the Rank* constants in rank.go + + // NarrowSide names the node whose value set is the narrower one. Only + // set when Rank == RankNarrowed. + NarrowSide string `json:"narrow_side,omitempty"` + + // Note carries a short, human-facing explanation for a Divergence whose + // Rank alone would not make the reason obvious. Empty when the + // Object/Kind/Property/Rank already say enough. + Note string `json:"note,omitempty"` +} + +// FindingKey identifies one finding regardless of which side of a node +// pair its values landed on, so a caller can count distinct findings across +// every pair without counting one drift once per pair that sees it. +// +// Object+Kind+Property is enough for all but constraints, which +// compareConstraints reports against the table with no Property: without +// the definition text a table missing five would count as one. Their +// values are sorted into the key because the odd node out is NodeA in one +// pair and NodeB in the next. +func (d Divergence) FindingKey() string { + key := d.Object + "\x00" + d.Kind + "\x00" + d.Property + if d.Kind != "constraint" { + return key + } + lo, hi := d.ValueOnA, d.ValueOnB + if hi < lo { + lo, hi = hi, lo + } + return key + "\x00" + lo + "\x00" + hi +} + +// Compare finds every structural difference between two Snapshots, for the +// given tables in schemaName. +// +// Both snapshots must have been collected with the same schemaName and +// tables. Reconciling a scope that resolved differently on the two nodes — +// a table present on one node but not the other — is the caller's job: pass +// Compare only the tables both sides agree are in scope, and report a scope +// mismatch as a separate message, not as a Divergence. A table that +// disappears between scope resolution and collection (dropped concurrently) +// is different — Compare does catch that, as RankAbsent on the table +// itself. +func Compare(schemaName string, tables []string, a, b Snapshot) []Divergence { + var out []Divergence + + for _, table := range tables { + tableID := TableID(schemaName, table) + aTable, aOK := a.Objects[tableID] + bTable, bOK := b.Objects[tableID] + + if !aOK || !bOK { + // This package never opens a connection of its own — it only + // prints an identifier a Snapshot already collected and quoted. + // Whichever side has the table names it; if neither does, the + // raw, unquoted schema.table is the only thing left to print. + qualified := aTable.Name + if qualified == "" { + qualified = bTable.Name + } + if qualified == "" { + qualified = schemaName + "." + table + } + out = append(out, Divergence{ + Object: qualified, Kind: "table", + NodeA: a.Node, NodeB: b.Node, + ValueOnA: presence(aOK), ValueOnB: presence(bOK), + Rank: RankAbsent, + Note: "the table is missing from one node's snapshot", + }) + continue + } + qualified := aTable.Name + + out = append(out, compareTableProperties(schemaName, table, qualified, a, b)...) + out = append(out, compareColumns(schemaName, table, a, b)...) + out = append(out, compareKey(schemaName, table, qualified, a, b)...) + out = append(out, compareConstraints(schemaName, table, qualified, a, b)...) + } + + out = append(out, compareReferencedTypes(a, b)...) + out = append(out, compareDatabaseLocale(a, b)...) + + return out +} + +// compareDatabaseLocale diffs the two databases' own collation settings. +// +// This is reported once per comparison rather than per column, because it +// is what an uncollated column inherits and what no column can show: both +// nodes record the "default" collation in pg_attribute whatever their +// database was created with. Two nodes that disagree here hold the same +// values but sort them differently, which means a unique index can disagree +// about which rows are duplicates — so this is RankIncompatible, not a +// cosmetic difference, even though no DDL on either node caused it. +func compareDatabaseLocale(a, b Snapshot) []Divergence { + aObj, aOK := a.Objects[DatabaseID()] + bObj, bOK := b.Objects[DatabaseID()] + if !aOK || !bOK { + // One side collected no locale, which happens when its scope held + // no tables. Nothing was compared there, so there is nothing to say. + return nil + } + + var out []Divergence + for _, prop := range []string{"lc_collate", "lc_ctype", "locale_provider", "locale"} { + av, bv := getProp(aObj.Properties, prop), getProp(bObj.Properties, prop) + if av == bv { + continue + } + out = append(out, Divergence{ + Object: databaseObjectName(aObj, bObj), Kind: "database", Property: prop, + NodeA: a.Node, NodeB: b.Node, ValueOnA: av, ValueOnB: bv, + Rank: RankIncompatible, + Note: "the databases were created with different collation settings, so text sorts and compares differently on the two nodes even where every column matches", + }) + } + return out +} + +// databaseObjectName names the database a locale finding is about, allowing +// for the two nodes having named theirs differently. +func databaseObjectName(aObj, bObj Object) string { + switch { + case aObj.Name == bObj.Name: + return aObj.Name + case aObj.Name == "": + return bObj.Name + case bObj.Name == "": + return aObj.Name + default: + return aObj.Name + "/" + bObj.Name + } +} + +// compareReferencedTypes diffs every domain/range/composite/enum type both +// snapshots collected. A qualified type name present in only one snapshot +// is not reported here: some column already carries that mismatch as its +// own "type" Divergence, so a second finding would only be noise. What this +// pass catches is what column-level comparison cannot: two columns that +// agree on a type's name but disagree about what it actually constrains. +// +// Objects is a map, so the keys are gathered and sorted before anything is +// compared: two runs over an unchanged pair of nodes must produce the same +// report in the same order. +func compareReferencedTypes(a, b Snapshot) []Divergence { + var out []Divergence + for _, kind := range []string{"domain", "range", "composite", "enum"} { + ids := make([]ObjectID, 0, len(a.Objects)) + for id := range a.Objects { + if id.Kind == kind { + ids = append(ids, id) + } + } + sort.Slice(ids, func(i, j int) bool { + if ids[i].Schema != ids[j].Schema { + return ids[i].Schema < ids[j].Schema + } + return ids[i].Name < ids[j].Name + }) + + for _, id := range ids { + aObj := a.Objects[id] + bObj, ok := b.Objects[id] + if !ok { + continue + } + switch kind { + case "domain": + out = append(out, compareDomainObject(aObj.Name, a.Node, b.Node, aObj.Properties, bObj.Properties)...) + case "range": + out = append(out, compareRangeObject(aObj.Name, a.Node, b.Node, aObj.Properties, bObj.Properties)...) + case "composite": + out = append(out, compareCompositeObject(aObj.Name, a.Node, b.Node, aObj.Properties, bObj.Properties)...) + case "enum": + out = append(out, compareEnumObject(aObj.Name, a.Node, b.Node, aObj.Properties, bObj.Properties)...) + } + } + } + return out +} + +// compareDomainObject diffs one domain's constraints between two nodes. A +// changed base type is classified with classifyType, same as a column's own +// type (e.g. varchar(20) to varchar(10) is RankNarrowed); NOT NULL added on +// a domain excludes NULL, so it is RankNarrowed too; a changed default is +// representation, not a value-set change, so RankEquivalentDiffering; CHECK +// constraints are compared as a set, same as table constraints. +func compareDomainObject(name, nodeA, nodeB string, aProps, bProps []Property) []Divergence { + var out []Divergence + + aNS, bNS := getProp(aProps, "basetype_namespace"), getProp(bProps, "basetype_namespace") + aName, bName := getProp(aProps, "basetype_name"), getProp(bProps, "basetype_name") + aKind, bKind := getProp(aProps, "basetype_kind"), getProp(bProps, "basetype_kind") + aMod64, _ := strconv.ParseInt(getProp(aProps, "basetypmod"), 10, 32) + bMod64, _ := strconv.ParseInt(getProp(bProps, "basetypmod"), 10, 32) + switch { + case aKind != bKind: + // See the same case in comparePropertiesForColumn: two kinds of + // type are never interchangeable, however alike their names. + out = append(out, Divergence{ + Object: name, Kind: "domain", Property: "basetype", + NodeA: nodeA, NodeB: nodeB, + ValueOnA: describeKind(getProp(aProps, "basetype_text"), aKind), + ValueOnB: describeKind(getProp(bProps, "basetype_text"), bKind), + Rank: RankIncompatible, + Note: "the domain's base type has the same name on both nodes but is a different kind of type", + }) + + case aNS != bNS || aName != bName || aMod64 != bMod64: + rank, side := classifyType(aNS, aName, int32(aMod64), bNS, bName, int32(bMod64), nodeA, nodeB) + if rank != "" { + // Printed from basetype_text, not from the portable + // (namespace, name) pair the comparison keys on, since that + // text is what shows a varchar(20) vs varchar(10) difference. + out = append(out, Divergence{ + Object: name, Kind: "domain", Property: "basetype", + NodeA: nodeA, NodeB: nodeB, + ValueOnA: getProp(aProps, "basetype_text"), ValueOnB: getProp(bProps, "basetype_text"), + Rank: rank, NarrowSide: side, + }) + } + } + + if av, bv := getProp(aProps, "notnull"), getProp(bProps, "notnull"); av != bv { + side := nodeA + if bv == "true" { + side = nodeB + } + out = append(out, Divergence{ + Object: name, Kind: "domain", Property: "notnull", + NodeA: nodeA, NodeB: nodeB, ValueOnA: av, ValueOnB: bv, + Rank: RankNarrowed, NarrowSide: side, + }) + } + + if av, bv := getProp(aProps, "default"), getProp(bProps, "default"); av != bv { + out = append(out, Divergence{ + Object: name, Kind: "domain", Property: "default", + NodeA: nodeA, NodeB: nodeB, ValueOnA: av, ValueOnB: bv, + Rank: RankEquivalentDiffering, + }) + } + + out = append(out, compareDomainChecks(name, nodeA, nodeB, aProps, bProps)...) + + return out +} + +// compareDomainChecks diffs one domain's CHECK set. +// +// "One side has a CHECK the other lacks" is a narrowing: that side accepts +// strictly fewer values. When each side has a CHECK the other lacks, +// neither value set contains the other, so that case is reported as one +// RankIncompatible finding listing both sides' extra CHECKs. Whether the +// two conditions happen to imply one another is not decided from the +// expression text. +func compareDomainChecks(name, nodeA, nodeB string, aProps, bProps []Property) []Divergence { + aChecks, bChecks := propertySet(aProps, "check"), propertySet(bProps, "check") + + aOnly := sortedMissing(aChecks, bChecks) + bOnly := sortedMissing(bChecks, aChecks) + + switch { + case len(aOnly) == 0 && len(bOnly) == 0: + return nil + + case len(aOnly) > 0 && len(bOnly) > 0: + return []Divergence{{ + Object: name, Kind: "domain", Property: "check", + NodeA: nodeA, NodeB: nodeB, + ValueOnA: strings.Join(aOnly, "; "), ValueOnB: strings.Join(bOnly, "; "), + Rank: RankIncompatible, + Note: "each side has a CHECK the other lacks, so neither value set is contained in the other", + }} + } + + only, side, narrowIsA := aOnly, nodeA, true + if len(bOnly) > 0 { + only, side, narrowIsA = bOnly, nodeB, false + } + + out := make([]Divergence, 0, len(only)) + for _, def := range only { + d := Divergence{ + Object: name, Kind: "domain", Property: "check", + NodeA: nodeA, NodeB: nodeB, + Rank: RankNarrowed, NarrowSide: side, + Note: "the CHECK exists on one side only, so that side narrows the domain further", + } + if narrowIsA { + d.ValueOnA, d.ValueOnB = def, "(absent)" + } else { + d.ValueOnA, d.ValueOnB = "(absent)", def + } + out = append(out, d) + } + return out +} + +// sortedMissing returns the members of have that want does not contain, in +// sorted order — sorted because the sets come from maps and a report's line +// order must not depend on Go's map iteration. +func sortedMissing(have, want map[string]bool) []string { + var out []string + for def := range have { + if !want[def] { + out = append(out, def) + } + } + sort.Strings(out) + return out +} + +// compareRangeObject diffs a range type's subtype, collation, opclass and +// canonical/subtype_diff functions. None of these have a narrowing story +// this package can reason about — any difference changes what values sort +// where or how they canonicalise, so all are RankIncompatible. +// +// When the subtype itself differs, only that is reported: the opclass and +// collation belong to the subtype, so one finding covers what would +// otherwise be three (subtype, opclass, possibly collation) for a single +// change. +func compareRangeObject(name, nodeA, nodeB string, aProps, bProps []Property) []Divergence { + // The subtype's identity is its name and its kind together, so a kind + // changed under an unchanged name is still a changed subtype. Both are + // folded into one finding, printed so that the two sides differ visibly + // even when only the kind moved. + aSubtype, bSubtype := getProp(aProps, "subtype"), getProp(bProps, "subtype") + aKind, bKind := getProp(aProps, "subtype_kind"), getProp(bProps, "subtype_kind") + + if aSubtype != bSubtype || aKind != bKind { + // Only the subtype is reported: the operator class, collation and + // canonical function all belong to it, so one change would + // otherwise produce three findings. + return []Divergence{{ + Object: name, Kind: "range", Property: "subtype", + NodeA: nodeA, NodeB: nodeB, + ValueOnA: describeKind(aSubtype, aKind), + ValueOnB: describeKind(bSubtype, bKind), + Rank: RankIncompatible, + Note: "the range's subtype differs; its operator class, collation and canonical function follow from it and are not reported separately", + }} + } + + var out []Divergence + for _, prop := range []string{"collation", "opclass", "canonical", "subtype_diff"} { + av, bv := getProp(aProps, prop), getProp(bProps, prop) + if av == bv { + continue + } + out = append(out, Divergence{ + Object: name, Kind: "range", Property: prop, + NodeA: nodeA, NodeB: nodeB, ValueOnA: av, ValueOnB: bv, + Rank: RankIncompatible, + }) + } + return out +} + +// compareCompositeObject diffs a composite type's attributes. Property +// Names already carry the attnum-order index, so a union-by-Name comparison +// is naturally in declaration order and reports an added/removed/retyped/ +// recollated attribute as one line: a missing attribute on one side simply +// has "" there, matching the absent-value convention used elsewhere. +func compareCompositeObject(name, nodeA, nodeB string, aProps, bProps []Property) []Divergence { + var out []Divergence + names := unionSorted(propNames(aProps, "attr:"), propNames(bProps, "attr:")) + for _, key := range names { + av, bv := getProp(aProps, key), getProp(bProps, key) + if av == bv { + continue + } + rank, side := classifyAttr(av, bv, nodeA, nodeB) + + // The property key is "attr:0007:city" and its value is a packed + // "ns.name/typmod/collation" string — neither fit for display. The + // paired attrtext property carries the same attribute as PostgreSQL + // prints it. + ord, attr := splitAttrKey(key) + suffix := strings.TrimPrefix(key, "attr:") + out = append(out, Divergence{ + Object: name, Kind: "composite", + Property: fmt.Sprintf("attribute %d (%s)", ord, attr), + NodeA: nodeA, NodeB: nodeB, + ValueOnA: displayOrRaw(getProp(aProps, "attrtext:"+suffix), av), + ValueOnB: displayOrRaw(getProp(bProps, "attrtext:"+suffix), bv), + Rank: rank, NarrowSide: side, + }) + } + return out +} + +// classifyAttr ranks a difference between two composite attributes. +// +// An attribute missing on one side is RankAbsent. Otherwise the packed +// values are split back into (type namespace, type name, typmod, collation) +// and put through classifyType, the same reasoning a table column's own +// type gets, so widening int4 -> int8 is RankNarrowed for both a column and +// a composite attribute. A pair that cannot be unpacked, or that differs +// only in collation, falls back to what can still be said for certain: the +// two are not the same. +func classifyAttr(packedA, packedB, nodeA, nodeB string) (rank, narrowSide string) { + if packedA == "" || packedB == "" { + return RankAbsent, "" + } + + aNS, aName, aKind, aMod, aColl, aOK := unpackAttr(packedA) + bNS, bName, bKind, bMod, bColl, bOK := unpackAttr(packedB) + if !aOK || !bOK { + return RankIncompatible, "" + } + + // Kind is part of the attribute type's identity, as it is for a column + // (see comparePropertiesForColumn); an enum replaced by a same-named + // domain is not a narrowing of anything. + if aKind != bKind { + return RankIncompatible, "" + } + + if aNS == bNS && aName == bName && aMod == bMod { + // Same type, so what differs is the collation: the attribute holds + // the same values on both nodes but compares and sorts them + // differently. + if aColl != bColl { + return RankEquivalentDiffering, "" + } + // Unreachable: identical fields pack to identical strings, and the + // caller only gets here for strings that differ. Kept loud rather + // than plausible, so a future change to packAttr's format shows up + // as an obviously wrong rank instead of a quiet one. + return RankIncompatible, "" + } + + rank, side := classifyType(aNS, aName, aMod, bNS, bName, bMod, nodeA, nodeB) + if rank == "" { + return RankIncompatible, "" + } + return rank, side +} + +// splitAttrKey turns collect.go's "attr:0007:city" back into (7, "city"). +// The number is the attribute's own attnum, so it is printed as it stands — +// it matches what \d+ on the type shows, gaps from dropped attributes +// included. +func splitAttrKey(key string) (int, string) { + rest := strings.TrimPrefix(key, "attr:") + idx, attr, found := strings.Cut(rest, ":") + if !found { + return 0, rest + } + ord, err := strconv.Atoi(idx) + if err != nil { + return 0, attr + } + return ord, attr +} + +// displayOrRaw prefers the display text collect.go paired with an +// attribute, falls back to the raw portable value when there is none, and +// says "absent" when the attribute itself is missing on that side. +func displayOrRaw(display, raw string) string { + switch { + case display != "": + return display + case raw != "": + return raw + default: + return "(absent)" + } +} + +// compareEnumObject diffs an enum's label list as one whole-list property: +// order is part of an enum's identity, so a single added/removed/reordered +// label is reported as one line naming the whole list. +func compareEnumObject(name, nodeA, nodeB string, aProps, bProps []Property) []Divergence { + av, bv := getProp(aProps, "labels"), getProp(bProps, "labels") + if av == bv { + return nil + } + return []Divergence{{ + Object: name, Kind: "enum", Property: "labels", + NodeA: nodeA, NodeB: nodeB, ValueOnA: av, ValueOnB: bv, + Rank: RankIncompatible, + Note: "an enum's set of labels and their order both drive comparison and casting", + }} +} + +// propNames returns the names of every property whose name starts with +// prefix (pass "" for all of them). +func propNames(props []Property, prefix string) []string { + names := make([]string, 0, len(props)) + for _, p := range props { + if strings.HasPrefix(p.Name, prefix) { + names = append(names, p.Name) + } + } + return names +} + +// propertySet gathers the values of every property called name into a set. +// It serves the kinds that are compared as a set rather than slot by slot - +// a domain's CHECKs, a table's constraints - where the question is only +// whether both nodes hold the same definitions, in any order. +func propertySet(props []Property, name string) map[string]bool { + set := make(map[string]bool) + for _, p := range props { + if p.Name == name { + set[p.Value] = true + } + } + return set +} + +// presence spells "the object exists here" as a value, so that a finding +// about a missing table or column still shows something on both sides of the +// report instead of an empty string on one of them. +func presence(ok bool) string { + if ok { + return "present" + } + return "absent" +} + +// typeKindNames spells out pg_type.typtype for a report. An unrecognised +// code is printed as-is rather than guessed at. +var typeKindNames = map[string]string{ + "b": "base type", + "c": "composite type", + "d": "domain", + "e": "enum", + "m": "multirange", + "p": "pseudo-type", + "r": "range", +} + +// describeKind names the kind alongside the type, for the one finding where +// the type names match and only the kind differs — printing "public.status" +// on both sides would otherwise read as a tool malfunction. +func describeKind(typeText, kind string) string { + name, ok := typeKindNames[kind] + if !ok { + name = fmt.Sprintf("typtype %q", kind) + } + if typeText == "" { + return name + } + return fmt.Sprintf("%s (%s)", typeText, name) +} + +// parseTypmod reads a type modifier a Snapshot recorded. A modifier that +// will not parse is an error rather than a zero: 0 is a legal typmod, so +// defaulting to it would turn a broken snapshot into a plausible-looking +// comparison. +func parseTypmod(value string) (int64, error) { + mod, err := strconv.ParseInt(value, 10, 32) + if err != nil { + return 0, fmt.Errorf("type modifier %q: %w", value, err) + } + return mod, nil +} + +// getProp returns the value of the property called name, or "" when there is +// no such property. A property that is absent and one that holds "" are +// deliberately not told apart: every caller here asks whether two nodes +// agree, and "neither node recorded anything" is agreement. +func getProp(props []Property, name string) string { + for _, p := range props { + if p.Name == name { + return p.Value + } + } + return "" +} + +// compareTableProperties diffs what is recorded about the table itself, +// rather than about its columns, key or constraints. Today that is only how +// the table is partitioned, which is why a table with no partitioning at all +// yields nothing here. +func compareTableProperties(schemaName, table, qualified string, a, b Snapshot) []Divergence { + aTable := a.Objects[TableID(schemaName, table)] + bTable := b.Objects[TableID(schemaName, table)] + + var out []Divergence + for _, name := range []string{"partition_bound", "partition_key"} { + av, bv := getProp(aTable.Properties, name), getProp(bTable.Properties, name) + if av == bv { + continue + } + out = append(out, Divergence{ + Object: qualified, Kind: "table", Property: name, + NodeA: a.Node, NodeB: b.Node, ValueOnA: av, ValueOnB: bv, + Rank: RankIncompatible, + Note: "the partitioning differs; whether one range contains the other is not checked", + }) + } + return out +} + +// compareColumns diffs every column either node holds, walking the union of +// the two column lists rather than one node's. That is what lets a column +// present on one side only be reported as absent instead of quietly skipped, +// and it keeps the result the same whichever snapshot was passed first. +func compareColumns(schemaName, table string, a, b Snapshot) []Divergence { + tableID := TableID(schemaName, table) + names := unionSorted(a.TableColumns[tableID], b.TableColumns[tableID]) + + var out []Divergence + for _, name := range names { + aCol, aOK := a.Objects[ColumnID(schemaName, table, name)] + bCol, bOK := b.Objects[ColumnID(schemaName, table, name)] + + // Whichever side has the column already carries its own quoted + // display name (built by qualify() when the Snapshot was + // collected) — see the note in Compare's own missing-table branch + // for why this package prints only names a Snapshot already + // quoted, never one it builds itself. + colQualified := aCol.Name + if colQualified == "" { + colQualified = bCol.Name + } + if colQualified == "" { + colQualified = schemaName + "." + table + "." + name + } + + if !aOK || !bOK { + out = append(out, Divergence{ + Object: colQualified, Kind: "column", + NodeA: a.Node, NodeB: b.Node, + ValueOnA: presence(aOK), ValueOnB: presence(bOK), + Rank: RankAbsent, + }) + continue + } + + out = append(out, comparePropertiesForColumn(colQualified, a.Node, b.Node, aCol.Properties, bCol.Properties)...) + } + return out +} + +// comparePropertiesForColumn diffs one column that both nodes have, property +// by property. +// +// The type is handled first and on its own, because it decides more than its +// own finding: an unreadable type modifier or a changed kind of type makes +// any narrowing reasoning meaningless, and a type that changed outright +// makes the column's collation a consequence of that change rather than a +// separate difference worth reporting. +func comparePropertiesForColumn(colQualified, nodeA, nodeB string, aProps, bProps []Property) []Divergence { + var out []Divergence + + aNS, bNS := getProp(aProps, "type_namespace"), getProp(bProps, "type_namespace") + aName, bName := getProp(aProps, "type_name"), getProp(bProps, "type_name") + aKind, bKind := getProp(aProps, "type_kind"), getProp(bProps, "type_kind") + aTypeMod64, aModErr := parseTypmod(getProp(aProps, "type_mod")) + bTypeMod64, bModErr := parseTypmod(getProp(bProps, "type_mod")) + + typeIsIncompatible := false + switch { + case aModErr != nil || bModErr != nil: + // Neither side's modifier can be trusted, so say that rather than + // silently comparing against a typmod of 0, which is a legal value + // for some types and would make a broken snapshot look like a real + // difference — or worse, like a match. + typeIsIncompatible = true + out = append(out, Divergence{ + Object: colQualified, Kind: "column", Property: "type", + NodeA: nodeA, NodeB: nodeB, + ValueOnA: getProp(aProps, "type"), ValueOnB: getProp(bProps, "type"), + Rank: RankIncompatible, + Note: "the snapshot's type modifier could not be read, so the two types cannot be compared", + }) + + case aKind != bKind: + // The kind is part of a type's identity. Dropping an enum and + // recreating the name as a domain leaves (namespace, name) and even + // the printed type untouched, so without this the substitution is + // invisible — and no narrowing reasoning applies between kinds. + typeIsIncompatible = true + out = append(out, Divergence{ + Object: colQualified, Kind: "column", Property: "type", + NodeA: nodeA, NodeB: nodeB, + ValueOnA: describeKind(getProp(aProps, "type"), aKind), + ValueOnB: describeKind(getProp(bProps, "type"), bKind), + Rank: RankIncompatible, + Note: "same type name on both nodes, but one node's is a different kind of type", + }) + + case aNS != bNS || aName != bName || aTypeMod64 != bTypeMod64: + rank, side := classifyType(aNS, aName, int32(aTypeMod64), bNS, bName, int32(bTypeMod64), nodeA, nodeB) + if rank != "" { + typeIsIncompatible = rank == RankIncompatible + out = append(out, Divergence{ + Object: colQualified, Kind: "column", Property: "type", + NodeA: nodeA, NodeB: nodeB, + ValueOnA: getProp(aProps, "type"), ValueOnB: getProp(bProps, "type"), + Rank: rank, NarrowSide: side, + }) + } + } + + // NOT NULL narrows the column's set of acceptable values (it excludes + // NULL), fitting the same "strict subset" reasoning as type width. + if av, bv := getProp(aProps, "notnull"), getProp(bProps, "notnull"); av != bv { + side := nodeA + if bv == "true" { + side = nodeB + } + out = append(out, Divergence{ + Object: colQualified, Kind: "column", Property: "notnull", + NodeA: nodeA, NodeB: nodeB, ValueOnA: av, ValueOnB: bv, + Rank: RankNarrowed, NarrowSide: side, + }) + } + + // None of these restrict which values fit; they change representation + // or behaviour for values that fit equally well on both sides. + for _, name := range []string{"identity", "generated", "options", "default"} { + av, bv := getProp(aProps, name), getProp(bProps, name) + if av == bv { + continue + } + out = append(out, Divergence{ + Object: colQualified, Kind: "column", Property: name, + NodeA: nodeA, NodeB: nodeB, ValueOnA: av, ValueOnB: bv, + Rank: RankEquivalentDiffering, + }) + } + + // Collation carries the collation's version as well as its name, so a + // version difference (a different glibc/ICU release, not a schema edit) + // needs the Note to say so. It is skipped once the type itself is + // incompatible, since a column retyped integer -> text gains a + // collation only as a consequence of that one change. + if av, bv := getProp(aProps, "collation"), getProp(bProps, "collation"); av != bv && !typeIsIncompatible { + out = append(out, Divergence{ + Object: colQualified, Kind: "column", Property: "collation", + NodeA: nodeA, NodeB: nodeB, ValueOnA: av, ValueOnB: bv, + Rank: RankEquivalentDiffering, + Note: "collation name/provider/version, in that order; a differing version means the nodes' collation libraries differ, so the same text can sort differently and a unique index can disagree about duplicates", + }) + } + + return out +} + +// compareKey diffs the row identity the two nodes replicate by: the replica +// identity mode, the key's columns in index order, and the operator classes +// those columns use. +// +// The mode and the columns are RankIncompatible, since two nodes that do not +// agree on which rows are the same row cannot converge by exchanging them. +// The operator classes are ranked lower: the same rows are still identified, +// but a different class means a different idea of which values are equal and +// how they sort. +func compareKey(schemaName, table, qualified string, a, b Snapshot) []Divergence { + aKey := a.Objects[KeyID(schemaName, table)] + bKey := b.Objects[KeyID(schemaName, table)] + + var out []Divergence + + if av, bv := getProp(aKey.Properties, "replica_identity"), getProp(bKey.Properties, "replica_identity"); av != bv { + out = append(out, Divergence{ + Object: qualified, Kind: "key", Property: "replica_identity", + NodeA: a.Node, NodeB: b.Node, ValueOnA: av, ValueOnB: bv, + Rank: RankIncompatible, + Note: "different row identity mode: each node would resolve conflicts differently", + }) + } + if av, bv := getProp(aKey.Properties, "key_columns"), getProp(bKey.Properties, "key_columns"); av != bv { + out = append(out, Divergence{ + Object: qualified, Kind: "key", Property: "key_columns", + NodeA: a.Node, NodeB: b.Node, ValueOnA: av, ValueOnB: bv, + Rank: RankIncompatible, + }) + } + if av, bv := getProp(aKey.Properties, "key_opclasses"), getProp(bKey.Properties, "key_opclasses"); av != bv { + out = append(out, Divergence{ + Object: qualified, Kind: "key", Property: "key_opclasses", + NodeA: a.Node, NodeB: b.Node, ValueOnA: av, ValueOnB: bv, + Rank: RankEquivalentDiffering, + Note: "same type, different operator class: a different notion of equality and ordering for this column", + }) + } + return out +} + +// compareConstraints diffs two tables' constraint sets. This is a set +// comparison, not a property comparison: PostgreSQL invents names for +// unnamed constraints, so there is no stable per-name slot to line up +// across nodes — only "does this exact definition exist on both sides". +func compareConstraints(schemaName, table, qualified string, a, b Snapshot) []Divergence { + aSet := constraintSet(a.Objects[ConstraintID(schemaName, table)]) + bSet := constraintSet(b.Objects[ConstraintID(schemaName, table)]) + + aOnly, bOnly := sortedMissing(aSet, bSet), sortedMissing(bSet, aSet) + + // A CHECK one node has and the other lacks makes that node's accepted + // values a strict subset, exactly as it does on a domain, so it is + // ranked the same way here: RankNarrowed, naming the narrow side. The + // same reasoning does not extend to the other constraint types — a + // missing FOREIGN KEY or UNIQUE is not a narrowing of a value set the + // two nodes otherwise share. See compareDomainChecks, which this + // deliberately mirrors. + // + // Both conditions below are needed. One side's extra constraints must + // be CHECKs and nothing else, and the other side must have no extra + // constraint at all: a CHECK on one node against a FOREIGN KEY on the + // other is not a narrowing either way, since each node then rejects + // rows the other accepts and neither set contains the other. + checkNarrows := (onlyChecks(aOnly) && len(bOnly) == 0) || + (onlyChecks(bOnly) && len(aOnly) == 0) + + var out []Divergence + for _, def := range aOnly { + d := Divergence{ + Object: qualified, Kind: "constraint", + NodeA: a.Node, NodeB: b.Node, ValueOnA: def, ValueOnB: "(absent)", + Rank: RankAbsent, + } + if checkNarrows && isCheck(def) { + d.Rank, d.NarrowSide = RankNarrowed, a.Node + d.Note = "the CHECK exists on one side only, so that side accepts strictly fewer rows" + } + out = append(out, d) + } + for _, def := range bOnly { + d := Divergence{ + Object: qualified, Kind: "constraint", + NodeA: a.Node, NodeB: b.Node, ValueOnA: "(absent)", ValueOnB: def, + Rank: RankAbsent, + } + if checkNarrows && isCheck(def) { + d.Rank, d.NarrowSide = RankNarrowed, b.Node + d.Note = "the CHECK exists on one side only, so that side accepts strictly fewer rows" + } + out = append(out, d) + } + + sort.Slice(out, func(i, j int) bool { + return out[i].ValueOnA+"\x00"+out[i].ValueOnB < out[j].ValueOnA+"\x00"+out[j].ValueOnB + }) + return out +} + +// isCheck reports whether a packed constraint value is a CHECK. collect.go +// writes contype first, so the prefix is the whole test. +func isCheck(def string) bool { + return strings.HasPrefix(def, "c|") +} + +// onlyChecks reports whether defs is non-empty and holds nothing but CHECK +// constraints — the condition under which one side can be called the +// narrower one. +func onlyChecks(defs []string) bool { + if len(defs) == 0 { + return false + } + for _, def := range defs { + if !isCheck(def) { + return false + } + } + return true +} + +// constraintSet gathers one table's constraint definitions into a set. It +// takes the whole Object rather than its properties, so that a table for +// which no constraint Object was collected at all comes back as an empty +// set: a table with no constraints and a table nothing was read from compare +// the same way, which is what lets compareConstraints stay a set difference. +func constraintSet(obj Object) map[string]bool { + set := make(map[string]bool, len(obj.Properties)) + for _, p := range obj.Properties { + if p.Name == "constraint" { + set[p.Value] = true + } + } + return set +} + +// unionSorted returns every name held by a or b, once each, in sorted order. +// The union is built through a map, so it has to be sorted before it is +// returned: a report's line order must not follow Go's map iteration, which +// differs from run to run. +func unionSorted(a, b []string) []string { + set := make(map[string]bool, len(a)+len(b)) + for _, s := range a { + set[s] = true + } + for _, s := range b { + set[s] = true + } + out := make([]string, 0, len(set)) + for s := range set { + out = append(out, s) + } + sort.Strings(out) + return out +} diff --git a/internal/consistency/schema/compare_test.go b/internal/consistency/schema/compare_test.go new file mode 100644 index 00000000..b1ae589e --- /dev/null +++ b/internal/consistency/schema/compare_test.go @@ -0,0 +1,1456 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package schema + +import ( + "fmt" + "strings" + "testing" +) + +// snapshotBuilder assembles a Snapshot by hand, without a database, so +// these tests can exercise Compare/classifyType directly. +type snapshotBuilder struct { + snap Snapshot +} + +func newSnapshot(node string) *snapshotBuilder { + return &snapshotBuilder{snap: Snapshot{ + Node: node, + Objects: make(map[ObjectID]Object), + TableColumns: make(map[ObjectID][]string), + }} +} + +// splitQualified is test-only sugar: it lets these tests write +// "public.orders" instead of a schema and a table argument each time, by +// cutting at the first dot. Tests that care about dotted identifiers spell +// the schema and name out through the ObjectID constructors directly. +func splitQualified(qualified string) (schema, name string) { + schema, name, found := strings.Cut(qualified, ".") + if !found { + return "", qualified + } + return schema, name +} + +func (s *snapshotBuilder) table(qualified string, props ...Property) *snapshotBuilder { + sortProperties(props) + schema, table := splitQualified(qualified) + s.snap.Objects[TableID(schema, table)] = Object{Kind: "table", Name: qualified, Properties: props} + return s +} + +func (s *snapshotBuilder) column(qualified, name string, props ...Property) *snapshotBuilder { + sortProperties(props) + schema, table := splitQualified(qualified) + colQualified := qualified + "." + name + s.snap.Objects[ColumnID(schema, table, name)] = Object{Kind: "column", Name: colQualified, Properties: props} + s.snap.TableColumns[TableID(schema, table)] = append(s.snap.TableColumns[TableID(schema, table)], name) + return s +} + +func (s *snapshotBuilder) key(qualified string, props ...Property) *snapshotBuilder { + sortProperties(props) + schema, table := splitQualified(qualified) + s.snap.Objects[KeyID(schema, table)] = Object{Kind: "key", Name: qualified, Properties: props} + return s +} + +func (s *snapshotBuilder) constraints(qualified string, defs ...string) *snapshotBuilder { + var props []Property + for _, d := range defs { + props = append(props, Property{Name: "constraint", Value: d}) + } + sortProperties(props) + schema, table := splitQualified(qualified) + s.snap.Objects[ConstraintID(schema, table)] = Object{Kind: "constraint", Name: qualified, Properties: props} + return s +} + +// domain/rng/composite/enum mirror the corresponding Kind collectReferencedTypes +// builds from a live catalog, for tests exercising a referenced type's own +// definition rather than a column's declared type. +func (s *snapshotBuilder) domain(qualified string, props ...Property) *snapshotBuilder { + sortProperties(props) + schema, name := splitQualified(qualified) + s.snap.Objects[TypeID("domain", schema, name)] = Object{Kind: "domain", Name: qualified, Properties: props} + return s +} + +func (s *snapshotBuilder) rng(qualified string, props ...Property) *snapshotBuilder { + sortProperties(props) + schema, name := splitQualified(qualified) + s.snap.Objects[TypeID("range", schema, name)] = Object{Kind: "range", Name: qualified, Properties: props} + return s +} + +func (s *snapshotBuilder) composite(qualified string, props ...Property) *snapshotBuilder { + // Not sorted: attribute order is part of a composite type's identity, + // and callers pass props already in the order they want compared. + schema, name := splitQualified(qualified) + s.snap.Objects[TypeID("composite", schema, name)] = Object{Kind: "composite", Name: qualified, Properties: props} + return s +} + +func (s *snapshotBuilder) enum(qualified, labels string) *snapshotBuilder { + schema, name := splitQualified(qualified) + s.snap.Objects[TypeID("enum", schema, name)] = Object{ + Kind: "enum", Name: qualified, + Properties: []Property{{Name: "labels", Value: labels}}, + } + return s +} + +// locale mirrors the database-wide Object CollectSnapshot builds from +// pg_database, for tests about the collation two nodes inherited rather +// than about anything in the compared schema. +func (s *snapshotBuilder) locale(name, collate, ctype, provider, locale string) *snapshotBuilder { + props := []Property{ + {Name: "lc_collate", Value: collate}, + {Name: "lc_ctype", Value: ctype}, + {Name: "locale_provider", Value: provider}, + {Name: "locale", Value: locale}, + } + sortProperties(props) + s.snap.Objects[DatabaseID()] = Object{Kind: "database", Name: name, Properties: props} + return s +} + +func (s *snapshotBuilder) build() Snapshot { return s.snap } + +// userTypeColumnProps is baseColumnProps for a column whose declared type is +// some referenced type object (domain/range/composite/enum), not a plain +// base type — type_namespace/type_name naming that type itself rather than +// pg_catalog.int4, and kind carrying pg_type.typtype ('d'/'r'/'c'/'e'). +// These tests are about the referenced type's own definition, exercised by +// compareReferencedTypes, not about the column, so this only needs to make +// both sides agree at the column level. +func userTypeColumnProps(namespace, name, kind string) []Property { + return []Property{ + {Name: "type", Value: name}, + {Name: "type_namespace", Value: namespace}, + {Name: "type_name", Value: name}, + {Name: "type_kind", Value: kind}, + {Name: "type_mod", Value: "-1"}, + {Name: "notnull", Value: "false"}, + {Name: "identity", Value: ""}, + {Name: "generated", Value: ""}, + {Name: "options", Value: ""}, + {Name: "collation", Value: ""}, + {Name: "default", Value: ""}, + } +} + +// baseColumnProps returns a plausible, complete property set for a column so +// tests only need to override the one property under test. +func baseColumnProps(overrides ...Property) []Property { + props := []Property{ + {Name: "type", Value: "integer"}, + {Name: "type_namespace", Value: "pg_catalog"}, + {Name: "type_name", Value: "int4"}, + {Name: "type_kind", Value: "b"}, + {Name: "type_mod", Value: "-1"}, + {Name: "notnull", Value: "false"}, + {Name: "identity", Value: ""}, + {Name: "generated", Value: ""}, + {Name: "options", Value: ""}, + {Name: "collation", Value: ""}, + {Name: "default", Value: ""}, + } + for _, o := range overrides { + for i := range props { + if props[i].Name == o.Name { + props[i].Value = o.Value + } + } + } + return props +} + +func findDivergence(t *testing.T, divs []Divergence, object, property string) Divergence { + t.Helper() + for _, d := range divs { + if d.Object == object && d.Property == property { + return d + } + } + t.Fatalf("no divergence found for object=%q property=%q in %+v", object, property, divs) + return Divergence{} +} + +// TestCompare_MissingColumnIsAbsent: a column dropped on one node only. +func TestCompare_MissingColumnIsAbsent(t *testing.T) { + table := "public.orders" + a := newSnapshot("n1").table(table). + column(table, "id", baseColumnProps()...). + column(table, "discount", baseColumnProps()...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + column(table, "id", baseColumnProps()...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"orders"}, a, b) + + d := findDivergence(t, divs, table+".discount", "") + if d.Rank != RankAbsent { + t.Fatalf("want RankAbsent, got %q", d.Rank) + } + if d.ValueOnA != "present" || d.ValueOnB != "absent" { + t.Fatalf("want presence present/absent, got %q/%q", d.ValueOnA, d.ValueOnB) + } +} + +// TestCompare_IntWideningIsNarrowedOnTheSmallerSide: int8 on n1, int4 on n2 +// — n2 is the narrow side. +func TestCompare_IntWideningIsNarrowedOnTheSmallerSide(t *testing.T) { + table := "public.customers" + a := newSnapshot("n1").table(table). + column(table, "id", baseColumnProps(Property{Name: "type", Value: "bigint"}, Property{Name: "type_name", Value: "int8"})...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + column(table, "id", baseColumnProps(Property{Name: "type", Value: "integer"}, Property{Name: "type_name", Value: "int4"})...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"customers"}, a, b) + + d := findDivergence(t, divs, table+".id", "type") + if d.Rank != RankNarrowed { + t.Fatalf("want RankNarrowed, got %q", d.Rank) + } + if d.NarrowSide != "n2" { + t.Fatalf("want narrow side n2 (int4), got %q", d.NarrowSide) + } +} + +// TestCompare_TextVsIntegerIsIncompatible: no widening relation applies. +func TestCompare_TextVsIntegerIsIncompatible(t *testing.T) { + table := "public.t" + a := newSnapshot("n1").table(table). + column(table, "x", baseColumnProps(Property{Name: "type", Value: "text"}, Property{Name: "type_name", Value: "text"})...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + column(table, "x", baseColumnProps(Property{Name: "type", Value: "integer"}, Property{Name: "type_name", Value: "int4"})...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"t"}, a, b) + + d := findDivergence(t, divs, table+".x", "type") + if d.Rank != RankIncompatible { + t.Fatalf("want RankIncompatible, got %q", d.Rank) + } +} + +// TestCompare_BpcharVsVarcharSameLengthIsEquivalentDiffering: char(n) and +// varchar(n) accept the same strings at the same declared length, but +// bpchar blank-pads a shorter value out to n characters and varchar does +// not, so the two nodes do not actually store or compare the same values - +// this is RankEquivalentDiffering, not "no difference". +func TestCompare_BpcharVsVarcharSameLengthIsEquivalentDiffering(t *testing.T) { + table := "public.t" + a := newSnapshot("n1").table(table). + // char(5): PostgreSQL stores the declared length as atttypmod-4. + column(table, "x", baseColumnProps(Property{Name: "type_name", Value: "bpchar"}, Property{Name: "type_mod", Value: "9"})...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + column(table, "x", baseColumnProps(Property{Name: "type_name", Value: "varchar"}, Property{Name: "type_mod", Value: "9"})...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"t"}, a, b) + + d := findDivergence(t, divs, table+".x", "type") + if d.Rank != RankEquivalentDiffering { + t.Fatalf("want RankEquivalentDiffering, got %q", d.Rank) + } +} + +// TestCompare_BpcharVsUnboundedTextIsEquivalentDiffering: the same holds +// even when neither side is length-limited - bare "char" (unbounded, like +// text) against text itself. It is bpchar's padding behaviour that +// differs, not a length mismatch, so the unbounded case is not exempt. +func TestCompare_BpcharVsUnboundedTextIsEquivalentDiffering(t *testing.T) { + table := "public.t" + a := newSnapshot("n1").table(table). + // type_mod stays at baseColumnProps' default of -1: unbounded. + column(table, "x", baseColumnProps(Property{Name: "type_name", Value: "bpchar"})...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + column(table, "x", baseColumnProps(Property{Name: "type_name", Value: "text"})...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"t"}, a, b) + + d := findDivergence(t, divs, table+".x", "type") + if d.Rank != RankEquivalentDiffering { + t.Fatalf("want RankEquivalentDiffering, got %q", d.Rank) + } +} + +// TestCompare_BpcharShorterThanVarcharIsNarrowed: char(5) accepts strictly +// fewer strings than varchar(10), so the difference in declared length must +// still be reported as a narrowing, not folded into the padding-only +// RankEquivalentDiffering case that applies when the declared lengths match. +func TestCompare_BpcharShorterThanVarcharIsNarrowed(t *testing.T) { + table := "public.t" + a := newSnapshot("n1").table(table). + // char(5): PostgreSQL stores the declared length as atttypmod-4. + column(table, "x", baseColumnProps(Property{Name: "type_name", Value: "bpchar"}, Property{Name: "type_mod", Value: "9"})...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + // varchar(10). + column(table, "x", baseColumnProps(Property{Name: "type_name", Value: "varchar"}, Property{Name: "type_mod", Value: "14"})...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"t"}, a, b) + + d := findDivergence(t, divs, table+".x", "type") + if d.Rank != RankNarrowed { + t.Fatalf("want RankNarrowed, got %q", d.Rank) + } + if d.NarrowSide != "n1" { + t.Fatalf("want n1 (char(5)) as the narrow side, got %q", d.NarrowSide) + } +} + +// TestCompare_BpcharBoundedVsUnboundedTextIsNarrowed: char(5) accepts +// strictly fewer strings than unbounded text, so a bounded bpchar against an +// unbounded text-like column is a narrowing, not the padding-only +// equivalence that applies when both sides are unbounded. +func TestCompare_BpcharBoundedVsUnboundedTextIsNarrowed(t *testing.T) { + table := "public.t" + a := newSnapshot("n1").table(table). + // char(5): PostgreSQL stores the declared length as atttypmod-4. + column(table, "x", baseColumnProps(Property{Name: "type_name", Value: "bpchar"}, Property{Name: "type_mod", Value: "9"})...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + // type_mod stays at baseColumnProps' default of -1: unbounded. + column(table, "x", baseColumnProps(Property{Name: "type_name", Value: "text"})...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"t"}, a, b) + + d := findDivergence(t, divs, table+".x", "type") + if d.Rank != RankNarrowed { + t.Fatalf("want RankNarrowed, got %q", d.Rank) + } + if d.NarrowSide != "n1" { + t.Fatalf("want n1 (char(5)) as the narrow side, got %q", d.NarrowSide) + } +} + +// TestCompare_TimestampVsTimestamptzIsEquivalentDiffering: same values fit, +// meaning differs. +func TestCompare_TimestampVsTimestamptzIsEquivalentDiffering(t *testing.T) { + table := "public.events" + a := newSnapshot("n1").table(table). + column(table, "at", baseColumnProps(Property{Name: "type", Value: "timestamp with time zone"}, Property{Name: "type_name", Value: "timestamptz"})...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + column(table, "at", baseColumnProps(Property{Name: "type", Value: "timestamp without time zone"}, Property{Name: "type_name", Value: "timestamp"})...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"events"}, a, b) + + d := findDivergence(t, divs, table+".at", "type") + if d.Rank != RankEquivalentDiffering { + t.Fatalf("want RankEquivalentDiffering, got %q", d.Rank) + } +} + +// TestCompare_TimestampPairWithEqualPrecisionIsEquivalentDiffering: the pair +// above, but with a precision named on both sides. Equal precision means the +// same values still fit, so only the time zone meaning differs. +func TestCompare_TimestampPairWithEqualPrecisionIsEquivalentDiffering(t *testing.T) { + table := "public.events" + a := newSnapshot("n1").table(table). + column(table, "at", baseColumnProps( + Property{Name: "type_name", Value: "timestamptz"}, + Property{Name: "type_mod", Value: "3"})...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + column(table, "at", baseColumnProps( + Property{Name: "type_name", Value: "timestamp"}, + Property{Name: "type_mod", Value: "3"})...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"events"}, a, b) + + d := findDivergence(t, divs, table+".at", "type") + if d.Rank != RankEquivalentDiffering { + t.Fatalf("want RankEquivalentDiffering, got %q", d.Rank) + } +} + +// TestCompare_TimestampPairWithDifferentPrecisionIsIncompatible: for these +// types atttypmod is the fractional-second precision itself, and a lower +// precision rounds the value away - timestamp(3) stores .123456 as .123. So +// the two sides do not hold the same values, which is what +// equivalent-differing claims, and the pair must not be ranked as if they +// did. Two columns of the same name and different precision already fall +// through to incompatible; this keeps the cross-name pair consistent with +// that, instead of giving the pair with more differences the milder rank. +func TestCompare_TimestampPairWithDifferentPrecisionIsIncompatible(t *testing.T) { + table := "public.events" + a := newSnapshot("n1").table(table). + column(table, "at", baseColumnProps( + Property{Name: "type_name", Value: "timestamptz"}, + Property{Name: "type_mod", Value: "3"})...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + column(table, "at", baseColumnProps( + Property{Name: "type_name", Value: "timestamp"}, + Property{Name: "type_mod", Value: "6"})...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"events"}, a, b) + + d := findDivergence(t, divs, table+".at", "type") + if d.Rank != RankIncompatible { + t.Fatalf("want RankIncompatible, got %q", d.Rank) + } +} + +// TestCompare_CollationVersionDifferenceIsEquivalentDiffering. +func TestCompare_CollationVersionDifferenceIsEquivalentDiffering(t *testing.T) { + table := "public.customers" + a := newSnapshot("n1").table(table). + column(table, "email", baseColumnProps( + Property{Name: "type", Value: "text"}, Property{Name: "type_name", Value: "text"}, + Property{Name: "collation", Value: "en_US.utf8/c/2.36"})...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + column(table, "email", baseColumnProps( + Property{Name: "type", Value: "text"}, Property{Name: "type_name", Value: "text"}, + Property{Name: "collation", Value: "en_US.utf8/c/2.31"})...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"customers"}, a, b) + + d := findDivergence(t, divs, table+".email", "collation") + if d.Rank != RankEquivalentDiffering { + t.Fatalf("want RankEquivalentDiffering, got %q", d.Rank) + } +} + +// TestCompare_DeltaApplyOptionDifferenceIsEquivalentDiffering: a Spock +// delta-apply column on one node, an ordinary column on the other. This +// package does not know what "delta-apply" means; it only compares the raw +// attoptions text. +func TestCompare_DeltaApplyOptionDifferenceIsEquivalentDiffering(t *testing.T) { + table := "public.accounts" + a := newSnapshot("n1").table(table). + column(table, "balance", baseColumnProps( + Property{Name: "type", Value: "numeric"}, Property{Name: "type_name", Value: "numeric"}, + Property{Name: "options", Value: "{log_old_value=true,delta_apply_function=spock.delta_apply}"})...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + column(table, "balance", baseColumnProps( + Property{Name: "type", Value: "numeric"}, Property{Name: "type_name", Value: "numeric"})...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"accounts"}, a, b) + + d := findDivergence(t, divs, table+".balance", "options") + if d.Rank != RankEquivalentDiffering { + t.Fatalf("want RankEquivalentDiffering, got %q", d.Rank) + } +} + +// TestCompare_KeyOpclassDifferenceIsEquivalentDiffering. +func TestCompare_KeyOpclassDifferenceIsEquivalentDiffering(t *testing.T) { + table := "public.customers" + a := newSnapshot("n1").table(table). + key(table, Property{Name: "replica_identity", Value: "i"}, + Property{Name: "key_columns", Value: "email"}, + Property{Name: "key_opclasses", Value: "text_ops"}). + constraints(table).build() + b := newSnapshot("n2").table(table). + key(table, Property{Name: "replica_identity", Value: "i"}, + Property{Name: "key_columns", Value: "email"}, + Property{Name: "key_opclasses", Value: "text_pattern_ops"}). + constraints(table).build() + + divs := Compare("public", []string{"customers"}, a, b) + + d := findDivergence(t, divs, table, "key_opclasses") + if d.Rank != RankEquivalentDiffering { + t.Fatalf("want RankEquivalentDiffering, got %q", d.Rank) + } +} + +// TestCompare_ReplicaIdentityMismatchIsIncompatible: PRIMARY KEY vs FULL, +// where conflicts would resolve differently on each side. +func TestCompare_ReplicaIdentityMismatchIsIncompatible(t *testing.T) { + table := "public.orders" + a := newSnapshot("n1").table(table). + key(table, Property{Name: "replica_identity", Value: "d"}). + constraints(table).build() + b := newSnapshot("n2").table(table). + key(table, Property{Name: "replica_identity", Value: "f"}). + constraints(table).build() + + divs := Compare("public", []string{"orders"}, a, b) + + d := findDivergence(t, divs, table, "replica_identity") + if d.Rank != RankIncompatible { + t.Fatalf("want RankIncompatible, got %q", d.Rank) + } +} + +// TestCompare_ConstraintNamesDifferDefinitionsMatch_NoDivergence checks that +// matching two constraints goes by definition text, never by PostgreSQL's +// invented names, or every pair of independently-created databases would +// report false constraint differences. +func TestCompare_ConstraintNamesDifferDefinitionsMatch_NoDivergence(t *testing.T) { + table := "public.customers" + // Same constraint, different auto-generated names — GetConstraintDescriptors + // never reads conname, so both snapshots record only the definition text. + a := newSnapshot("n1").table(table).key(table). + constraints(table, "u|deferrable=false|validated=true|UNIQUE (email)").build() + b := newSnapshot("n2").table(table).key(table). + constraints(table, "u|deferrable=false|validated=true|UNIQUE (email)").build() + + divs := Compare("public", []string{"customers"}, a, b) + if len(divs) != 0 { + t.Fatalf("want no divergences for identical constraint definitions, got %+v", divs) + } +} + +// TestCompare_ExtraCheckConstraintIsNarrowedNotAbsent checks that a CHECK +// only one node has is ranked as a narrowing of that node, the same way +// compareDomainChecks ranks a domain's one-sided CHECK: the node holding it +// accepts strictly fewer rows. Ranking it RankAbsent instead would give the +// same change two different exit codes depending on whether the CHECK sat +// on a table or on a domain. +func TestCompare_ExtraCheckConstraintIsNarrowedNotAbsent(t *testing.T) { + table := "public.customers" + a := newSnapshot("n1").table(table).key(table). + constraints(table, "c|deferrable=false|validated=true|CHECK (age >= 0)").build() + b := newSnapshot("n2").table(table).key(table).constraints(table).build() + + divs := Compare("public", []string{"customers"}, a, b) + if len(divs) != 1 { + t.Fatalf("want exactly one divergence, got %d: %+v", len(divs), divs) + } + d := divs[0] + if d.Kind != "constraint" || d.ValueOnB != "(absent)" { + t.Fatalf("expected the extra constraint on n1 to be reported, got %+v", d) + } + if d.Rank != RankNarrowed || d.NarrowSide != "n1" { + t.Fatalf("want narrowed on n1, got %q on %q", d.Rank, d.NarrowSide) + } +} + +// TestCompare_ExtraNonCheckConstraintStaysAbsent checks that the narrowing +// reasoning above is confined to CHECKs. A UNIQUE or FOREIGN KEY one node +// lacks does not make the other node's accepted values a subset of it — +// the two disagree about a constraint, which is RankAbsent. +func TestCompare_ExtraNonCheckConstraintStaysAbsent(t *testing.T) { + table := "public.customers" + a := newSnapshot("n1").table(table).key(table). + constraints(table, "u|deferrable=false|validated=true|UNIQUE (email)").build() + b := newSnapshot("n2").table(table).key(table).constraints(table).build() + + divs := Compare("public", []string{"customers"}, a, b) + if len(divs) != 1 { + t.Fatalf("want exactly one divergence, got %d: %+v", len(divs), divs) + } + if divs[0].Rank != RankAbsent { + t.Fatalf("want absent for a one-sided UNIQUE, got %q", divs[0].Rank) + } +} + +// TestCompare_CheckOnEachSideIsNotNarrowed checks that when each node has a +// CHECK the other lacks, neither is called the narrow side: neither value +// set contains the other, so both findings stay RankAbsent. +func TestCompare_CheckOnEachSideIsNotNarrowed(t *testing.T) { + table := "public.customers" + a := newSnapshot("n1").table(table).key(table). + constraints(table, "c|deferrable=false|validated=true|CHECK (age >= 0)").build() + b := newSnapshot("n2").table(table).key(table). + constraints(table, "c|deferrable=false|validated=true|CHECK (age >= 18)").build() + + divs := Compare("public", []string{"customers"}, a, b) + if len(divs) != 2 { + t.Fatalf("want two divergences, got %d: %+v", len(divs), divs) + } + for _, d := range divs { + if d.Rank != RankAbsent { + t.Fatalf("want absent when each side has its own CHECK, got %q: %+v", d.Rank, d) + } + } +} + +// TestCompare_ExtraCheckAgainstExtraNonCheckIsNotNarrowed checks the case +// between the two above: one node has an extra CHECK and the other an extra +// FOREIGN KEY. The CHECK alone would make n1 the narrow side, but n2 holds a +// constraint of its own that n1 does not, so n2 rejects rows n1 accepts. +// Neither set of accepted rows contains the other, and neither finding may +// be called a narrowing. +func TestCompare_ExtraCheckAgainstExtraNonCheckIsNotNarrowed(t *testing.T) { + table := "public.customers" + a := newSnapshot("n1").table(table).key(table). + constraints(table, "c|deferrable=false|validated=true|CHECK (age >= 0)").build() + b := newSnapshot("n2").table(table).key(table). + constraints(table, "f|deferrable=false|validated=true|FOREIGN KEY (org_id) REFERENCES orgs(id)").build() + + divs := Compare("public", []string{"customers"}, a, b) + if len(divs) != 2 { + t.Fatalf("want two divergences, got %d: %+v", len(divs), divs) + } + for _, d := range divs { + if d.Rank != RankAbsent { + t.Fatalf("want absent when each side has an extra constraint, got %q: %+v", d.Rank, d) + } + if d.NarrowSide != "" { + t.Fatalf("no side may be called narrow here, got %q: %+v", d.NarrowSide, d) + } + } +} + +// TestCompare_DateAgainstTimestampIsIncompatible checks that date is not +// treated as a narrower timestamp. It looks like one, but neither value set +// contains the other: a timestamp carries a time of day that a date cannot +// hold, and date reaches 5874897 AD while the timestamp types stop at +// 294276 AD - PostgreSQL itself refuses '5874897-12-31'::date::timestamp +// with "date out of range for timestamp". +func TestCompare_DateAgainstTimestampIsIncompatible(t *testing.T) { + for _, other := range []string{"timestamp", "timestamptz"} { + table := "public.events" + a := newSnapshot("n1").table(table). + column(table, "at", baseColumnProps(Property{Name: "type_name", Value: "date"})...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + column(table, "at", baseColumnProps(Property{Name: "type_name", Value: other})...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"events"}, a, b) + + d := findDivergence(t, divs, table+".at", "type") + if d.Rank != RankIncompatible { + t.Fatalf("date against %s: want RankIncompatible, got %q", other, d.Rank) + } + if d.NarrowSide != "" { + t.Fatalf("date against %s: no side is the narrow one, got %q", other, d.NarrowSide) + } + } +} + +// timestampColumn builds the two snapshots the timestamp precision tests +// need: one column of the given type name and modifier on each node. +func timestampColumn(t *testing.T, aName, aMod, bName, bMod string) []Divergence { + t.Helper() + table := "public.events" + a := newSnapshot("n1").table(table). + column(table, "at", baseColumnProps( + Property{Name: "type_name", Value: aName}, + Property{Name: "type_mod", Value: aMod})...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + column(table, "at", baseColumnProps( + Property{Name: "type_name", Value: bName}, + Property{Name: "type_mod", Value: bMod})...). + key(table).constraints(table).build() + return Compare("public", []string{"events"}, a, b) +} + +// TestCompare_ImplicitAndExplicitTimestampPrecisionAreOneType: an +// unspecified modifier is stored as -1 and means the type's full precision, +// which for both timestamp types is 6. So "timestamp" and "timestamp(6)" are +// one type written two ways, and there is nothing to report. +func TestCompare_ImplicitAndExplicitTimestampPrecisionAreOneType(t *testing.T) { + if divs := timestampColumn(t, "timestamp", "-1", "timestamp", "6"); len(divs) != 0 { + t.Fatalf("timestamp against timestamp(6) is the same type, got %+v", divs) + } + if divs := timestampColumn(t, "timestamptz", "6", "timestamptz", "-1"); len(divs) != 0 { + t.Fatalf("timestamptz(6) against timestamptz is the same type, got %+v", divs) + } +} + +// TestCompare_LowerTimestampPrecisionIsNarrowed: at the same type name, the +// side with fewer fractional digits holds strictly fewer values - every +// timestamp(3) value is exactly representable as timestamp(6), and not the +// other way round. That is the definition of RankNarrowed, so reporting it +// as incompatible would claim the two sets are unrelated when one contains +// the other. +func TestCompare_LowerTimestampPrecisionIsNarrowed(t *testing.T) { + cases := []struct { + name string + aName, aMod string + bName, bMod string + wantNarrowSide string + }{ + {"explicit against explicit", "timestamp", "3", "timestamp", "6", "n1"}, + {"the narrow side on n2", "timestamp", "6", "timestamp", "3", "n2"}, + {"explicit against implicit 6", "timestamp", "3", "timestamp", "-1", "n1"}, + {"timestamptz keeps its own rule", "timestamptz", "-1", "timestamptz", "0", "n2"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + divs := timestampColumn(t, c.aName, c.aMod, c.bName, c.bMod) + d := findDivergence(t, divs, "public.events.at", "type") + if d.Rank != RankNarrowed { + t.Fatalf("want RankNarrowed, got %q", d.Rank) + } + if d.NarrowSide != c.wantNarrowSide { + t.Fatalf("want the narrow side on %s, got %q", c.wantNarrowSide, d.NarrowSide) + } + }) + } +} + +// TestCompare_TimestampPairAcrossNamesAtOnePrecision: the time zone +// difference on its own is still equivalent-differing, including when one +// side spells out the precision the other leaves implicit. +func TestCompare_TimestampPairAcrossNamesAtOnePrecision(t *testing.T) { + for _, c := range [][4]string{ + {"timestamptz", "-1", "timestamp", "6"}, + {"timestamptz", "3", "timestamp", "3"}, + } { + divs := timestampColumn(t, c[0], c[1], c[2], c[3]) + d := findDivergence(t, divs, "public.events.at", "type") + if d.Rank != RankEquivalentDiffering { + t.Fatalf("%s(%s) against %s(%s): want RankEquivalentDiffering, got %q", + c[0], c[1], c[2], c[3], d.Rank) + } + } +} + +// TestCompare_ColumnOrderDoesNotMatter checks that building two snapshots +// with columns added in different orders produces no divergence, since +// attnum is never collected. +func TestCompare_ColumnOrderDoesNotMatter(t *testing.T) { + table := "public.t" + a := newSnapshot("n1").table(table). + column(table, "a", baseColumnProps()...). + column(table, "b", baseColumnProps()...). + key(table).constraints(table).build() + b := newSnapshot("n2").table(table). + column(table, "b", baseColumnProps()...). + column(table, "a", baseColumnProps()...). + key(table).constraints(table).build() + + divs := Compare("public", []string{"t"}, a, b) + if len(divs) != 0 { + t.Fatalf("want no divergences when only column insertion order differs, got %+v", divs) + } +} + +// TestCompare_SameDomainDifferentLocalDetailsOnlyNoDivergence is a +// regression test: two nodes' own OIDs for a domain never reach this +// package (ColumnDescriptor.TypeOID stays node-local — see collect.go), so +// two byte-identical domains must compare as identical regardless of what +// OID a pg_dump/restore or an independent CREATE DOMAIN happened to assign +// on each side. +func TestCompare_SameDomainDifferentLocalDetailsOnlyNoDivergence(t *testing.T) { + table := "public.city" + domainProps := func() []Property { + return []Property{ + {Name: "basetype_namespace", Value: "pg_catalog"}, + {Name: "basetype_name", Value: "numeric"}, + {Name: "basetypmod", Value: "-1"}, + {Name: "notnull", Value: "false"}, + {Name: "default", Value: ""}, + } + } + a := newSnapshot("n1").table(table). + column(table, "budget", userTypeColumnProps("public", "city_budget", "d")...). + key(table).constraints(table). + domain("public.city_budget", domainProps()...).build() + b := newSnapshot("n2").table(table). + column(table, "budget", userTypeColumnProps("public", "city_budget", "d")...). + key(table).constraints(table). + domain("public.city_budget", domainProps()...).build() + + divs := Compare("public", []string{"city"}, a, b) + if len(divs) != 0 { + t.Fatalf("want no divergences for an identical domain reached via two different OIDs, got %+v", divs) + } +} + +// TestCompare_DomainCheckDroppedOnOneSideIsNarrowed covers two freshly +// built nodes that happen to assign a domain the same OID, then +// `ALTER DOMAIN pos DROP CONSTRAINT pos_check` runs on only one of them. +// This must be caught by descending into the domain's own CHECK set the +// same way table constraints are compared. +func TestCompare_DomainCheckDroppedOnOneSideIsNarrowed(t *testing.T) { + table := "public.readings" + base := func() []Property { + return []Property{ + {Name: "basetype_namespace", Value: "pg_catalog"}, + {Name: "basetype_name", Value: "int4"}, + {Name: "basetypmod", Value: "-1"}, + {Name: "notnull", Value: "false"}, + {Name: "default", Value: ""}, + } + } + a := newSnapshot("n1").table(table). + column(table, "pos", userTypeColumnProps("public", "pos", "d")...). + key(table).constraints(table). + domain("public.pos", append(base(), Property{Name: "check", Value: "CHECK (VALUE >= 0)"})...).build() + b := newSnapshot("n2").table(table). + column(table, "pos", userTypeColumnProps("public", "pos", "d")...). + key(table).constraints(table). + domain("public.pos", base()...).build() + + divs := Compare("public", []string{"readings"}, a, b) + + d := findDivergence(t, divs, "public.pos", "check") + if d.Rank != RankNarrowed { + t.Fatalf("want RankNarrowed, got %q", d.Rank) + } + if d.NarrowSide != "n1" { + t.Fatalf("want n1 (the side that kept the stricter CHECK) as the narrow side, got %q", d.NarrowSide) + } +} + +// TestCompare_DomainBaseTypeNarrowedUsesClassifyType: a domain over +// varchar(20) on one side, the same domain name over varchar(10) on the +// other — the base-type change is classified with classifyType, the same +// as a column's declared type, not just flagged incompatible. +func TestCompare_DomainBaseTypeNarrowedUsesClassifyType(t *testing.T) { + table := "public.t" + a := newSnapshot("n1").table(table). + column(table, "code", userTypeColumnProps("public", "code_t", "d")...). + key(table).constraints(table). + domain("public.code_t", + Property{Name: "basetype_namespace", Value: "pg_catalog"}, + Property{Name: "basetype_name", Value: "varchar"}, + Property{Name: "basetypmod", Value: "24"}, // varchar(20): atttypmod = declared length + 4 + Property{Name: "notnull", Value: "false"}, Property{Name: "default", Value: ""}).build() + b := newSnapshot("n2").table(table). + column(table, "code", userTypeColumnProps("public", "code_t", "d")...). + key(table).constraints(table). + domain("public.code_t", + Property{Name: "basetype_namespace", Value: "pg_catalog"}, + Property{Name: "basetype_name", Value: "varchar"}, + Property{Name: "basetypmod", Value: "14"}, // varchar(10) + Property{Name: "notnull", Value: "false"}, Property{Name: "default", Value: ""}).build() + + divs := Compare("public", []string{"t"}, a, b) + + d := findDivergence(t, divs, "public.code_t", "basetype") + if d.Rank != RankNarrowed { + t.Fatalf("want RankNarrowed, got %q", d.Rank) + } + if d.NarrowSide != "n2" { + t.Fatalf("want n2 (varchar(10), the shorter side) as the narrow side, got %q", d.NarrowSide) + } +} + +// TestCompare_EnumLabelAddedIsIncompatible covers `ALTER TYPE mood ADD +// VALUE 'furious'` run on only one node. +func TestCompare_EnumLabelAddedIsIncompatible(t *testing.T) { + table := "public.t" + a := newSnapshot("n1").table(table). + column(table, "m", userTypeColumnProps("public", "mood", "e")...). + key(table).constraints(table). + enum("public.mood", "happy,sad").build() + b := newSnapshot("n2").table(table). + column(table, "m", userTypeColumnProps("public", "mood", "e")...). + key(table).constraints(table). + enum("public.mood", "happy,sad,furious").build() + + divs := Compare("public", []string{"t"}, a, b) + + d := findDivergence(t, divs, "public.mood", "labels") + if d.Rank != RankIncompatible { + t.Fatalf("want RankIncompatible, got %q", d.Rank) + } +} + +// TestCompare_CompositeAttributeTypeChangedIsReported: a composite type's +// own attribute changed type on one side. Attribute order is what +// Property.Name's "attr:%04d:name" index encodes, so two attributes +// swapping declaration order is itself a real difference for a composite +// type. +func TestCompare_CompositeAttributeTypeChangedIsReported(t *testing.T) { + table := "public.t" + // Built through packAttr, the same as collect.go itself does, instead + // of a hand-written string: a value that does not come from packAttr + // fails unpackAttr and only exercises classifyAttr's "cannot be + // unpacked" fallback, never the real int4-vs-int8 narrowing path this + // test means to cover. f1 and f2 get distinct ordinals (1 and 2, as + // two attributes of one composite type would), even though only f1 + // changes here. + aInt4 := packAttr("pg_catalog", "int4", "b", -1, "") + bInt8 := packAttr("pg_catalog", "int8", "b", -1, "") + text := packAttr("pg_catalog", "text", "b", -1, "") + a := newSnapshot("n1").table(table). + column(table, "thing", userTypeColumnProps("public", "things", "c")...). + key(table).constraints(table). + composite("public.things", + Property{Name: "attr:0001:f1", Value: aInt4}, + Property{Name: "attr:0002:f2", Value: text}).build() + b := newSnapshot("n2").table(table). + column(table, "thing", userTypeColumnProps("public", "things", "c")...). + key(table).constraints(table). + composite("public.things", + Property{Name: "attr:0001:f1", Value: bInt8}, + Property{Name: "attr:0002:f2", Value: text}).build() + + divs := Compare("public", []string{"t"}, a, b) + + // The property is named for the reader — "attribute 1 (f1)", carrying + // the attribute's own attnum — while the + // index in the underlying "attr:%04d:name" key is what kept the + // comparison in declaration order. + d := findDivergence(t, divs, "public.things", "attribute 1 (f1)") + // int4 -> int8 narrows exactly as it does for a table column (see + // classifyAttr's own doc comment): n1's int4 accepts a strict subset + // of what n2's int8 does. + if d.Rank != RankNarrowed { + t.Fatalf("want RankNarrowed, got %q", d.Rank) + } + if d.NarrowSide != "n1" { + t.Fatalf("want n1 as the narrow side, got %q", d.NarrowSide) + } + if d.ValueOnA != aInt4 || d.ValueOnB != bInt8 { + t.Fatalf("expected the raw packed attribute values to be shown when no display text was collected, got %q / %q", + d.ValueOnA, d.ValueOnB) + } +} + +// TestPackAttrRoundTripsArbitraryCollation checks that packAttr/unpackAttr +// round-trip regardless of what the collation field contains. A collation +// name is a quoted identifier here (see collect.go's own collation string, +// "namespace.name/provider/version"), so it can contain punctuation a +// delimiter-based pack would need to reserve for itself. +func TestPackAttrRoundTripsArbitraryCollation(t *testing.T) { + cases := []string{ + "", + "public.\"und-x-icu\"/i/153.94", + "3:not-a-length-prefix", + "trailing\x1fseparator", + } + for _, collation := range cases { + packed := packAttr("pg_catalog", "text", "b", 20, collation) + ns, name, kind, mod, coll, ok := unpackAttr(packed) + if !ok { + t.Errorf("unpackAttr could not decode packAttr(..., %q)", collation) + continue + } + if ns != "pg_catalog" || name != "text" || kind != "b" || mod != 20 || coll != collation { + t.Errorf("packAttr(..., %q) round-tripped to (%q, %q, %q, %d, %q)", + collation, ns, name, kind, mod, coll) + } + } +} + +// TestCompare_RangeSubtypeChangedIsIncompatible: a range type's subtype +// itself differs between nodes. +func TestCompare_RangeSubtypeChangedIsIncompatible(t *testing.T) { + table := "public.t" + a := newSnapshot("n1").table(table). + column(table, "r", userTypeColumnProps("public", "myrange", "r")...). + key(table).constraints(table). + rng("public.myrange", + Property{Name: "subtype", Value: "pg_catalog.text"}, + Property{Name: "collation", Value: "default/c/2.36"}, + Property{Name: "opclass", Value: "text_ops"}, + Property{Name: "canonical", Value: ""}, Property{Name: "subtype_diff", Value: ""}).build() + b := newSnapshot("n2").table(table). + column(table, "r", userTypeColumnProps("public", "myrange", "r")...). + key(table).constraints(table). + rng("public.myrange", + Property{Name: "subtype", Value: "pg_catalog.varchar"}, + Property{Name: "collation", Value: "default/c/2.36"}, + Property{Name: "opclass", Value: "text_ops"}, + Property{Name: "canonical", Value: ""}, Property{Name: "subtype_diff", Value: ""}).build() + + divs := Compare("public", []string{"t"}, a, b) + + d := findDivergence(t, divs, "public.myrange", "subtype") + if d.Rank != RankIncompatible { + t.Fatalf("want RankIncompatible, got %q", d.Rank) + } +} + +// TestDottedIdentifiersDoNotCollide is the regression test for the key +// ambiguity ObjectID removed: table "a.b" with a column "c", and table "a" +// with a column "b.c", must not collide on the same key. +func TestDottedIdentifiersDoNotCollide(t *testing.T) { + // typed spells out a column of the given built-in type, printing the + // type name as its own display text so the assertions below can tell + // the two colliding candidates apart by value. + typed := func(typeName string) []Property { + return baseColumnProps( + Property{Name: "type", Value: typeName}, + Property{Name: "type_name", Value: typeName}, + ) + } + build := func(dottedTableType, dottedColumnType string) Snapshot { + return newSnapshot("n1"). + table("public.a.b"). + column("public.a.b", "c", typed(dottedTableType)...). + table("public.a"). + column("public.a", "b.c", typed(dottedColumnType)...). + build() + } + + a := build("int4", "int4") + b := build("int4", "int8") + b.Node = "n2" + + tables := []string{"a", "a.b"} + divs := Compare("public", tables, a, b) + + if len(divs) != 1 { + t.Fatalf("want exactly one divergence, got %d: %+v", len(divs), divs) + } + if divs[0].Object != "public.a.b.c" || divs[0].Property != "type" { + t.Fatalf("unexpected object/property: %q / %q", divs[0].Object, divs[0].Property) + } + // Both candidate objects print as "public.a.b.c" — the display name is + // ambiguous by nature. What must not be ambiguous is which one was + // compared, so check the values: only table "a"'s column changed. + if divs[0].ValueOnA != "int4" || divs[0].ValueOnB != "int8" { + t.Fatalf("the wrong object was compared: %q vs %q", divs[0].ValueOnA, divs[0].ValueOnB) + } + if divs[0].Rank != RankNarrowed || divs[0].NarrowSide != "n1" { + t.Fatalf("want narrowed on n1, got %q on %q", divs[0].Rank, divs[0].NarrowSide) + } + + // And the reverse pairing must be seen just as well. + c := build("int8", "int4") + c.Node = "n2" + divs = Compare("public", tables, a, c) + if len(divs) != 1 || divs[0].ValueOnB != "int8" { + t.Fatalf("want one divergence on table %q, got %+v", "a.b", divs) + } +} + +// TestCompositeAttributeRankMatchesColumnRank checks that int4 -> int8 is +// RankNarrowed for a composite attribute, matching the rank a table column +// gets for the same change. +func TestCompositeAttributeRankMatchesColumnRank(t *testing.T) { + attr := func(typeName string) Property { + return Property{ + Name: "attr:0001:x", + Value: packAttr("pg_catalog", typeName, "b", -1, ""), + } + } + text := func(typeText string) Property { + return Property{Name: "attrtext:0001:x", Value: typeText} + } + + a := newSnapshot("n1").composite("public.pair", attr("int4"), text("integer")).build() + b := newSnapshot("n2").composite("public.pair", attr("int8"), text("bigint")).build() + + divs := Compare("public", nil, a, b) + if len(divs) != 1 { + t.Fatalf("want one divergence, got %d: %+v", len(divs), divs) + } + if divs[0].Rank != RankNarrowed || divs[0].NarrowSide != "n1" { + t.Fatalf("want narrowed on n1 (int4 is the narrow side), got %q on %q", divs[0].Rank, divs[0].NarrowSide) + } + if divs[0].ValueOnA != "integer" || divs[0].ValueOnB != "bigint" { + t.Fatalf("want the printable types, got %q vs %q", divs[0].ValueOnA, divs[0].ValueOnB) + } +} + +// TestCompareOrderIsDeterministic checks that findings come out in a +// stable order across runs, even though referenced types come out of a map +// and Go randomises map iteration. +func TestCompareOrderIsDeterministic(t *testing.T) { + build := func(node string, labels string) Snapshot { + return newSnapshot(node). + enum("public.zulu", labels). + enum("public.alpha", labels). + enum("public.mike", labels). + build() + } + a := build("n1", "x,y") + b := build("n2", "x,y,z") + + want := []string{"public.alpha", "public.mike", "public.zulu"} + for run := 0; run < 20; run++ { + divs := Compare("public", nil, a, b) + if len(divs) != len(want) { + t.Fatalf("run %d: want %d divergences, got %d", run, len(want), len(divs)) + } + for i, name := range want { + if divs[i].Object != name { + t.Fatalf("run %d: position %d is %q, want %q", run, i, divs[i].Object, name) + } + } + } +} + +// TestColumnTypeKindChangeIsIncompatible checks the case a comparison +// keyed only on (namespace, name, typmod) cannot see: an enum dropped on +// one node and recreated under the same name as a domain. Nothing about +// the name or the printed type moves, and the two type Objects no longer +// share an ObjectID, so before type_kind joined the identity this came out +// as a clean match. +func TestColumnTypeKindChangeIsIncompatible(t *testing.T) { + table := "public.orders" + build := func(node, kind string) Snapshot { + s := newSnapshot(node).table(table).key(table).constraints(table). + column(table, "status", userTypeColumnProps("public", "status", kind)...) + if kind == "e" { + s = s.enum("public.status", joinList([]string{"new", "done"})) + } else { + s = s.domain("public.status", + Property{Name: "basetype_namespace", Value: "pg_catalog"}, + Property{Name: "basetype_name", Value: "text"}, + Property{Name: "basetype_kind", Value: "b"}, + Property{Name: "basetypmod", Value: "-1"}, + Property{Name: "basetype_text", Value: "text"}, + Property{Name: "notnull", Value: "false"}, + Property{Name: "default", Value: ""}, + ) + } + return s.build() + } + + divs := Compare("public", []string{"orders"}, build("n1", "e"), build("n2", "d")) + + d := findDivergence(t, divs, "public.orders.status", "type") + if d.Rank != RankIncompatible { + t.Fatalf("want incompatible for enum replaced by a same-named domain, got %q", d.Rank) + } + // The two sides must not print identically, or the report reads as a + // tool malfunction: same name, same type text, "differs". + if d.ValueOnA == d.ValueOnB { + t.Fatalf("both sides print as %q; the kind difference is invisible", d.ValueOnA) + } +} + +// TestCompositeAttributeTypeKindChangeIsIncompatible is the same substitution +// one level down, inside a composite's attribute, where the packed attribute +// value is what carries type identity. +func TestCompositeAttributeTypeKindChangeIsIncompatible(t *testing.T) { + attr := func(kind string) Property { + return Property{ + Name: "attr:0001:status", + Value: packAttr("public", "status", kind, -1, ""), + } + } + + a := newSnapshot("n1").composite("public.row_t", attr("e")).build() + b := newSnapshot("n2").composite("public.row_t", attr("d")).build() + + divs := Compare("public", nil, a, b) + if len(divs) != 1 { + t.Fatalf("want one divergence, got %d: %+v", len(divs), divs) + } + if divs[0].Rank != RankIncompatible { + t.Fatalf("want incompatible for an attribute's type changing kind, got %q", divs[0].Rank) + } +} + +// TestDomainBaseTypeKindChangeIsIncompatible is the same substitution under +// a domain's base type. +func TestDomainBaseTypeKindChangeIsIncompatible(t *testing.T) { + build := func(node, kind string) Snapshot { + return newSnapshot(node).domain("public.code_t", + Property{Name: "basetype_namespace", Value: "public"}, + Property{Name: "basetype_name", Value: "code"}, + Property{Name: "basetype_kind", Value: kind}, + Property{Name: "basetypmod", Value: "-1"}, + Property{Name: "basetype_text", Value: "code"}, + Property{Name: "notnull", Value: "false"}, + Property{Name: "default", Value: ""}, + ).build() + } + + divs := Compare("public", nil, build("n1", "e"), build("n2", "d")) + if len(divs) != 1 { + t.Fatalf("want one divergence, got %d: %+v", len(divs), divs) + } + if divs[0].Rank != RankIncompatible { + t.Fatalf("want incompatible for a base type changing kind, got %q", divs[0].Rank) + } + if divs[0].ValueOnA == divs[0].ValueOnB { + t.Fatalf("both sides print as %q; the kind difference is invisible", divs[0].ValueOnA) + } +} + +// TestRangeSubtypeKindChangeIsReported is the same substitution under a +// range's subtype. The subtype name is unchanged, so a comparison that +// short-circuits on "the subtype text matches" reports nothing at all. +func TestRangeSubtypeKindChangeIsReported(t *testing.T) { + build := func(node, kind string) Snapshot { + return newSnapshot(node).rng("public.myrange", + Property{Name: "subtype", Value: "public.stamp"}, + Property{Name: "subtype_kind", Value: kind}, + Property{Name: "collation", Value: ""}, + Property{Name: "opclass", Value: "pg_catalog.datetime_ops"}, + Property{Name: "canonical", Value: ""}, + Property{Name: "subtype_diff", Value: ""}, + ).build() + } + + divs := Compare("public", nil, build("n1", "d"), build("n2", "b")) + if len(divs) != 1 { + t.Fatalf("want one divergence, got %d: %+v", len(divs), divs) + } + if divs[0].Property != "subtype" || divs[0].Rank != RankIncompatible { + t.Fatalf("want an incompatible subtype finding, got %+v", divs[0]) + } +} + +// TestJoinListKeepsDistinctListsDistinct pins the property that makes it +// safe to compare a list of values as one string. Enum labels and key +// column names are compared that way, and both may contain a comma, so a +// comma-joined ("a,b", "c") and ("a", "b,c") would compare equal — the +// same collision ObjectID exists to avoid, one level down in the values. +func TestJoinListKeepsDistinctListsDistinct(t *testing.T) { + collide := [][2][]string{ + {{"a,b", "c"}, {"a", "b,c"}}, + {{"x"}, {"x", ""}}, + {{"a.b", "c"}, {"a", "b.c"}}, + } + for _, pair := range collide { + if got, want := joinList(pair[0]), joinList(pair[1]); got == want { + t.Errorf("%q and %q both join to %q", pair[0], pair[1], got) + } + } + + // And the joined form must still change when the list does, including + // when only the order changes: an enum's label order is part of its + // identity. + if joinList([]string{"a", "b"}) == joinList([]string{"b", "a"}) { + t.Error("reordered labels join to the same string") + } +} + +// TestJoinListRoundTripsArbitraryContent checks that joinList/splitList +// round-trip a value regardless of what it contains - an enum label is a +// string literal, not an identifier, so nothing stops a user from writing +// one that contains exactly what a delimiter-based join would have picked +// to keep values apart, or one that happens to look like joinList's own +// packed form. +func TestJoinListRoundTripsArbitraryContent(t *testing.T) { + cases := [][]string{ + {"new\x1fdone"}, // a former delimiter, embedded in one label + {"3:foo", "bar"}, // looks like another packed field + {""}, // an empty label + {"a", "", "b"}, // an empty label in the middle of the list + {"5:00:00", "colons:everywhere"}, // digits and colons throughout + } + for _, values := range cases { + got, ok := splitList(joinList(values)) + if !ok { + t.Errorf("splitList could not decode joinList(%q)", values) + continue + } + if len(got) != len(values) { + t.Errorf("joinList(%q) round-tripped to %q", values, got) + continue + } + for i := range values { + if got[i] != values[i] { + t.Errorf("joinList(%q) round-tripped to %q", values, got) + break + } + } + } +} + +// TestEnumLabelsWithCommasDoNotCollide checks the same thing end to end: +// two label lists that a comma would flatten into the same text are +// reported as differing. +func TestEnumLabelsWithCommasDoNotCollide(t *testing.T) { + a := newSnapshot("n1").enum("public.tricky", joinList([]string{"a,b", "c"})).build() + b := newSnapshot("n2").enum("public.tricky", joinList([]string{"a", "b,c"})).build() + + divs := Compare("public", nil, a, b) + if len(divs) != 1 { + t.Fatalf("want the differing label lists reported, got %d: %+v", len(divs), divs) + } + if divs[0].Property != "labels" { + t.Fatalf("want a labels finding, got %+v", divs[0]) + } +} + +// TestDatabaseLocaleDifferenceIsReported checks that two nodes whose +// databases were created with different collations are reported even when +// every column matches — the case no per-column check can see, since an +// uncollated column records only "the default" on both nodes. +func TestDatabaseLocaleDifferenceIsReported(t *testing.T) { + table := "public.orders" + build := func(node, collate string) Snapshot { + return newSnapshot(node).table(table).key(table).constraints(table). + column(table, "name", baseColumnProps( + Property{Name: "type", Value: "text"}, + Property{Name: "type_name", Value: "text"}, + )...). + locale("appdb", collate, collate, "c", ""). + build() + } + + divs := Compare("public", []string{"orders"}, + build("n1", "en_US.UTF-8"), build("n2", "C")) + + d := findDivergence(t, divs, "appdb", "lc_collate") + if d.Rank != RankIncompatible { + t.Fatalf("want incompatible for differing database collations, got %q", d.Rank) + } + if d.ValueOnA != "en_US.UTF-8" || d.ValueOnB != "C" { + t.Fatalf("unexpected values: %q vs %q", d.ValueOnA, d.ValueOnB) + } +} + +// TestDatabaseLocaleMatchingIsSilent guards the other direction: the new +// database Object must not produce a finding on nodes that agree. +func TestDatabaseLocaleMatchingIsSilent(t *testing.T) { + build := func(node string) Snapshot { + return newSnapshot(node).locale("appdb", "C", "C", "c", "").build() + } + if divs := Compare("public", nil, build("n1"), build("n2")); len(divs) != 0 { + t.Fatalf("want no divergences for matching locales, got %+v", divs) + } +} + +// TestCompositeAttributeOrdinalsSurviveADrop checks that dropping one +// attribute does not make the attributes after it look changed too. The +// property key carries each attribute's own attnum, so a gap stays a gap +// instead of renumbering everything below it. +func TestCompositeAttributeOrdinalsSurviveADrop(t *testing.T) { + attr := func(attnum int, name, typeName string) []Property { + return []Property{ + {Name: fmt.Sprintf("attr:%04d:%s", attnum, name), + Value: packAttr("pg_catalog", typeName, "b", -1, "")}, + {Name: fmt.Sprintf("attrtext:%04d:%s", attnum, name), Value: typeName}, + } + } + compose := func(node string, attrs ...[]Property) Snapshot { + var props []Property + for _, a := range attrs { + props = append(props, a...) + } + return newSnapshot(node).composite("public.addr", props...).build() + } + + // n2 dropped attribute 2 ("zip"). The attributes that follow keep their + // attnums, so only the drop should be reported. + a := compose("n1", attr(1, "street", "text"), attr(2, "zip", "int4"), attr(3, "city", "text")) + b := compose("n2", attr(1, "street", "text"), attr(3, "city", "text")) + + divs := Compare("public", nil, a, b) + if len(divs) != 1 { + t.Fatalf("want only the dropped attribute reported, got %d: %+v", len(divs), divs) + } + if divs[0].Rank != RankAbsent { + t.Fatalf("want absent for the dropped attribute, got %q", divs[0].Rank) + } + if !strings.Contains(divs[0].Property, "zip") { + t.Fatalf("want the finding to name the dropped attribute, got %q", divs[0].Property) + } + // The ordinal printed is the attribute's own attnum, so a reader can + // line the finding up against \d+ on the type. + if !strings.Contains(divs[0].Property, "2") { + t.Fatalf("want the finding to carry attnum 2, got %q", divs[0].Property) + } +} + +// TestCompare_TableMissingFromSnapshotIsOneAbsentFinding pins what +// CollectSnapshot's existence gate is for: a table absent from one snapshot +// is one finding, on the table, not one per column. +func TestCompare_TableMissingFromSnapshotIsOneAbsentFinding(t *testing.T) { + table := "public.orders" + a := newSnapshot("n1").table(table). + column(table, "id", baseColumnProps()...). + column(table, "discount", baseColumnProps()...). + column(table, "note", baseColumnProps()...). + key(table).constraints(table).build() + b := newSnapshot("n2").build() + + divs := Compare("public", []string{"orders"}, a, b) + + if len(divs) != 1 { + t.Fatalf("want exactly one finding for a table missing from a snapshot, got %d: %+v", len(divs), divs) + } + if divs[0].Kind != "table" { + t.Fatalf("want the finding on the table, got kind %q: %+v", divs[0].Kind, divs[0]) + } + if divs[0].Rank != RankAbsent { + t.Fatalf("want RankAbsent, got %q", divs[0].Rank) + } + if divs[0].ValueOnA != "present" || divs[0].ValueOnB != "absent" { + t.Fatalf("want presence present/absent, got %q/%q", divs[0].ValueOnA, divs[0].ValueOnB) + } +} + +// TestFindingKey_ConstraintsOnOneTableStayDistinct: every constraint +// finding carries the table as Object and no Property, so Object+Kind+ +// Property alone would count several missing constraints as one. +func TestFindingKey_ConstraintsOnOneTableStayDistinct(t *testing.T) { + divs := []Divergence{ + {Object: "public.orders", Kind: "constraint", NodeA: "n1", NodeB: "n2", + ValueOnA: "c|deferrable=false|validated=true|CHECK (qty > 0)", ValueOnB: "(absent)"}, + {Object: "public.orders", Kind: "constraint", NodeA: "n1", NodeB: "n2", + ValueOnA: "u|deferrable=false|validated=true|UNIQUE (code)", ValueOnB: "(absent)"}, + {Object: "public.orders", Kind: "constraint", NodeA: "n1", NodeB: "n2", + ValueOnA: "f|deferrable=false|validated=true|FOREIGN KEY (cid) REFERENCES c(id)", ValueOnB: "(absent)"}, + } + + // The premise: all three look alike by object, kind and property, which + // is why those cannot be the whole key. + for _, d := range divs[1:] { + if d.Object != divs[0].Object || d.Kind != divs[0].Kind || d.Property != divs[0].Property { + t.Fatalf("this test needs all three findings to share object/kind/property, got %+v", d) + } + } + + keys := make(map[string]bool) + for _, d := range divs { + keys[d.FindingKey()] = true + } + if len(keys) != len(divs) { + t.Fatalf("want %d distinct keys for %d different constraints, got %d", len(divs), len(divs), len(keys)) + } +} + +// TestFindingKey_ConstraintKeySurvivesASideSwap: the odd node out is NodeA +// in one pair and NodeB in the next, so one missing constraint arrives with +// its values on opposite sides. Still one finding. +func TestFindingKey_ConstraintKeySurvivesASideSwap(t *testing.T) { + const def = "c|deferrable=false|validated=true|CHECK (qty > 0)" + onOneSide := Divergence{Object: "public.orders", Kind: "constraint", + NodeA: "n1", NodeB: "n3", ValueOnA: def, ValueOnB: "(absent)"} + onTheOther := Divergence{Object: "public.orders", Kind: "constraint", + NodeA: "n3", NodeB: "n2", ValueOnA: "(absent)", ValueOnB: def} + + if onOneSide.FindingKey() != onTheOther.FindingKey() { + t.Fatalf("the same constraint counted twice across a side swap:\n %q\n %q", + onOneSide.FindingKey(), onTheOther.FindingKey()) + } +} + +// TestFindingKey_NonConstraintFindingsIgnoreTheirValues: folding values +// into a key that does not need them would split one drifted column across +// the pairs that see it. +func TestFindingKey_NonConstraintFindingsIgnoreTheirValues(t *testing.T) { + onOneSide := Divergence{Object: "public.orders.qty", Kind: "column", Property: "type", + NodeA: "n1", NodeB: "n3", ValueOnA: "integer", ValueOnB: "bigint"} + onTheOther := Divergence{Object: "public.orders.qty", Kind: "column", Property: "type", + NodeA: "n3", NodeB: "n2", ValueOnA: "bigint", ValueOnB: "integer"} + + if onOneSide.FindingKey() != onTheOther.FindingKey() { + t.Fatalf("one drifted column counted twice:\n %q\n %q", + onOneSide.FindingKey(), onTheOther.FindingKey()) + } +} + +// TestFindingKey_DistinguishesPropertiesOnOneObject is the guard in the +// other direction: two properties of the same column are two findings. +func TestFindingKey_DistinguishesPropertiesOnOneObject(t *testing.T) { + typeDrift := Divergence{Object: "public.orders.qty", Kind: "column", Property: "type"} + notNullDrift := Divergence{Object: "public.orders.qty", Kind: "column", Property: "notnull"} + + if typeDrift.FindingKey() == notNullDrift.FindingKey() { + t.Fatalf("type and notnull on one column collapsed to one key: %q", typeDrift.FindingKey()) + } +} diff --git a/internal/consistency/schema/descriptordefs.go b/internal/consistency/schema/descriptordefs.go new file mode 100644 index 00000000..d30921d0 --- /dev/null +++ b/internal/consistency/schema/descriptordefs.go @@ -0,0 +1,130 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +// Package schema answers one question: how do two nodes' tables differ in +// structure? It knows nothing about replication or which tables were chosen +// for comparison — those are internal/consistency/topology's and +// internal/consistency/scope's jobs, and this package imports neither. Its +// input is a list of table names and a connection per node; its output is a +// flat list of differences with no notion of "origin" or "target". +// +// This split lets structure comparison run without a working replication +// topology — a freshly built node compared against a known-good one, a +// sanity check between two unrelated databases. +package schema + +// Property is one named, textual fact about a structural object: a column's +// type, a constraint's definition, an index's key columns. It is text and +// not a richer type because every kind of object needs the same treatment — +// sort, compare, print — and inventing a type per property kind would only +// get in the way of that. +type Property struct { + Name string + Value string +} + +// Object is one structural thing on one node: a table, a column, a +// constraint. Properties is always kept sorted by Name — this is what makes +// two independently-collected Objects comparable property-by-property, and +// what makes their checksum stable. What identifies an Object across nodes +// is its ObjectID (below), never this struct's own fields: Name here is a +// display string and nothing more. +type Object struct { + Kind string // "table" | "column" | "key" | "constraint" | "domain" | "range" | "composite" | "enum" + // Name is the object as a person should see it — "public.orders", + // "public.orders.discount", or for "domain"/"range"/"composite"/"enum" + // the referenced type's own name ("public.city_budget"), since those + // Objects exist once per distinct type, not once per column that + // happens to use it. It is built by joining identifiers with dots, so + // it is ambiguous by construction and must never be used as a key — + // that is ObjectID's job. + Name string + Properties []Property +} + +// ObjectID identifies one structural object within one Snapshot. +// +// It is a struct, not a "kind:schema.table.column" string: a PostgreSQL +// identifier may itself contain a dot, so table "a.b"'s column "c" and +// table "a"'s column "b.c" would both land on "column:public.a.b.c" under a +// concatenated key, silently colliding in Objects. Kept as separate fields, +// the two cannot collide, and no escaping scheme has to be trusted. +type ObjectID struct { + Kind string // as Object.Kind + Schema string // the namespace the object lives in + // Name is the table's name for the table-scoped kinds ("table", + // "column", "key", "constraint"), and the type's own name for + // "domain"/"range"/"composite"/"enum". + Name string + // Attribute is the column name for Kind "column", and empty for every + // other kind — a table and its own key or constraint set share + // (Schema, Name) and are told apart by Kind alone. + Attribute string +} + +// TableID, ColumnID, KeyID, ConstraintID and TypeID are the only ways an +// ObjectID is built, so that no caller has to remember which field a +// column name goes in. +func TableID(schema, table string) ObjectID { + return ObjectID{Kind: "table", Schema: schema, Name: table} +} + +// ColumnID identifies one column of one table. The column name goes in +// Attribute and the table's in Name, which is what keeps a table and its +// columns from sharing a key. +func ColumnID(schema, table, column string) ObjectID { + return ObjectID{Kind: "column", Schema: schema, Name: table, Attribute: column} +} + +// KeyID identifies one table's replica identity. There is one such Object +// per table, not one per key column, because the key is compared as a whole: +// the columns (a, b) and (b, a) are different keys. +func KeyID(schema, table string) ObjectID { + return ObjectID{Kind: "key", Schema: schema, Name: table} +} + +// ConstraintID identifies one table's constraints, all of which live on a +// single Object. They cannot be split into one Object per constraint the way +// columns are: PostgreSQL invents a name for an unnamed constraint, so there +// is no name that means the same thing on both nodes to key them by. +func ConstraintID(schema, table string) ObjectID { + return ObjectID{Kind: "constraint", Schema: schema, Name: table} +} + +// TypeID identifies a referenced type. kind is "domain", "range", +// "composite" or "enum"; schema is the type's own namespace, which is not +// necessarily the schema being compared (a column can use a domain that +// lives elsewhere). +func TypeID(kind, schema, typeName string) ObjectID { + return ObjectID{Kind: kind, Schema: schema, Name: typeName} +} + +// DatabaseID identifies the one database-wide Object a Snapshot carries: +// the collation settings every uncollated column inherits. There is exactly +// one per snapshot, so it needs no schema or name to tell it apart. +func DatabaseID() ObjectID { + return ObjectID{Kind: "database"} +} + +// Snapshot is everything Collect read from one node. Objects and +// TableColumns are both keyed by ObjectID for the reason spelled out there. +type Snapshot struct { + Node string + Objects map[ObjectID]Object + + // TableColumns maps a table's TableID to the names of its own columns, + // in the order Collect read them (already alphabetical — see the + // GetColumnDescriptors query). Comparing two tables' columns means + // comparing the union of these two lists, one name at a time; this + // field exists so that union can be built without scanning the whole + // Objects map. + TableColumns map[ObjectID][]string +} diff --git a/internal/consistency/schema/rank.go b/internal/consistency/schema/rank.go new file mode 100644 index 00000000..064fb4aa --- /dev/null +++ b/internal/consistency/schema/rank.go @@ -0,0 +1,261 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package schema + +// Rank names a kind of structural difference. Each one must be derivable +// from the two definitions alone — never from a guess about behaviour, +// data, or replication. These are plain strings, not an enum with severity +// baked in: how alarming each Rank is belongs in configuration, not here. +const ( + // RankAbsent: the object (or property) exists on one node and not the + // other. + RankAbsent = "absent" + // RankIncompatible: both sides have something, and neither's set of + // possible values contains the other's. + RankIncompatible = "incompatible" + // RankNarrowed: one side's set of possible values is a strict subset of + // the other's. NarrowSide names the narrow side. + RankNarrowed = "narrowed" + // RankEquivalentDiffering: the same values fit on both sides, but they + // are represented or handled differently (a different collation on an + // identical type, a different operator class, Spock's delta-apply + // option present on one side and not the other). + RankEquivalentDiffering = "equivalent-differing" + // RankCosmetic: does not affect the shape of the data. Nothing + // collect.go reads today actually produces this rank, since the + // cosmetic-only properties (column order, constraint names, non-unique + // indexes, storage parameters, ownership, comments) are either not + // collected yet or excluded by construction. + RankCosmetic = "cosmetic" +) + +// Well-known built-in type names this package knows how to reason about +// narrowing for, identified by their portable (namespace, typname) pair — +// never by OID. An OID is only stable within one already-running cluster: +// two independently initdb'd instances hand out different OIDs to every +// object created after initdb, so a type's OID differing between nodes says +// nothing about the schema. Type names in pg_catalog do not have this +// problem: PostgreSQL has never renamed int4, varchar, or timestamptz +// between major versions. +// +// This list is deliberately small; a type mismatch not covered here is +// reported as RankIncompatible. +const pgCatalog = "pg_catalog" + +const ( + nameInt2 = "int2" + nameInt4 = "int4" + nameInt8 = "int8" + nameFloat4 = "float4" + nameFloat8 = "float8" + nameVarchar = "varchar" + nameBpchar = "bpchar" + nameText = "text" + nameDate = "date" + nameTimestamp = "timestamp" + nameTimestampz = "timestamptz" +) + +// qname builds the (namespace, name) pair every type-identity comparison in +// this package keys on. +type qname struct{ namespace, name string } + +// pgqname builds the qname of a built-in type, which is every type this file +// knows how to reason about: a user-defined type is compared by its own +// definition (see compareReferencedTypes), not by the narrowing tables here. +func pgqname(name string) qname { return qname{pgCatalog, name} } + +var integerWidth = map[qname]int{pgqname(nameInt2): 2, pgqname(nameInt4): 4, pgqname(nameInt8): 8} +var floatWidth = map[qname]int{pgqname(nameFloat4): 4, pgqname(nameFloat8): 8} + +// maxTimestampPrecision is the fractional-second precision both timestamp +// types carry when none is declared. PostgreSQL never stores more than this: +// a larger declaration is clamped down to it, with a warning. +const maxTimestampPrecision int32 = 6 + +// timestampPrecision reports q's effective fractional-second precision, and +// whether q is one of the two timestamp types at all. +// +// An unspecified modifier is stored as -1 and stands for the type's full +// precision, which is maxTimestampPrecision. Normalising it here is what +// keeps "timestamp" and "timestamp(6)" from being read as two different +// types: they are one type written two ways, and PostgreSQL stores the same +// value for either. +func timestampPrecision(q qname, mod int32) (precision int32, ok bool) { + if q != pgqname(nameTimestamp) && q != pgqname(nameTimestampz) { + return 0, false + } + if mod < 0 { + return maxTimestampPrecision, true + } + return mod, true +} + +// classifyTimestamps ranks a difference between two timestamp columns. They +// can differ in the time zone half of the type, in the precision, or in +// both, and the three cases do not get the same answer. +// +// Precision alone is a narrowing. A lower precision holds strictly fewer +// values — every timestamp(3) is exactly representable as timestamp(6) — +// and PostgreSQL rounds the other direction away rather than refusing it, so +// the side with fewer digits is the narrow one. +// +// The time zone half alone is not a narrowing. Both sides hold the same +// instants, but a timestamp with no zone attached does not mean the same +// thing as a timestamptz once a DST boundary is crossed, which is what +// RankEquivalentDiffering is for. +// +// When both halves differ, no single rank states it: the value sets differ +// and the meaning differs. The pair is then reported as incompatible rather +// than as whichever half sounds milder. +func classifyTimestamps(aq qname, aPrec int32, bq qname, bPrec int32, nodeA, nodeB string) (rank, narrowSide string) { + switch { + case aPrec == bPrec && aq == bq: + return "", "" + case aPrec == bPrec: + return RankEquivalentDiffering, "" + case aq != bq: + return RankIncompatible, "" + case aPrec < bPrec: + return RankNarrowed, nodeA + default: + return RankNarrowed, nodeB + } +} + +// date against timestamp or timestamptz is deliberately NOT a narrowing, +// although it looks like one. RankNarrowed means one value set is a strict +// subset of the other, and here neither is: +// +// - a timestamp carries a time of day, which a date cannot represent, so +// the timestamp side is not contained in the date side; +// - date reaches 5874897 AD while both timestamp types stop at 294276 AD, +// so the date side is not contained in the timestamp side either. +// PostgreSQL refuses '5874897-12-31'::date::timestamp outright, with +// "date out of range for timestamp". +// +// Two sets that each hold values the other cannot is the definition of +// RankIncompatible, which classifyType reaches by falling through to its +// last line. There is no table here on purpose: an earlier version had one, +// and a table named "dateNarrowsInto" invites the next reader to add a type +// to it without asking whether the subset relation actually holds. + +// isTextLike reports whether q is one of the three character types whose +// differences classifyType ranks by declared length, rather than treating +// any change of name as a plain type change. +func isTextLike(q qname) bool { + return q == pgqname(nameVarchar) || q == pgqname(nameBpchar) || q == pgqname(nameText) +} + +// isBpchar reports whether q is bpchar, the one character type that pads a +// value out to its declared length. That padding is why two character types +// of the same length are still not the same thing - see classifyType. +func isBpchar(q qname) bool { + return q == pgqname(nameBpchar) +} + +// textLength returns a text-like type's declared length, and whether it is +// effectively unbounded: bare text, or varchar/char with no length +// modifier recorded (atttypmod < 0). PostgreSQL stores the declared length +// as atttypmod-4 for varchar/bpchar. +func textLength(q qname, mod int32) (length int, unbounded bool) { + if q == pgqname(nameText) || mod < 4 { + return 0, true + } + return int(mod) - 4, false +} + +// classifyType compares a column's declared type between two nodes and +// returns the Rank of the difference, and — for RankNarrowed — which +// node's side is the narrow one. Types are identified purely by their +// portable (namespace, name) pair plus typmod. An empty rank means "no +// meaningful difference"; a caller need not check equality before calling +// this. +func classifyType(aNamespace, aName string, aMod int32, bNamespace, bName string, bMod int32, nodeA, nodeB string) (rank, narrowSide string) { + aq := qname{aNamespace, aName} + bq := qname{bNamespace, bName} + + if aq == bq && aMod == bMod { + return "", "" + } + + if aPrec, aIsTimestamp := timestampPrecision(aq, aMod); aIsTimestamp { + if bPrec, bIsTimestamp := timestampPrecision(bq, bMod); bIsTimestamp { + return classifyTimestamps(aq, aPrec, bq, bPrec, nodeA, nodeB) + } + } + + if aw, aok := integerWidth[aq]; aok { + if bw, bok := integerWidth[bq]; bok { + return narrowByWidth(aw, bw, nodeA, nodeB) + } + } + if aw, aok := floatWidth[aq]; aok { + if bw, bok := floatWidth[bq]; bok { + return narrowByWidth(aw, bw, nodeA, nodeB) + } + } + if isTextLike(aq) && isTextLike(bq) { + // bpchar (CHAR(n)) blank-pads to its declared length; varchar and + // text do not. Length is checked first, the same way as for two + // non-bpchar text-like columns: a shorter declared length (or a + // bounded length against an unbounded side) still accepts strictly + // fewer strings, so it is RankNarrowed regardless of which side is + // bpchar. Only once both sides accept the same strings (equal + // declared length, or both unbounded) does the bpchar-vs-non-bpchar + // difference matter on its own: char(5) and varchar(5) accept the + // same strings but do not store or compare them the same way (a + // value shorter than 5 gets trailing spaces on the bpchar side + // only), so that case is RankEquivalentDiffering, not "no + // difference". + differsInBpchar := isBpchar(aq) != isBpchar(bq) + aLen, aUnb := textLength(aq, aMod) + bLen, bUnb := textLength(bq, bMod) + switch { + case aUnb && bUnb: + if differsInBpchar { + return RankEquivalentDiffering, "" + } + return "", "" + case aUnb: + return RankNarrowed, nodeB + case bUnb: + return RankNarrowed, nodeA + case aLen == bLen: + if differsInBpchar { + return RankEquivalentDiffering, "" + } + return "", "" + case aLen < bLen: + return RankNarrowed, nodeA + default: + return RankNarrowed, nodeB + } + } + return RankIncompatible, "" +} + +// narrowByWidth ranks a difference between two types of one family that +// differ only in how many bytes they hold: every value of the narrower type +// fits the wider one, so the narrow side is named and the rank is +// RankNarrowed. Equal widths mean the two are the same type under two names, +// which is reported as no difference at all. +func narrowByWidth(aWidth, bWidth int, nodeA, nodeB string) (string, string) { + switch { + case aWidth == bWidth: + return "", "" + case aWidth < bWidth: + return RankNarrowed, nodeA + default: + return RankNarrowed, nodeB + } +} diff --git a/internal/consistency/schema/report.go b/internal/consistency/schema/report.go new file mode 100644 index 00000000..b538fc72 --- /dev/null +++ b/internal/consistency/schema/report.go @@ -0,0 +1,138 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package schema + +import ( + "encoding/json" + "fmt" + "strings" +) + +// The five exit codes for the five Ranks, most severe last. A caller +// (schema-diff --compare=structure today) picks the single worst code +// across every Divergence found and exits with that, so a script wrapping +// ace can act on "how bad" without parsing the text report. RankAbsent +// shares RankIncompatible's code: both mean replication would break, either +// because a column vanished or because two types cannot hold each other's +// values. +const ( + ExitIdentical = 0 + ExitCosmetic = 16 + ExitEquivalentDiffering = 32 + ExitNarrowed = 48 + ExitIncompatible = 64 +) + +var rankExitCode = map[string]int{ + RankCosmetic: ExitCosmetic, + RankEquivalentDiffering: ExitEquivalentDiffering, + RankNarrowed: ExitNarrowed, + RankIncompatible: ExitIncompatible, + RankAbsent: ExitIncompatible, +} + +// WorstExitCode returns the exit code for the single most severe Rank +// present in divs, or ExitIdentical if divs is empty. An unrecognised Rank +// is treated as ExitIncompatible, so a bug that invents a new Rank string +// fails loudly. +func WorstExitCode(divs []Divergence) int { + worst := ExitIdentical + for _, d := range divs { + code, ok := rankExitCode[d.Rank] + if !ok { + code = ExitIncompatible + } + if code > worst { + worst = code + } + } + return worst +} + +// FormatDivergences renders divs as a person-facing list of findings, one +// line per Divergence (plus an optional indented Note line), in whatever +// order divs was given - callers that want a stable order should sort +// first. It does not print anything when divs is empty; callers own the +// "no differences" message, since what that should say varies (a node +// pair's own report vs. a single mismatched table's error). +func FormatDivergences(divs []Divergence) string { + var b strings.Builder + for _, d := range divs { + valueOnA, valueOnB := displayValue(d.Property, d.ValueOnA), displayValue(d.Property, d.ValueOnB) + if d.Property != "" { + fmt.Fprintf(&b, " - %s.%s: on %s = %q, on %s = %q [%s]\n", + d.Object, d.Property, d.NodeA, valueOnA, d.NodeB, valueOnB, rankText(d)) + } else { + fmt.Fprintf(&b, " - %s (%s): on %s = %q, on %s = %q [%s]\n", + d.Object, d.Kind, d.NodeA, valueOnA, d.NodeB, valueOnB, rankText(d)) + } + if d.Note != "" { + fmt.Fprintf(&b, " (%s)\n", d.Note) + } + } + return strings.TrimRight(b.String(), "\n") +} + +// listProperties are the Property names whose ValueOnA/ValueOnB were built +// by joinList (see collect.go), so FormatDivergences must decode them back +// into their members before printing — %q on the packed form would print a +// key of columns (a, b) as "2:a2:b", not "a, b". Restricted to these known +// properties rather than decoding whatever value happens to parse: a +// property that owes its shape to something else (a CHECK definition, a +// default expression) is never run through this decoding, so it cannot be +// misread as a packed list just because it happens to contain a colon and +// a leading digit. +var listProperties = map[string]bool{ + "labels": true, + "key_columns": true, + "key_opclasses": true, +} + +// displayValue renders one compared value for a person, decoding it first +// when Property says it is one joinList packed. Comparison itself never +// goes through this decoding — it works on the packed form directly. +func displayValue(property, value string) string { + if !listProperties[property] { + return value + } + values, ok := splitList(value) + if !ok { + return value + } + return strings.Join(values, ", ") +} + +// MarshalJSON decodes ValueOnA/ValueOnB for the structured report the way +// FormatDivergences does for the text one, so one run cannot describe the +// same finding two ways. Only the rendering is decoded: the Divergence +// keeps the packed form, which is what tells a key of the columns (a, b) +// from a key of the one column named "a,b". +func (d Divergence) MarshalJSON() ([]byte, error) { + // A local type has no methods, so json marshals it as a plain struct + // rather than calling this again. + type divergenceJSON Divergence + out := divergenceJSON(d) + out.ValueOnA = displayValue(d.Property, d.ValueOnA) + out.ValueOnB = displayValue(d.Property, d.ValueOnB) + return json.Marshal(out) +} + +// rankText renders a Divergence's Rank, and for RankNarrowed names the node +// whose side is the narrow one (Divergence.NarrowSide) — the useful half of +// the finding, since "narrowed" alone does not say which node is about to +// reject the other's rows. +func rankText(d Divergence) string { + if d.Rank == RankNarrowed && d.NarrowSide != "" { + return fmt.Sprintf("%s, narrower on %s", d.Rank, d.NarrowSide) + } + return d.Rank +} diff --git a/internal/consistency/schema/report_test.go b/internal/consistency/schema/report_test.go new file mode 100644 index 00000000..ecbb975b --- /dev/null +++ b/internal/consistency/schema/report_test.go @@ -0,0 +1,259 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package schema + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestWorstExitCode_EmptyIsIdentical(t *testing.T) { + if code := WorstExitCode(nil); code != ExitIdentical { + t.Fatalf("expected ExitIdentical for no divergences, got %d", code) + } +} + +func TestWorstExitCode_PicksTheMostSevereRank(t *testing.T) { + divs := []Divergence{ + {Rank: RankCosmetic}, + {Rank: RankEquivalentDiffering}, + {Rank: RankNarrowed}, + } + if code := WorstExitCode(divs); code != ExitNarrowed { + t.Fatalf("expected ExitNarrowed (worst of the three), got %d", code) + } +} + +func TestWorstExitCode_AbsentAndIncompatibleShareTheWorstCode(t *testing.T) { + if code := WorstExitCode([]Divergence{{Rank: RankAbsent}}); code != ExitIncompatible { + t.Fatalf("expected RankAbsent to map to ExitIncompatible, got %d", code) + } + if code := WorstExitCode([]Divergence{{Rank: RankIncompatible}}); code != ExitIncompatible { + t.Fatalf("expected RankIncompatible to map to ExitIncompatible, got %d", code) + } +} + +func TestFormatDivergences_EmptyIsEmptyString(t *testing.T) { + if got := FormatDivergences(nil); got != "" { + t.Fatalf("expected empty string for no divergences, got %q", got) + } +} + +func TestFormatDivergences_IncludesObjectPropertyAndBothValues(t *testing.T) { + divs := []Divergence{ + { + Object: "public.orders.amount", Kind: "column", Property: "type", + NodeA: "n1", NodeB: "n2", ValueOnA: "int4", ValueOnB: "int8", + Rank: RankNarrowed, NarrowSide: "n1", + }, + } + got := FormatDivergences(divs) + for _, want := range []string{"public.orders.amount", "type", "n1", "n2", "int4", "int8", RankNarrowed} { + if !strings.Contains(got, want) { + t.Errorf("expected formatted output to contain %q, got: %s", want, got) + } + } +} + +func TestFormatDivergences_AppendsNoteWhenPresent(t *testing.T) { + divs := []Divergence{ + { + Object: "public.orders", Kind: "key", Property: "replica_identity", + NodeA: "n1", NodeB: "n2", ValueOnA: "d", ValueOnB: "f", + Rank: RankIncompatible, Note: "explains itself", + }, + } + got := FormatDivergences(divs) + if !strings.Contains(got, "explains itself") { + t.Errorf("expected Note to appear in output, got: %s", got) + } +} + +// TestReportRendersListSeparatorReadably checks that a value joined for +// comparison with a unit separator is not printed with it: a key of two +// columns must read as "a, b", not "a\x1fb". +func TestReportRendersListSeparatorReadably(t *testing.T) { + out := FormatDivergences([]Divergence{{ + Object: "public.t", Kind: "key", Property: "key_columns", + NodeA: "n1", NodeB: "n2", + ValueOnA: joinList([]string{"a", "b"}), + ValueOnB: joinList([]string{"b", "a"}), + Rank: RankIncompatible, + }}) + + if strings.Contains(out, "2:a2:b") || strings.Contains(out, "2:b2:a") { + t.Fatalf("the packed list form reached the report: %s", out) + } + if !strings.Contains(out, `"a, b"`) || !strings.Contains(out, `"b, a"`) { + t.Fatalf("want readable column lists, got: %s", out) + } +} + +// TestReportLeavesNonListPropertiesAlone checks that displayValue only +// decodes the properties joinList actually built - a value that merely +// looks like a packed list (starts with digits and a colon) must not be +// misread as one just because its own Property is not among those. +func TestReportLeavesNonListPropertiesAlone(t *testing.T) { + out := FormatDivergences([]Divergence{{ + Object: "public.t", Kind: "domain", Property: "check", + NodeA: "n1", NodeB: "n2", + ValueOnA: "CHECK (5:00:00 < start_time)", ValueOnB: "(absent)", + Rank: RankNarrowed, + }}) + + if !strings.Contains(out, "CHECK (5:00:00 < start_time)") { + t.Fatalf("want the CHECK text printed verbatim, got: %s", out) + } +} + +// TestDivergenceJSON_DecodesPackedListValues: the structured report must +// not leak the packed form - the same enum drift once read "sad, ok, happy" +// in text and "3:sad2:ok5:happy" in JSON. +func TestDivergenceJSON_DecodesPackedListValues(t *testing.T) { + d := Divergence{ + Object: "public.mood", Kind: "enum", Property: "labels", + NodeA: "n1", NodeB: "n2", + ValueOnA: joinList([]string{"sad", "ok", "happy"}), + ValueOnB: joinList([]string{"sad", "happy"}), + Rank: RankIncompatible, + } + + encoded, err := json.Marshal(d) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + var decoded map[string]any + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if got := decoded["value_on_a"]; got != "sad, ok, happy" { + t.Errorf("value_on_a = %q, want the decoded members", got) + } + if got := decoded["value_on_b"]; got != "sad, happy" { + t.Errorf("value_on_b = %q, want the decoded members", got) + } + if strings.Contains(string(encoded), "3:sad") { + t.Errorf("the packed form escaped into the report: %s", encoded) + } +} + +// TestDivergenceJSON_KeyColumnsAreDecoded covers the other packed +// properties, the ones a primary-key mismatch reports. +func TestDivergenceJSON_KeyColumnsAreDecoded(t *testing.T) { + for _, property := range []string{"key_columns", "key_opclasses"} { + d := Divergence{ + Object: "public.orders", Kind: "key", Property: property, + NodeA: "n1", NodeB: "n2", + ValueOnA: joinList([]string{"id"}), + ValueOnB: joinList([]string{"id", "tenant"}), + Rank: RankIncompatible, + } + encoded, err := json.Marshal(d) + if err != nil { + t.Fatalf("%s: marshal: %v", property, err) + } + var decoded map[string]any + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatalf("%s: unmarshal: %v", property, err) + } + if got := decoded["value_on_b"]; got != "id, tenant" { + t.Errorf("%s: value_on_b = %q, want %q", property, got, "id, tenant") + } + } +} + +// TestDivergenceJSON_LeavesNonListPropertiesAlone: a default expression is +// not a packed list, even when it starts with a digit and a colon. +func TestDivergenceJSON_LeavesNonListPropertiesAlone(t *testing.T) { + d := Divergence{ + Object: "public.orders.note", Kind: "column", Property: "default", + NodeA: "n1", NodeB: "n2", + ValueOnA: "2:00", ValueOnB: "'x'::text", + Rank: RankEquivalentDiffering, + } + + encoded, err := json.Marshal(d) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var decoded map[string]any + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if got := decoded["value_on_a"]; got != "2:00" { + t.Errorf("value_on_a = %q, want it left alone", got) + } + if got := decoded["value_on_b"]; got != "'x'::text" { + t.Errorf("value_on_b = %q, want it left alone", got) + } +} + +// TestDivergenceJSON_AgreesWithTheTextReport is the invariant the others +// are instances of: one run, one description per finding. +func TestDivergenceJSON_AgreesWithTheTextReport(t *testing.T) { + d := Divergence{ + Object: "public.orders", Kind: "key", Property: "key_columns", + NodeA: "n1", NodeB: "n2", + ValueOnA: joinList([]string{"id"}), + ValueOnB: joinList([]string{"id", "tenant"}), + Rank: RankIncompatible, + } + + encoded, err := json.Marshal(d) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var decoded map[string]any + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + text := FormatDivergences([]Divergence{d}) + for _, field := range []string{"value_on_a", "value_on_b"} { + value, _ := decoded[field].(string) + if !strings.Contains(text, value) { + t.Errorf("json %s = %q does not appear in the text report:\n%s", field, value, text) + } + } +} + +// TestDivergenceJSON_KeepsTheOtherFields checks the marshaller did not drop +// anything while swapping the two values out. +func TestDivergenceJSON_KeepsTheOtherFields(t *testing.T) { + d := Divergence{ + Object: "public.orders.qty", Kind: "column", Property: "type", + NodeA: "n1", NodeB: "n2", ValueOnA: "integer", ValueOnB: "bigint", + Rank: RankNarrowed, NarrowSide: "n1", Note: "a note", + } + + encoded, err := json.Marshal(d) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var decoded map[string]any + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + want := map[string]string{ + "object": "public.orders.qty", "kind": "column", "property": "type", + "node_a": "n1", "node_b": "n2", "value_on_a": "integer", "value_on_b": "bigint", + "rank": RankNarrowed, "narrow_side": "n1", "note": "a note", + } + for field, expected := range want { + if got := decoded[field]; got != expected { + t.Errorf("%s = %v, want %q", field, got, expected) + } + } +} diff --git a/internal/consistency/scope/schema.go b/internal/consistency/scope/schema.go new file mode 100644 index 00000000..d629c9b5 --- /dev/null +++ b/internal/consistency/scope/schema.go @@ -0,0 +1,56 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package scope + +import ( + "context" + "fmt" + + "github.com/pgedge/ace/db/queries" +) + +// SchemaProvider resolves every base table of one PostgreSQL namespace. +// Resolve delegates to queries.GetTablesInSchema, the same query +// schema-diff's data comparison already uses, so both share one definition +// of what a schema's area means. +type SchemaProvider struct { + SchemaName string +} + +var _ Provider = SchemaProvider{} + +// Resolve reads the base tables of SchemaProvider.SchemaName from the node +// behind db. An unknown schema resolves to an empty Scope; existence is +// checked by the caller. +func (p SchemaProvider) Resolve(ctx context.Context, db queries.DBQuerier) (Scope, error) { + tables, err := queries.GetTablesInSchema(ctx, db, p.SchemaName) + if err != nil { + return Scope{}, fmt.Errorf("could not list tables in schema %q: %w", p.SchemaName, err) + } + + scope := Scope{ + Tables: make([]QualifiedName, 0, len(tables)), + Source: p.Describe(), + } + for _, table := range tables { + scope.Tables = append(scope.Tables, QualifiedName{Schema: p.SchemaName, Table: table}) + } + return scope, nil +} + +// Describe labels this source for a report header, and for the Source field +// Resolve records on the Scope it returns. It names the namespace rather +// than the tables found in it, so the label is the same on every node even +// when the nodes disagree about what the namespace holds. +func (p SchemaProvider) Describe() string { + return fmt.Sprintf("schema %s", p.SchemaName) +} diff --git a/internal/consistency/scope/scopedefs.go b/internal/consistency/scope/scopedefs.go new file mode 100644 index 00000000..34eeafef --- /dev/null +++ b/internal/consistency/scope/scopedefs.go @@ -0,0 +1,79 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +// Package scope resolves which tables a comparison run covers. schema-diff, +// table-diff and repset-diff each arrive at a table list differently (a +// namespace, an explicit name, a Spock replication set, a native +// publication); this package gives each a common interface, so the +// comparison layer (internal/consistency/schema) never needs to know which +// one produced the list, and never imports anything Spock-specific. +// +// Only the "schema" source is implemented so far: all base tables of one +// namespace, matching schema-diff's existing behaviour. Adding another +// source is meant to be one new file here plus a flag in the owning command. +package scope + +import ( + "context" + + "github.com/pgedge/ace/db/queries" +) + +// QualifiedName identifies one table by its namespace and name. It does not +// carry a node: the same QualifiedName is used to look the table up on every +// node being compared. +type QualifiedName struct { + Schema string + Table string +} + +// String renders the qualified name the way it is shown to a person: +// "schema.table". +func (q QualifiedName) String() string { + return q.Schema + "." + q.Table +} + +// Scope is the list of tables a comparison run should cover, as resolved on +// one node. Columns is only populated when the source restricts which +// columns are in scope (a Spock replication set with a column list, a +// publication with attnames); a missing entry means "all columns of that +// table are in scope". +type Scope struct { + // Tables lists every table this source resolved on the node it was + // asked to read. Order is not significant; callers that need a stable + // order (for printing, for hashing) must sort it themselves. + Tables []QualifiedName + + // Columns restricts the columns considered in scope for a table, when + // the source knows such a restriction. A table with no entry here is + // read in full. + Columns map[QualifiedName][]string + + // Source names which Provider produced this Scope, for the report + // header. Not interpreted by the comparison layer. + Source string +} + +// Provider resolves a Scope by reading one node. Each source is one +// implementation of this interface in its own file. DBQuerier is +// db/queries.DBQuerier, so a Provider can be handed a pool, a transaction, +// or anything else that already satisfies it, with no adapter needed. +type Provider interface { + // Resolve reads the node behind db and returns the tables (and, where + // applicable, columns) within this source's area. It reads only the + // node it is given; reconciling disagreement between nodes is the + // caller's job. + Resolve(ctx context.Context, db queries.DBQuerier) (Scope, error) + + // Describe returns a short human-readable label for the report header, + // e.g. "schema public" or "repset \"default\"". + Describe() string +} diff --git a/internal/consistency/topology/spock.go b/internal/consistency/topology/spock.go new file mode 100644 index 00000000..5a615e41 --- /dev/null +++ b/internal/consistency/topology/spock.go @@ -0,0 +1,103 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package topology + +import ( + "context" + "fmt" + + "github.com/jackc/pgx/v5/pgxpool" + "github.com/pgedge/ace/db/queries" + utils "github.com/pgedge/ace/pkg/common" + "github.com/pgedge/ace/pkg/types" +) + +// FetchSpockNodeConfig reads one node's Spock subscriptions and replication +// set membership, and surfaces hints for common misconfigurations: a +// subscription whose origin node could not be resolved, a subscription with +// no replication sets attached, or replication sets that exist but contain +// no tables (usually because those tables have no primary key). +// +// This is the single place that reads Spock's topology catalogs, so anything +// needing "who replicates from whom" can call it instead of re-deriving it. +func FetchSpockNodeConfig(ctx context.Context, pool *pgxpool.Pool, nodeName string) (NodeConfig, error) { + config := NodeConfig{NodeName: nodeName, Hints: []string{}} + + nodeInfos, err := queries.GetSpockNodeAndSubInfo(ctx, pool) + if err != nil { + return config, fmt.Errorf("querying spock.node and spock.subscription on node %s failed: %w", nodeName, err) + } + + if len(nodeInfos) > 0 { + config.NodeName = nodeInfos[0].NodeName + for _, ni := range nodeInfos { + sub := types.SpockSubscription{} + if ni.SubName != "" { + sub.SubName = ni.SubName + sub.ProviderNode = ni.SubOriginName + sub.SubEnabled = ni.SubEnabled + sub.ReplicationSets = ni.SubReplicationSets + if ni.SubOriginName == "" { + hint := fmt.Sprintf("Subscription '%s' has an unresolved origin node; its reciprocal peer cannot be determined and it may be reported below as a missing subscription.", sub.SubName) + if !utils.Contains(config.Hints, hint) { + config.Hints = append(config.Hints, hint) + } + } + if len(ni.SubReplicationSets) == 0 { + hint := fmt.Sprintf("Subscription '%s' has no replication sets.", sub.SubName) + if !utils.Contains(config.Hints, hint) { + config.Hints = append(config.Hints, hint) + } + } + config.Subscriptions = append(config.Subscriptions, sub) + } + // A row with an empty SubName is a node that has no + // subscription at all (the query's left join produced one row + // of nulls rather than none). Appending sub here would add an + // empty types.SpockSubscription{} to the list for every such + // node, which is not a subscription to report or match against. + } + } else { + config.Hints = append(config.Hints, "Hint: No subscriptions have been created on this node.") + } + + repRows, err := queries.GetSpockRepSetInfo(ctx, pool) + if err != nil { + return config, fmt.Errorf("querying spock.tables on node %s failed: %w", nodeName, err) + } + config.RepSetInfo = repRows + + var tablesInRepSets []string + for _, rs := range repRows { + if rs.SetName != "" { + tablesInRepSets = append(tablesInRepSets, rs.RelName...) + } + } + if len(repRows) > 0 && len(tablesInRepSets) == 0 { + config.Hints = append(config.Hints, "Hint: Tables not in replication set might not have primary keys, or you need to run repset-add-table.") + } + + return config, nil +} + +// SubscriptionsByProvider indexes subscriptions by the node they replicate +// from. Matching reciprocal subscriptions must go by provider-node identity, +// not by subscription name, because users are free to rename subscriptions. +func SubscriptionsByProvider(subs []types.SpockSubscription) map[string]types.SpockSubscription { + byProvider := make(map[string]types.SpockSubscription, len(subs)) + for _, s := range subs { + if s.ProviderNode != "" { + byProvider[s.ProviderNode] = s + } + } + return byProvider +} diff --git a/internal/consistency/topology/topologydefs.go b/internal/consistency/topology/topologydefs.go new file mode 100644 index 00000000..7d531001 --- /dev/null +++ b/internal/consistency/topology/topologydefs.go @@ -0,0 +1,30 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +// Package topology answers one question: which node replicates from which, +// over which subscription, carrying which replication sets. Keeping this +// separate from table structure lets structure comparison run without a +// working replication topology, and lets topology be reused by anything +// that needs to know "who talks to whom". +package topology + +import "github.com/pgedge/ace/pkg/types" + +// NodeConfig aggregates one node's replication topology: its subscriptions, +// the replication sets it knows about, and hints about incomplete +// configuration (unresolved origins, empty replication sets, tables outside +// any replication set). +type NodeConfig struct { + NodeName string `json:"node_name"` + Subscriptions []types.SpockSubscription `json:"subscriptions"` + RepSetInfo []types.SpockRepSetInfo `json:"rep_set_info"` + Hints []string `json:"hints"` +} diff --git a/pkg/common/exitcode.go b/pkg/common/exitcode.go new file mode 100644 index 00000000..7f217ea4 --- /dev/null +++ b/pkg/common/exitcode.go @@ -0,0 +1,42 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package common + +// ExitCodeError wraps an error with a specific process exit code, letting a +// caller distinguish how severe a failure was without parsing stdout. +// +// This lives in pkg/common because the value is most naturally constructed +// where the divergence is found (inside internal/consistency/diff), and +// internal/cli already depends on that package. +type ExitCodeError struct { + Code int + Err error +} + +// Error returns the wrapped error's message, so that carrying an exit code +// changes nothing about how the failure reads. A value built without a +// wrapped error still has to say something, since a caller is free to print +// it before it looks at the code. +func (e *ExitCodeError) Error() string { + if e.Err == nil { + return "exit code error" + } + return e.Err.Error() +} + +// Unwrap returns the wrapped error, so errors.Is and errors.As see past the +// exit code to the failure underneath it. +func (e *ExitCodeError) Unwrap() error { return e.Err } + +// ExitCode is the method main.go looks for (via errors.As) to pick a +// process exit code other than the generic 1 a plain error gets. +func (e *ExitCodeError) ExitCode() int { return e.Code } diff --git a/tests/integration/schema_diff_structure_test.go b/tests/integration/schema_diff_structure_test.go new file mode 100644 index 00000000..622529da --- /dev/null +++ b/tests/integration/schema_diff_structure_test.go @@ -0,0 +1,592 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package integration + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "strings" + "testing" + + "github.com/jackc/pgx/v5/pgxpool" + "github.com/pgedge/ace/internal/consistency/diff" + "github.com/pgedge/ace/internal/consistency/schema" + utils "github.com/pgedge/ace/pkg/common" + "github.com/stretchr/testify/require" +) + +// These tests cover schema-diff --compare=structure against a real two-node +// cluster, going through real catalogs on two independently initialised +// nodes so the query behind CollectSnapshot actually executes. +// +// The schema uses a domain, an enum and a composite type: PostgreSQL assigns +// each a different OID per node, so structural comparison must key on their +// names and definitions. + +const structureSchema = "ace_structure_test" + +const structureTable = "t" + +// setupStructureSchema creates structureSchema with identical DDL on both +// nodes, but shifts node2's OID counter first so no user-defined type ends +// up with the same OID on both nodes. +func setupStructureSchema(t *testing.T) { + t.Helper() + ctx := context.Background() + + pools := []*pgxpool.Pool{pgCluster.Node1Pool, pgCluster.Node2Pool} + for _, pool := range pools { + _, err := pool.Exec(ctx, fmt.Sprintf(`DROP SCHEMA IF EXISTS %s CASCADE`, structureSchema)) + require.NoError(t, err) + _, err = pool.Exec(ctx, fmt.Sprintf(`CREATE SCHEMA %s`, structureSchema)) + require.NoError(t, err) + } + + // Burn OIDs on node2 only. + _, err := pgCluster.Node2Pool.Exec(ctx, fmt.Sprintf(` + DO $$ + BEGIN + FOR i IN 1..40 LOOP + EXECUTE format('CREATE DOMAIN %s.burn%%s AS int', i); + EXECUTE format('DROP DOMAIN %s.burn%%s', i); + END LOOP; + END $$`, structureSchema, structureSchema)) + require.NoError(t, err) + + ddl := []string{ + fmt.Sprintf(`CREATE DOMAIN %s.short AS varchar(20)`, structureSchema), + fmt.Sprintf(`CREATE TYPE %s.mood AS ENUM ('sad', 'ok', 'happy')`, structureSchema), + fmt.Sprintf(`CREATE TYPE %s.addr AS (city text, zip int)`, structureSchema), + fmt.Sprintf(`CREATE TABLE %s.%s ( + id int PRIMARY KEY, + code %s.short, + m %s.mood, + a %s.addr, + n int + )`, structureSchema, structureTable, structureSchema, structureSchema, structureSchema), + } + for _, pool := range pools { + for _, stmt := range ddl { + _, err := pool.Exec(ctx, stmt) + require.NoError(t, err, "ddl: %s", stmt) + } + } + + t.Cleanup(func() { + for _, pool := range pools { + pool.Exec(ctx, fmt.Sprintf(`DROP SCHEMA IF EXISTS %s CASCADE`, structureSchema)) //nolint:errcheck // best-effort cleanup + } + }) + + requireUserTypeOIDsDiffer(t) +} + +// requireUserTypeOIDsDiffer pins the premise of these tests: the two nodes +// really do disagree about the OIDs of identically-defined types. Without +// this the "identical structure reports nothing" test below could pass for +// the wrong reason. +func requireUserTypeOIDsDiffer(t *testing.T) { + t.Helper() + ctx := context.Background() + const q = `SELECT t.oid::int FROM pg_type t + JOIN pg_namespace n ON n.oid = t.typnamespace + WHERE n.nspname = $1 AND t.typname = 'short'` + + var oid1, oid2 int + require.NoError(t, pgCluster.Node1Pool.QueryRow(ctx, q, structureSchema).Scan(&oid1)) + require.NoError(t, pgCluster.Node2Pool.QueryRow(ctx, q, structureSchema).Scan(&oid2)) + require.NotEqual(t, oid1, oid2, + "this test needs the two nodes to assign different OIDs to the same domain") +} + +func structureTaskForTest() *diff.SchemaDiffCmd { + task := newTestSchemaDiffTask(structureSchema, fmt.Sprintf("%s,%s", serviceN1, serviceN2)) + task.Compare = diff.CompareStructure + return task +} + +// compareStructureForTest collects both nodes' snapshots and compares them, +// the same way schemaStructureDiff does, so a test can assert on individual +// Divergences. +func compareStructureForTest(t *testing.T) []schema.Divergence { + t.Helper() + ctx := context.Background() + tables := []string{structureTable} + + snapA, err := schema.CollectSnapshot(ctx, pgCluster.Node1Pool, serviceN1, structureSchema, tables) + require.NoError(t, err) + snapB, err := schema.CollectSnapshot(ctx, pgCluster.Node2Pool, serviceN2, structureSchema, tables) + require.NoError(t, err) + + return schema.Compare(structureSchema, tables, snapA, snapB) +} + +func findDivergenceFor(t *testing.T, divs []schema.Divergence, object, property string) schema.Divergence { + t.Helper() + for _, d := range divs { + if d.Object == object && d.Property == property { + return d + } + } + t.Fatalf("no divergence for object %q property %q in %+v", object, property, divs) + return schema.Divergence{} +} + +// TestSchemaDiffStructure_IdenticalSchemaReportsNothing is the false-positive +// guard: the same DDL on both nodes, with every user-defined type carrying a +// different OID on each. Anything reported here is noise, and noise in this +// mode is expensive — its whole purpose is a severity-coded exit status a +// script can act on. +func TestSchemaDiffStructure_IdenticalSchemaReportsNothing(t *testing.T) { + setupStructureSchema(t) + + divs := compareStructureForTest(t) + require.Empty(t, divs, "identical structure must produce no divergences") + + require.NoError(t, structureTaskForTest().SchemaTableDiff(), + "identical structure must not produce an exit-code error") +} + +// TestSchemaDiffStructure_ColumnAndTypeDriftIsReported drifts one column, one +// enum and one composite attribute, and checks each is found with the +// expected rank, including which side of a narrowing is the narrow one. +func TestSchemaDiffStructure_ColumnAndTypeDriftIsReported(t *testing.T) { + setupStructureSchema(t) + ctx := context.Background() + + for _, stmt := range []string{ + // int4 -> int8: node1 is now the narrow side. + fmt.Sprintf(`ALTER TABLE %s.%s ALTER COLUMN n TYPE bigint`, structureSchema, structureTable), + // A column node1 does not have at all. + fmt.Sprintf(`ALTER TABLE %s.%s ADD COLUMN extra text`, structureSchema, structureTable), + // A value node1's enum cannot hold. The type's OID does not change, + // so nothing but its definition can reveal this. + fmt.Sprintf(`ALTER TYPE %s.mood ADD VALUE 'furious'`, structureSchema), + } { + _, err := pgCluster.Node2Pool.Exec(ctx, stmt) + require.NoError(t, err, "drift: %s", stmt) + } + + // Drift a composite type's attribute too. ALTER TYPE ... ALTER ATTRIBUTE + // refuses to run at all - CASCADE included - while any plain table + // column uses the type directly (CASCADE there only reaches typed + // tables, ones created with CREATE TABLE ... OF the type, which this + // one is not). So the column is dropped first and re-added after the + // type is rebuilt; column order is not part of what this mode compares. + for _, stmt := range []string{ + fmt.Sprintf(`ALTER TABLE %s.%s DROP COLUMN a`, structureSchema, structureTable), + fmt.Sprintf(`DROP TYPE %s.addr`, structureSchema), + fmt.Sprintf(`CREATE TYPE %s.addr AS (city text, zip bigint)`, structureSchema), + fmt.Sprintf(`ALTER TABLE %s.%s ADD COLUMN a %s.addr`, structureSchema, structureTable, structureSchema), + } { + _, err := pgCluster.Node2Pool.Exec(ctx, stmt) + require.NoError(t, err, "drift: %s", stmt) + } + + divs := compareStructureForTest(t) + + qualified := structureSchema + "." + structureTable + + colType := findDivergenceFor(t, divs, qualified+".n", "type") + require.Equal(t, schema.RankNarrowed, colType.Rank) + require.Equal(t, serviceN1, colType.NarrowSide, "int4 is the narrow side") + + extra := findDivergenceFor(t, divs, qualified+".extra", "") + require.Equal(t, schema.RankAbsent, extra.Rank) + + mood := findDivergenceFor(t, divs, structureSchema+".mood", "labels") + require.Equal(t, schema.RankIncompatible, mood.Rank) + require.Contains(t, mood.ValueOnB, "furious") + require.NotContains(t, mood.ValueOnA, "furious") + + // The composite attribute is named for a reader, with its value shown + // as PostgreSQL prints the type. int4 -> int8 is a narrowing here too, + // the same as it is for a table column (see colType above). + addr := findDivergenceFor(t, divs, structureSchema+".addr", "attribute 2 (zip)") + require.Equal(t, schema.RankNarrowed, addr.Rank) + require.Equal(t, serviceN1, addr.NarrowSide, "int4 is the narrow side") + require.Equal(t, "integer", addr.ValueOnA) + require.Equal(t, "bigint", addr.ValueOnB) + + // The exit code the mode exists for. + err := structureTaskForTest().SchemaTableDiff() + var exitErr *utils.ExitCodeError + require.True(t, errors.As(err, &exitErr), "expected an ExitCodeError, got %v", err) + require.Equal(t, schema.ExitIncompatible, exitErr.Code) +} + +// TestSchemaDiffStructure_DomainDefinitionDrift covers two domain cases: a +// narrowed base type, and CHECK sets where each side has a constraint the +// other lacks, which must be reported as one incompatible finding. +func TestSchemaDiffStructure_DomainDefinitionDrift(t *testing.T) { + setupStructureSchema(t) + ctx := context.Background() + + _, err := pgCluster.Node1Pool.Exec(ctx, + fmt.Sprintf(`ALTER DOMAIN %s.short ADD CONSTRAINT short_min CHECK (length(VALUE) > 2)`, structureSchema)) + require.NoError(t, err) + _, err = pgCluster.Node2Pool.Exec(ctx, + fmt.Sprintf(`ALTER DOMAIN %s.short ADD CONSTRAINT short_max CHECK (length(VALUE) < 10)`, structureSchema)) + require.NoError(t, err) + + divs := compareStructureForTest(t) + + var checks []schema.Divergence + for _, d := range divs { + if d.Object == structureSchema+".short" && d.Property == "check" { + checks = append(checks, d) + } + } + require.Len(t, checks, 1, + "mutually exclusive CHECK sets must be one finding, not one per side: %+v", checks) + require.Equal(t, schema.RankIncompatible, checks[0].Rank) + // Checked against the number only, not the full "length(VALUE) > 2" + // text: PostgreSQL may print VALUE with an explicit ::text cast here + // depending on version, and that cast is not what this test is about. + require.Contains(t, checks[0].ValueOnA, "> 2") + require.Contains(t, checks[0].ValueOnB, "< 10") +} + +// TestSchemaDiffStructure_ReportOrderIsStable checks that two comparisons of +// the same unchanged pair of nodes return findings in the same order, since +// type-level findings are gathered from a map. +func TestSchemaDiffStructure_ReportOrderIsStable(t *testing.T) { + setupStructureSchema(t) + ctx := context.Background() + + for _, stmt := range []string{ + fmt.Sprintf(`CREATE DOMAIN %s.d1 AS int CHECK (VALUE > 1)`, structureSchema), + fmt.Sprintf(`CREATE DOMAIN %s.d2 AS int CHECK (VALUE > 2)`, structureSchema), + fmt.Sprintf(`CREATE DOMAIN %s.d3 AS int CHECK (VALUE > 3)`, structureSchema), + } { + _, err := pgCluster.Node1Pool.Exec(ctx, stmt) + require.NoError(t, err) + _, err = pgCluster.Node2Pool.Exec(ctx, replaceCheck(stmt)) + require.NoError(t, err) + } + for i, col := range []string{"c1", "c2", "c3"} { + stmt := fmt.Sprintf(`ALTER TABLE %s.%s ADD COLUMN %s %s.d%d`, + structureSchema, structureTable, col, structureSchema, i+1) + _, err := pgCluster.Node1Pool.Exec(ctx, stmt) + require.NoError(t, err) + _, err = pgCluster.Node2Pool.Exec(ctx, stmt) + require.NoError(t, err) + } + + first := compareStructureForTest(t) + require.NotEmpty(t, first, "the three domains differ, so there is something to order") + + for i := 0; i < 5; i++ { + again := compareStructureForTest(t) + require.Equal(t, first, again, "the same comparison produced a different report") + } +} + +// TestSchemaDiffStructure_SkipTablesExcludesTable checks that --skip-tables +// (SkipTables) excludes a table from --compare=structure the same way it +// already does from the default per-table data diff: a table named there +// must not contribute to the report or the exit code, even though it still +// exists and still genuinely differs. +func TestSchemaDiffStructure_SkipTablesExcludesTable(t *testing.T) { + setupStructureSchema(t) + ctx := context.Background() + + // A real divergence: node2 gets an extra column. Left unskipped, this + // must produce an ExitCodeError - this is the control the skipped case + // below is contrasted against. + _, err := pgCluster.Node2Pool.Exec(ctx, + fmt.Sprintf(`ALTER TABLE %s.%s ADD COLUMN extra text`, structureSchema, structureTable)) + require.NoError(t, err) + + t.Run("NotSkippedStillReportsTheDivergence", func(t *testing.T) { + err := structureTaskForTest().SchemaTableDiff() + var exitErr *utils.ExitCodeError + require.True(t, errors.As(err, &exitErr), "expected an ExitCodeError, got %v", err) + require.Equal(t, schema.ExitIncompatible, exitErr.Code) + }) + + t.Run("SkippedTableProducesNoDivergence", func(t *testing.T) { + task := structureTaskForTest() + task.SkipTables = structureTable + require.NoError(t, task.SchemaTableDiff(), + "the only table in scope was named by --skip-tables, so nothing was left to compare") + }) +} + +// TestSchemaDiffStructure_SkipTablesExcludesMissingTableFromReport checks +// that --skip-tables also keeps a table missing on some nodes out of the +// "tables missing on some nodes" section and out of the exit code, not only +// out of the per-table structural comparison (that part is covered by +// TestSchemaDiffStructure_SkipTablesExcludesTable above). Without this, a +// table the user explicitly excluded would still be reported as missing and +// would still force schema.ExitIncompatible. +func TestSchemaDiffStructure_SkipTablesExcludesMissingTableFromReport(t *testing.T) { + setupStructureSchema(t) + ctx := context.Background() + + const onlyOnNode1 = "only_on_node1" + _, err := pgCluster.Node1Pool.Exec(ctx, + fmt.Sprintf(`CREATE TABLE %s.%s (id int PRIMARY KEY)`, structureSchema, onlyOnNode1)) + require.NoError(t, err) + + t.Run("NotSkippedStillReportsMissingTable", func(t *testing.T) { + err := structureTaskForTest().SchemaTableDiff() + var exitErr *utils.ExitCodeError + require.True(t, errors.As(err, &exitErr), "expected an ExitCodeError, got %v", err) + require.Equal(t, schema.ExitIncompatible, exitErr.Code) + }) + + t.Run("SkippedMissingTableProducesNoFailure", func(t *testing.T) { + task := structureTaskForTest() + task.SkipTables = onlyOnNode1 + require.NoError(t, task.SchemaTableDiff(), + "the only asymmetric table was named by --skip-tables, so it must not be reported as missing or force a failure") + }) +} + +// captureStdout redirects os.Stdout for the duration of fn and returns +// everything written to it. schemaStructureDiff prints straight to +// os.Stdout rather than returning the report, so this is the only way to +// check what --output=json actually produced. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + r, w, err := os.Pipe() + require.NoError(t, err) + orig := os.Stdout + os.Stdout = w + + fn() + + require.NoError(t, w.Close()) + os.Stdout = orig + out, err := io.ReadAll(r) + require.NoError(t, err) + return string(out) +} + +// TestSchemaDiffStructure_JSONOutput checks --output=json end-to-end: it +// must be valid JSON carrying the same divergence the text report shows, +// with the same exit code, and it must only replace the text report when +// --output was actually given (OutputExplicit) - see SchemaDiffCmd's doc +// comment on that field for why "json" being the flag's own default is not +// enough on its own. +func TestSchemaDiffStructure_JSONOutput(t *testing.T) { + setupStructureSchema(t) + ctx := context.Background() + + _, err := pgCluster.Node2Pool.Exec(ctx, + fmt.Sprintf(`ALTER TABLE %s.%s ADD COLUMN extra text`, structureSchema, structureTable)) + require.NoError(t, err) + + task := structureTaskForTest() + task.Output = "json" + task.OutputExplicit = true + + var runErr error + stdout := captureStdout(t, func() { + runErr = task.SchemaTableDiff() + }) + + var exitErr *utils.ExitCodeError + require.True(t, errors.As(runErr, &exitErr), "expected an ExitCodeError, got %v", runErr) + require.Equal(t, schema.ExitIncompatible, exitErr.Code) + + var report diff.StructureDiffReport + require.NoError(t, json.Unmarshal([]byte(stdout), &report), "stdout must be valid JSON: %s", stdout) + require.Equal(t, structureSchema, report.Schema) + require.Equal(t, exitErr.Code, report.ExitCode) + require.Len(t, report.Comparisons, 1) + + found := false + for _, d := range report.Comparisons[0].Divergences { + if d.Object == structureSchema+"."+structureTable+".extra" { + found = true + require.Equal(t, schema.RankAbsent, d.Rank) + } + } + require.True(t, found, "expected the added column to appear in the JSON report: %+v", report) +} + +// TestSchemaDiffStructure_TextOutputWithoutExplicitFlag checks the other +// half of the same guard: a run that never passes --output must keep +// printing the prose report, even though "json" is the flag's own default +// value once it reaches SchemaDiffCmd.Output. +func TestSchemaDiffStructure_TextOutputWithoutExplicitFlag(t *testing.T) { + setupStructureSchema(t) + + task := structureTaskForTest() + // task.Output and task.OutputExplicit both left at their zero values, + // the same state a task never touched by the --output flag is in. + + stdout := captureStdout(t, func() { + _ = task.SchemaTableDiff() + }) + require.True(t, strings.HasPrefix(strings.TrimSpace(stdout), "==="), + "default output must stay the prose report, not JSON: %s", stdout) +} + +// replaceCheck strips the CHECK clause, so node2 gets the same domains +// without the constraint node1's carry. +func replaceCheck(stmt string) string { + if idx := indexOfCheck(stmt); idx >= 0 { + return stmt[:idx] + } + return stmt +} + +func indexOfCheck(stmt string) int { + const needle = " CHECK (" + for i := 0; i+len(needle) <= len(stmt); i++ { + if stmt[i:i+len(needle)] == needle { + return i + } + } + return -1 +} + +// requireAttoptionsOrderDiffers pins the premise of the test below: the two +// nodes really do hold the same options in a different order. Without it +// the test could pass because PostgreSQL normalised the order itself. +func requireAttoptionsOrderDiffers(t *testing.T, column string) { + t.Helper() + ctx := context.Background() + const q = `SELECT COALESCE(a.attoptions::text, '') + FROM pg_attribute a + JOIN pg_class c ON c.oid = a.attrelid + JOIN pg_namespace ns ON ns.oid = c.relnamespace + WHERE ns.nspname = $1 AND c.relname = $2 AND a.attname = $3` + + var raw1, raw2 string + require.NoError(t, pgCluster.Node1Pool.QueryRow(ctx, q, structureSchema, structureTable, column).Scan(&raw1)) + require.NoError(t, pgCluster.Node2Pool.QueryRow(ctx, q, structureSchema, structureTable, column).Scan(&raw2)) + require.NotEqual(t, raw1, raw2, + "this test needs the two nodes to hold the same attoptions in a different order") +} + +// TestSchemaDiffStructure_ColumnOptionOrderIsNotADifference: two nodes that +// set the same two options in opposite orders hold different attoptions +// strings for an identical column. Sorting in the query is what keeps that +// from reading as drift. +func TestSchemaDiffStructure_ColumnOptionOrderIsNotADifference(t *testing.T) { + setupStructureSchema(t) + ctx := context.Background() + + node1First := []string{ + fmt.Sprintf(`ALTER TABLE %s.%s ALTER COLUMN n SET (n_distinct = -0.5)`, structureSchema, structureTable), + fmt.Sprintf(`ALTER TABLE %s.%s ALTER COLUMN n SET (n_distinct_inherited = 100)`, structureSchema, structureTable), + } + for _, stmt := range node1First { + _, err := pgCluster.Node1Pool.Exec(ctx, stmt) + require.NoError(t, err) + } + for i := len(node1First) - 1; i >= 0; i-- { + _, err := pgCluster.Node2Pool.Exec(ctx, node1First[i]) + require.NoError(t, err) + } + + requireAttoptionsOrderDiffers(t, "n") + + divs := compareStructureForTest(t) + require.Empty(t, divs, + "the same column options in a different order is not a difference: %+v", divs) +} + +// TestSchemaDiffStructure_ColumnOptionValueDriftIsStillReported is the +// guard in the other direction: sorting attoptions must not hide two nodes +// that genuinely disagree about an option's value. +func TestSchemaDiffStructure_ColumnOptionValueDriftIsStillReported(t *testing.T) { + setupStructureSchema(t) + ctx := context.Background() + + _, err := pgCluster.Node1Pool.Exec(ctx, + fmt.Sprintf(`ALTER TABLE %s.%s ALTER COLUMN n SET (n_distinct = -0.5)`, structureSchema, structureTable)) + require.NoError(t, err) + _, err = pgCluster.Node2Pool.Exec(ctx, + fmt.Sprintf(`ALTER TABLE %s.%s ALTER COLUMN n SET (n_distinct = -0.9)`, structureSchema, structureTable)) + require.NoError(t, err) + + divs := compareStructureForTest(t) + + d := findDivergenceFor(t, divs, fmt.Sprintf("%s.%s.n", structureSchema, structureTable), "options") + require.Contains(t, d.ValueOnA, "-0.5") + require.Contains(t, d.ValueOnB, "-0.9") +} + +// TestSchemaDiffStructure_TableDroppedBeforeCollectionIsOneFinding: a table +// that disappears before CollectSnapshot reads the catalog is one finding, +// on the table. Until the table Object was gated on the table existing, +// both snapshots held one either way, so Compare never took its absent +// branch and reported every column instead. +func TestSchemaDiffStructure_TableDroppedBeforeCollectionIsOneFinding(t *testing.T) { + setupStructureSchema(t) + ctx := context.Background() + + _, err := pgCluster.Node2Pool.Exec(ctx, + fmt.Sprintf(`DROP TABLE %s.%s`, structureSchema, structureTable)) + require.NoError(t, err) + + divs := compareStructureForTest(t) + + require.Len(t, divs, 1, + "a table missing on one node is one finding, not one per column: %+v", divs) + require.Equal(t, "table", divs[0].Kind) + require.Equal(t, schema.RankAbsent, divs[0].Rank) + require.Equal(t, "present", divs[0].ValueOnA) + require.Equal(t, "absent", divs[0].ValueOnB) +} + +// TestSchemaDiffStructure_ConstraintCountMatchesTheReport: constraint +// findings all carry the table and no Property, so a summary keyed on +// object+kind+property counted several as one, disagreeing with the report +// printed right above it. FindingKey keeps the two in step. +func TestSchemaDiffStructure_ConstraintCountMatchesTheReport(t *testing.T) { + setupStructureSchema(t) + ctx := context.Background() + + // Three constraints on node1 only, all on the one table, so all three + // findings share an Object, a Kind, and an empty Property. + for _, stmt := range []string{ + fmt.Sprintf(`ALTER TABLE %s.%s ADD CONSTRAINT n_positive CHECK (n > 0)`, structureSchema, structureTable), + fmt.Sprintf(`ALTER TABLE %s.%s ADD CONSTRAINT n_bounded CHECK (n < 1000)`, structureSchema, structureTable), + fmt.Sprintf(`ALTER TABLE %s.%s ADD CONSTRAINT code_unique UNIQUE (code)`, structureSchema, structureTable), + } { + _, err := pgCluster.Node1Pool.Exec(ctx, stmt) + require.NoError(t, err) + } + + divs := compareStructureForTest(t) + + var constraintFindings int + for _, d := range divs { + if d.Kind == "constraint" { + constraintFindings++ + } + } + require.Equal(t, 3, constraintFindings, + "the three one-sided constraints are three findings: %+v", divs) + + task := structureTaskForTest() + var runErr error + captureStdout(t, func() { + runErr = task.SchemaTableDiff() + }) + require.Error(t, runErr) + + // The summary counts distinct findings across node pairs, so it must + // agree with the three the report itself lists. + require.Contains(t, runErr.Error(), "3 divergence(s) found", + "the summary must count each constraint, not collapse them: %v", runErr) +} diff --git a/tests/integration/scope_test.go b/tests/integration/scope_test.go new file mode 100644 index 00000000..dc9aa78f --- /dev/null +++ b/tests/integration/scope_test.go @@ -0,0 +1,72 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package integration + +import ( + "context" + "fmt" + "testing" + + "github.com/pgedge/ace/internal/consistency/scope" + "github.com/stretchr/testify/require" +) + +// TestSchemaProvider_ResolvesBaseTablesOnly verifies that scope.SchemaProvider +// returns every base table in a schema and excludes views, since views are +// DDL-only and schema-diff does not compare their data. +func TestSchemaProvider_ResolvesBaseTablesOnly(t *testing.T) { + ctx := context.Background() + tableName := "scope_provider_table" + viewName := "scope_provider_view" + + _, err := pgCluster.Node1Pool.Exec(ctx, fmt.Sprintf( + `CREATE TABLE IF NOT EXISTS %s.%s (id INT PRIMARY KEY)`, testSchema, tableName)) + require.NoError(t, err, "create table") + _, err = pgCluster.Node1Pool.Exec(ctx, fmt.Sprintf( + `CREATE OR REPLACE VIEW %s.%s AS SELECT id FROM %s.%s`, testSchema, viewName, testSchema, tableName)) + require.NoError(t, err, "create view") + + t.Cleanup(func() { + pgCluster.Node1Pool.Exec(ctx, fmt.Sprintf(`DROP VIEW IF EXISTS %s.%s`, testSchema, viewName)) //nolint:errcheck + pgCluster.Node1Pool.Exec(ctx, fmt.Sprintf(`DROP TABLE IF EXISTS %s.%s CASCADE`, testSchema, tableName)) //nolint:errcheck + }) + + provider := scope.SchemaProvider{SchemaName: testSchema} + resolved, err := provider.Resolve(ctx, pgCluster.Node1Pool) + require.NoError(t, err) + + var sawTable, sawView bool + for _, q := range resolved.Tables { + require.Equal(t, testSchema, q.Schema, "every entry must belong to the requested schema") + switch q.Table { + case tableName: + sawTable = true + case viewName: + sawView = true + } + } + require.True(t, sawTable, "base table must be in scope") + require.False(t, sawView, "view must not be in scope: schema-diff treats views as DDL-only") + require.Equal(t, provider.Describe(), resolved.Source) +} + +// TestSchemaProvider_UnknownSchemaIsEmptyNotError checks that Resolve leaves +// schema-existence checks to the caller: an unknown schema resolves to zero +// tables, not an error. +func TestSchemaProvider_UnknownSchemaIsEmptyNotError(t *testing.T) { + ctx := context.Background() + provider := scope.SchemaProvider{SchemaName: "schema_that_does_not_exist_anywhere"} + + resolved, err := provider.Resolve(ctx, pgCluster.Node1Pool) + require.NoError(t, err) + require.Empty(t, resolved.Tables) +} diff --git a/tests/integration/table_diff_schema_mismatch_test.go b/tests/integration/table_diff_schema_mismatch_test.go new file mode 100644 index 00000000..8aa6f455 --- /dev/null +++ b/tests/integration/table_diff_schema_mismatch_test.go @@ -0,0 +1,119 @@ +// /////////////////////////////////////////////////////////////////////////// +// +// # ACE - Active Consistency Engine +// +// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/) +// +// This software is released under the PostgreSQL License: +// https://opensource.org/license/postgresql +// +// /////////////////////////////////////////////////////////////////////////// + +package integration + +import ( + "context" + "fmt" + "testing" + + "github.com/jackc/pgx/v5/pgxpool" + "github.com/stretchr/testify/require" +) + +// These tests cover the detailed error table-diff raises when its cheap +// column/key-name check finds the two nodes disagree: RunChecks hands off +// to diagnoseSchemaMismatch, which collects both nodes' structure and +// reports what actually differs instead of just saying that something does. + +const mismatchTable = "ace_mismatch_test" + +// setupKeyMismatch gives both nodes the same columns but different primary +// keys, which is what makes RunChecks' key comparison disagree while its +// column comparison does not. +func setupKeyMismatch(t *testing.T) { + t.Helper() + ctx := context.Background() + + drop := fmt.Sprintf(`DROP TABLE IF EXISTS %s.%s`, testSchema, mismatchTable) + for _, pool := range []*pgxpool.Pool{pgCluster.Node1Pool, pgCluster.Node2Pool} { + _, err := pool.Exec(ctx, drop) + require.NoError(t, err) + } + + _, err := pgCluster.Node1Pool.Exec(ctx, fmt.Sprintf( + `CREATE TABLE %s.%s (id int NOT NULL, tenant int NOT NULL, note text, PRIMARY KEY (id))`, + testSchema, mismatchTable)) + require.NoError(t, err) + _, err = pgCluster.Node2Pool.Exec(ctx, fmt.Sprintf( + `CREATE TABLE %s.%s (id int NOT NULL, tenant int NOT NULL, note text, PRIMARY KEY (id, tenant))`, + testSchema, mismatchTable)) + require.NoError(t, err) + + t.Cleanup(func() { + for _, pool := range []*pgxpool.Pool{pgCluster.Node1Pool, pgCluster.Node2Pool} { + pool.Exec(ctx, drop) //nolint:errcheck // best-effort cleanup + } + }) +} + +// TestTableDiffSchemaMismatch_KeyColumnsAreReadable: key_columns is +// compared in the packed form, so a key of the columns (a, b) stays +// distinct from a key of the one column named "a,b". That form must not +// reach the error text: a two-column key reads "id, tenant". +func TestTableDiffSchemaMismatch_KeyColumnsAreReadable(t *testing.T) { + setupKeyMismatch(t) + + qualified := fmt.Sprintf("%s.%s", testSchema, mismatchTable) + task := newTestTableDiffTask(t, qualified, []string{serviceN1, serviceN2}) + task.Ctx = context.Background() + + err := task.RunChecks(false) + require.Error(t, err, "the two nodes have different primary keys, so the check must fail") + + msg := err.Error() + require.Contains(t, msg, "key_columns", + "the error should name the property that differs: %s", msg) + require.Contains(t, msg, "id, tenant", + "a two-column key must be readable: %s", msg) + require.NotContains(t, msg, "6:tenant", + "the packed form escaped into the error: %s", msg) +} + +// TestTableDiffSchemaMismatch_NarrowingNamesTheNarrowSide: a narrowing is +// only actionable if the message says which node is the narrow one. +func TestTableDiffSchemaMismatch_NarrowingNamesTheNarrowSide(t *testing.T) { + ctx := context.Background() + drop := fmt.Sprintf(`DROP TABLE IF EXISTS %s.%s`, testSchema, mismatchTable) + for _, pool := range []*pgxpool.Pool{pgCluster.Node1Pool, pgCluster.Node2Pool} { + _, err := pool.Exec(ctx, drop) + require.NoError(t, err) + } + t.Cleanup(func() { + for _, pool := range []*pgxpool.Pool{pgCluster.Node1Pool, pgCluster.Node2Pool} { + pool.Exec(ctx, drop) //nolint:errcheck // best-effort cleanup + } + }) + + // Different column sets, so RunChecks' column comparison disagrees, and + // a differing type on the column they share, so the collected structure + // has a narrowing in it to report. + _, err := pgCluster.Node1Pool.Exec(ctx, fmt.Sprintf( + `CREATE TABLE %s.%s (id int PRIMARY KEY, qty int8)`, testSchema, mismatchTable)) + require.NoError(t, err) + _, err = pgCluster.Node2Pool.Exec(ctx, fmt.Sprintf( + `CREATE TABLE %s.%s (id int PRIMARY KEY, qty int4, extra text)`, testSchema, mismatchTable)) + require.NoError(t, err) + + qualified := fmt.Sprintf("%s.%s", testSchema, mismatchTable) + task := newTestTableDiffTask(t, qualified, []string{serviceN1, serviceN2}) + task.Ctx = context.Background() + + err = task.RunChecks(false) + require.Error(t, err) + + msg := err.Error() + require.Contains(t, msg, "narrower on", + "a narrowing must name the narrow node: %s", msg) + require.Contains(t, msg, serviceN2, + "node2 holds the int4, so it is the narrow side: %s", msg) +}