Skip to content

Add constraint checking functions and tests - #363

Closed
RampantDespair wants to merge 10 commits into
theory:mainfrom
RampantDespair:dev
Closed

Add constraint checking functions and tests#363
RampantDespair wants to merge 10 commits into
theory:mainfrom
RampantDespair:dev

Conversation

@RampantDespair

Copy link
Copy Markdown
Contributor

Implemented new functions for checking the existence of constraints in PostgreSQL: _has_constraint, has_constraint, and hasnt_constraint. These functions support both schema-qualified and unqualified table names, along with type and description parameters.

Additionally, added comprehensive tests for these functions in constraint.sql, ensuring they handle various scenarios correctly, including existence checks and type validation. Created expected output for the tests in constraint.out to verify functionality.

This update enhances the constraint validation capabilities of pgTAP, providing users with more robust testing options.

Implemented new functions for checking the existence of constraints in PostgreSQL: `_has_constraint`, `has_constraint`, and `hasnt_constraint`. These functions support both schema-qualified and unqualified table names, along with type and description parameters.

Additionally, added comprehensive tests for these functions in `constraint.sql`, ensuring they handle various scenarios correctly, including existence checks and type validation. Created expected output for the tests in `constraint.out` to verify functionality.

This update enhances the constraint validation capabilities of pgTAP, providing users with more robust testing options.
@RampantDespair

Copy link
Copy Markdown
Contributor Author

@theory
This is ready for review.

@theory

theory commented Jun 1, 2026

Copy link
Copy Markdown
Owner

I'm a little confused. We already have _hasc() and the functions that use it. This appears to be a duplication. Is there something you need that's missing?

It also recreates the functions in the tests, which should not be necessary.

This looks like a bot-created change. Please don't submit machine-generated code. If there is a feature you need, please implement it or ask for it in an issue.

@RampantDespair

RampantDespair commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

I'm a little confused. We already have _hasc() and the functions that use it. This appears to be a duplication. Is there something you need that's missing?

I didn't know it existed, it's undocumented...

And in any case, it doesn't cover my use-case, which is:

        ok(
            EXISTS(
                SELECT FROM pg_constraint
                INNER JOIN pg_class ON pg_constraint.conrelid = pg_class.oid
                INNER JOIN pg_namespace ON pg_class.relnamespace = pg_namespace.oid
                WHERE
                    pg_namespace.nspname = 'schema'
                    AND pg_class.relname = 'table'
                    AND pg_constraint.conname = 'ck__something__something'
                    AND pg_constraint.contype = 'c'
                    AND pg_constraint.coninhcount = 0
            ),
            'Constraint ck__something__something' should exist'::TEXT
        ),

The has_check doesn't cover that either, it just checks whether a table has a check on it, isn't useful when there's more than one. Same goes for col_has_check.
The same goes for other constraint variations.

It also recreates the functions in the tests, which should not be necessary.

I can fix that.

This looks like a bot-created change. Please don't submit machine-generated code. If there is a feature you need, please implement it or ask for it in an issue.

I can redo the PR to build on top of _hasc because once again, I wasn't aware of it's existence. Does that sound good?

@RampantDespair

RampantDespair commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

@theory
Thoughts on these new checks?
It's now built upon _hasc, I kept the generic has_constraint and hasnt_constraint.
I also took the liberty of extending has_check, hasnt_check, has_fk, hasnt_fk, has_pk, hasnt_pk, has_unique and hasnt_unique.

I also added has_exclude and hasnt_exclude.
I could probably add your generic "does table have an exclude at all" for symmetry with the other checks.

And speaking of symmetry, I notice there wasnt any hasnt_check unlike for fk, pk and unique.
If you want I can add that as well.

Finally I also noted overlap problems which I suppose I'll need to resolve assuming you think this addition as a place in your project. Which didn't really seem to be the case from your first response, hopefully this fix changes that.

@theory

theory commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Since we have the other constraint-checking functions, I don't think you need add public functions char argument. If the goal is to complement the existing constraint-checking functions by adding a name argument, do that.

[Reads some more]

Oh, I see you have those in the upgrade script. They need to be in pgtap.sql.in, too. All changed canonically go into that file; then dupe them in the upgrade script.

