fix: strip same-schema qualifiers from CHECK constraint expressions (#445)

[andyc-valstro]

Summary

pgschema plan was reporting unresolvable and perpetual diffs on CHECK constraints that reference functions or custom types defined in the same schema being managed. The plan would constantly want to drop and re-create these constraints, even immediately after a successful pgschema apply.

Root cause: When comparing desired vs current state, CHECK constraint expressions were normalized inconsistently:

• Desired state (loaded into temp schema): After normalizeSchemaNames replaces temp schema references and strips qualifiers, function calls become unqualified (e.g., validate_foo(val))
• Current state (read from target DB): pg_get_constraintdef() may render same-schema references as qualified (e.g., public.validate_foo(val)) depending on the session's search_path, and normalizeCheckClause did NOT strip these

Fix: Extended normalizeCheckClause to accept the table's schema and strip same-schema qualifiers from:

• Function call references: public.validate_foo(val) → validate_foo(val)
• Type cast references: ::public.status_enum → ::status_enum

This matches the existing normalization pattern already used by:

• Domain constraints (normalizeDomainConstraint)
• Column defaults (normalizeDefaultValue)
• Policy expressions (normalizePolicyExpression)

Cross-schema qualifiers are preserved (e.g., other_schema.validate_foo(val) stays qualified).

Fixes pgplex#445

Test plan

1. Added unit tests to TestNormalizeCheckClause covering:

   • Stripping same-schema function qualifiers
   • Stripping same-schema type cast qualifiers
   • Preserving cross-schema qualifiers
   • No stripping when tableSchema is empty

2. Added diff/integration test case testdata/diff/create_table/issue_445_check_constraint_schema_qualifier/ verifying no diff when old=new with function-referencing CHECK constraint.

go test -v ./ir -run TestNormalizeCheckClause
PGSCHEMA_TEST_FILTER="create_table/issue_445" go test -v ./internal/diff -run TestDiffFromFiles
PGSCHEMA_TEST_FILTER="create_table/issue_445" go test -v ./cmd -run TestPlanAndApply

All 150+ existing diff tests, dump tests, and integration tests pass with no regressions.

  

[wellcode-ai]

🔍 General Code Quality Feedback

🔍 Comprehensive Code Review

Consolidated Feedback

• 🔍 Code Review Analysis

Overall Assessment: The pull request addresses a critical issue with CHECK constraint normalization in the pgschema tool, enhancing its reliability. The changes are well-structured and include comprehensive tests, but there are areas for improvement in documentation and code clarity.

Critical Issues:

• Issue 1: Missing Documentation for New Functionality → The new parameter tableSchema in normalizeCheckClause and normalizeConstraint lacks adequate documentation. It is essential to explain its purpose and usage to ensure maintainability. Actionable solution: Add comments to these functions detailing the new parameter and its role in the normalization process.
• Issue 2: Potential for Confusion with Naming → The function normalizeCheckClause could be misleading as it now handles more than just normalization; it also strips qualifiers. Actionable solution: Consider renaming the function to normalizeAndStripCheckClause or similar to reflect its expanded functionality.

Improvements:

• Suggestion 1: Enhance Test Coverage → While the tests cover various scenarios, consider adding edge cases, such as handling invalid input or unexpected formats in checkClause. How to implement: Create additional test cases in TestNormalizeCheckClause that simulate these edge cases.
• Suggestion 2: Refactor Complex Logic → The logic for stripping qualifiers in normalizeCheckClause could be extracted into a separate helper function for clarity. How to implement: Create a function stripSameSchemaQualifiers that takes checkClause and tableSchema as parameters and returns the cleaned clause.

Positive Notes:

• The addition of unit tests is commendable, as it ensures that the new functionality is well-tested and reduces the risk of regressions. The thoroughness of the test plan demonstrates a strong commitment to quality.
• The code follows a consistent pattern with existing normalization functions, which aids in maintainability and understanding of the codebase.

Next Steps:

1. Add Documentation: Update the documentation for normalizeCheckClause and normalizeConstraint to include details about the new tableSchema parameter.
2. Rename Functions: Consider renaming normalizeCheckClause to better reflect its functionality.
3. Enhance Test Coverage: Add edge case tests to TestNormalizeCheckClause to ensure robustness.
4. Refactor Logic: Extract the qualifier stripping logic into a separate helper function for improved clarity and maintainability.
5. Review Naming Conventions: Ensure that all variable and function names are clear and descriptive to avoid confusion in the future.

By addressing these issues and suggestions, the code quality and maintainability of the project will be significantly improved, leading to a more robust and reliable system.

🤖 Generated by Wellcode.ai