feat: add constraints in .pgschemaignore#430
Conversation
Greptile SummaryThis PR adds a
Confidence Score: 4/5Safe to merge with minor polish; the core filtering logic is correct and consistent with existing patterns. The ignore guard in buildConstraints is placed correctly and the end-to-end wiring from TOML to IgnoreConfig to inspector is sound. The issues found are a stale copy-pasted doc comment, a plural method name that breaks the established ShouldIgnore[Singular] convention, and no integration tests for the new section — none of these affect runtime correctness today. cmd/util/ignoreloader.go needs its stale doc comment fixed; ir/ignore.go and ir/inspector.go need the method renamed from plural to singular to stay consistent with the rest of the codebase. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant ignoreloader as cmd/util/ignoreloader.go
participant IgnoreConfig as ir/ignore.go (IgnoreConfig)
participant Inspector as ir/inspector.go (Inspector)
participant PG as PostgreSQL
User->>ignoreloader: LoadIgnoreFileWithStructureFromPath()
ignoreloader->>ignoreloader: toml.DecodeFile to TomlConfig Constraints
ignoreloader->>IgnoreConfig: IgnoreConfig Constraints from tomlConfig.Constraints.Patterns
Inspector->>PG: GetConstraintsForSchema(targetSchema)
PG-->>Inspector: constraint rows
loop each constraint row
Inspector->>IgnoreConfig: ShouldIgnoreConstraints(constraintName)
alt pattern matches
IgnoreConfig-->>Inspector: true, skip row
else no match
IgnoreConfig-->>Inspector: false, process constraint
Inspector->>Inspector: accumulate into constraintGroups
end
end
Inspector->>Inspector: attach constraints to tables in IR
Reviews (1): Last reviewed commit: "🚀 add constraints in .pgschemaignore" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // ConstraintsIgnoreConfig represents default privilege-specific ignore configuration | ||
| // Patterns match on grantee role names | ||
| type ConstraintsIgnoreConfig struct { | ||
| Patterns []string `toml:"patterns,omitempty"` | ||
| } |
There was a problem hiding this comment.
ConstraintsIgnoreConfig
The doc comment for this type was copied verbatim from DefaultPrivilegeIgnoreConfig and still describes grantee role names, which is unrelated to constraints. Any reader trying to understand what patterns should look like will be misled into thinking they should supply role names instead of constraint names.
| // ConstraintsIgnoreConfig represents default privilege-specific ignore configuration | ||
| // Patterns match on grantee role names | ||
| type ConstraintsIgnoreConfig struct { | ||
| Patterns []string `toml:"patterns,omitempty"` | ||
| } |
There was a problem hiding this comment.
constraints ignore section
Every other ignore category (tables, views, functions, sequences, privileges, default_privileges) has coverage in cmd/ignore_integration_test.go. A foreign-key scenario is the explicit motivation for this PR, yet there are no tests that verify a cross-schema FK is correctly filtered out of the dump/plan output. Without this, a regression (e.g., pattern not matching, scoping bug) would go undetected.
rename func ShouldIgnoreConstraints Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
rename ShouldIgnoreConstraints Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds support for ignoring constraints via .pgschemaignore to better support multi-module deployments where cross-schema foreign keys/constraints should not be managed by a given module’s migration runs (Issue #429).
Changes:
- Added
constraintspatterns toir.IgnoreConfigplus aShouldIgnoreConstrainthelper. - Extended
.pgschemaignorestructured TOML loader to parse a new[constraints]section. - Updated DB inspection to skip building constraints whose names match the ignore patterns.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| ir/inspector.go | Skips building ignored constraints during DB inspection. |
| ir/ignore.go | Adds constraints ignore patterns and ShouldIgnoreConstraint. |
| cmd/util/ignoreloader.go | Adds TOML parsing/mapping for [constraints] section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Check if constraint should be ignored | ||
| if i.ignoreConfig != nil && i.ignoreConfig.ShouldIgnoreConstraint(constraintName) { | ||
| continue |
| Sequences SequenceIgnoreConfig `toml:"sequences,omitempty"` | ||
| Privileges PrivilegeIgnoreConfig `toml:"privileges,omitempty"` | ||
| DefaultPrivileges DefaultPrivilegeIgnoreConfig `toml:"default_privileges,omitempty"` | ||
| Constraints ConstraintsIgnoreConfig `toml:"constraints,omitempty"` | ||
| } |
| Types []string `toml:"types,omitempty"` | ||
| Sequences []string `toml:"sequences,omitempty"` | ||
| Privileges []string `toml:"privileges,omitempty"` | ||
| DefaultPrivileges []string `toml:"default_privileges,omitempty"` | ||
| Constraints []string `toml:"constraints,omitempty"` |
| func (c *IgnoreConfig) ShouldIgnoreConstraint(constraintName string) bool { | ||
| if c == nil { | ||
| return false | ||
| } | ||
| return c.shouldIgnore(constraintName, c.Constraints) |
| // ConstraintsIgnoreConfig represents default privilege-specific ignore configuration | ||
| // Patterns match on grantee role names |
| // TomlConfig represents the TOML structure of the .pgschemaignore file | ||
| // This is used for parsing more complex configurations if needed in the future | ||
| type TomlConfig struct { | ||
| Tables TableIgnoreConfig `toml:"tables,omitempty"` | ||
| Views ViewIgnoreConfig `toml:"views,omitempty"` | ||
| Functions FunctionIgnoreConfig `toml:"functions,omitempty"` | ||
| Procedures ProcedureIgnoreConfig `toml:"procedures,omitempty"` | ||
| Types TypeIgnoreConfig `toml:"types,omitempty"` | ||
| Sequences SequenceIgnoreConfig `toml:"sequences,omitempty"` | ||
| Privileges PrivilegeIgnoreConfig `toml:"privileges,omitempty"` | ||
| DefaultPrivileges DefaultPrivilegeIgnoreConfig `toml:"default_privileges,omitempty"` | ||
| Constraints ConstraintsIgnoreConfig `toml:"constraints,omitempty"` | ||
| } |
| // ShouldIgnoreConstraint checks if a constraint should be ignored based on the patterns | ||
| func (c *IgnoreConfig) ShouldIgnoreConstraint(constraintName string) bool { | ||
| if c == nil { | ||
| return false | ||
| } | ||
| return c.shouldIgnore(constraintName, c.Constraints) | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
RicardoBorgesGO
changed the title
This PR a scenario where the system is separated into large modules, each module has a separate schema, but there are foreign keys between these schemas, making it impossible to use pgschema for migrations.
Issue #429