I guess I have no objection to also adding a generic has_constraint() script to which one can pass the constraint type char argument, since the others are there, too. Yes, adding hasnt_check() would also be welcome (unless it creates some kind of conflict).

Please also add documentation to doc/pgtap.md and tests for all of the new functions.

@theory theory self-assigned this Jun 2, 2026
@RampantDespair

Copy link
Copy Markdown
Contributor Author

Oh, I see you have those in the upgrade script. They need to be in pgtap.sql.in, too. All changed canonically go into that file; then dupe them in the upgrade script.

Appended at the bottom, or alongside next of kin (to maintain structure)?

IE:
has_pk
has_pk
has_pk
[new_]has_pk
etc.

@RampantDespair

Copy link
Copy Markdown
Contributor Author

Okay this is ready for another round of feedback / review.
Pending questions:

  • What are your thoughts on the overlaps present in the has/hasnt tests documented here 27dbf34 (this PR)
  • And what are the guidelines when editing pgtap.sql.in, is it append-only? or should I place stuff where it most makes sense in terms of consistency and structure?

@RampantDespair

Copy link
Copy Markdown
Contributor Author

@theory Any news?

@theory

theory commented Jun 9, 2026

Copy link
Copy Markdown
Owner

I'm traveling, so only working on stuff like this occasionally. Might be a few weeks before I get back to it. Apologies.

@theory theory left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the submission. However, this PR doesn't quite follow pgTAP's patterns. To whit, we already have has_*() functions for every type of constraint except constraint trigger and exclusion constraint. If you just want the ability to also specify the constraint name, just add alternatives.

I suggest making two PRs: One to add alternate signatures for the existing constraint functions to test constraint names, and a second to add the missing has_*() constraint functions.

Other general comments:

  • All changes must be made to pgtap.sql.in; it's the only script run most of the time. Duplicate anything that is added or changed in sql/pgtap--1.3.4--1.3.5.sql.
  • Add tests for all changes, not just new functions. This PR doesn't test the new signatures for the has_*() functions.
  • Document the new functions and function signatures in doc/pgtap.md
  • List the changes in Changes

@RampantDespair

Copy link
Copy Markdown
Contributor Author

To whit, we already have has_*() functions for every type of constraint except constraint trigger and exclusion constraint.

Kind of but it's not symmetrical at all and constraint names aren't supported.

I suggest making two PRs: One to add alternate signatures for the existing constraint functions to test constraint names, and a second to add the missing has_*() constraint functions.

Agreed, I got carried away and probably 3 PRs might be the better call, because there's 3 pending issues:

  • implementation symmetry
  • supporting constraint names
  • supporting new constraint types

Can you confirm that's a better way to go?

All changes must be made to pgtap.sql.in; it's the only script run most of the time. Duplicate anything that is added or changed in sql/pgtap--1.3.4--1.3.5.sql.

Got it.

Add tests for all changes, not just new functions. This PR doesn't test the new signatures for the has_*() functions.
Document the new functions and function signatures in doc/pgtap.md
List the changes in Changes

I was waiting for feedback before getting into all that.

All this being said, your thoughts on the overlap problems?
https://github.com/theory/pgtap/pull/363/changes#diff-8082998251e8e6435656f047a1806be03546d95c6c5f70bcc64b017f3869bca4R256
https://github.com/theory/pgtap/pull/363/changes#diff-8082998251e8e6435656f047a1806be03546d95c6c5f70bcc64b017f3869bca4R674
Or do you want to continue this conversation in the new PRs?

@theory

theory commented Jul 23, 2026

Copy link
Copy Markdown
Owner

Can you confirm that's a better way to go?

Yes.

All this being said, your thoughts on the overlap problems?

Overlap problems?

Or do you want to continue this conversation in the new PRs?

I'm fine to keep chatting here if it's helpful until you've made the other PRs.

@RampantDespair

Copy link
Copy Markdown
Contributor Author

Overlap problems?

The ones clearly indicated in the source code and linked here:
#363 (changes)
#363 (changes)

But regardless, part 1 (#369) and 2 (#370) are out, I'll wait for those to get resolved and merged before going forward with the exclude and constraint_name PRs. So in the end it will be 4 bite size PRs instead of 1 monster PR.

@theory theory closed this in #370 Aug 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants