Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

create table_constraint_checksum function for pg_tapgen #154

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

marco44
Copy link
Contributor

@marco44 marco44 commented Feb 4, 2018

Another request for comments, still linked with theory/tap-parser-sourcehandler-pgtap#18

This would make checking all constraints much simpler in pg_tapgen I think… pg_tapgen could generate all check for constraints in a database in one go, calling only this function systematically.

If you agree, please tell me how to proceed next… should there be all combinations of this function's parameters available ? (with and without schema, with and without description… there could be a great amount of possible combinations).

The thing I don't totally feel comfortable with this is if pg_get_constraintdef changes between releases. But consrc could change also anyway...

@theory
Copy link
Owner

theory commented Feb 5, 2018

If you're thinking you might want to write a slew of these functions, you might want to consider making your own extension that depends on pgTAP and release it on PGXN.

@marco44
Copy link
Contributor Author

marco44 commented Feb 5, 2018

I just would like to add the minimum to be able to extend pg_tapgen to test more things. Shouldn't this be in pg_tab, then ?

@theory
Copy link
Owner

theory commented Feb 5, 2018

Sure, that make sense. Do please add tests, however.

@marco44
Copy link
Contributor Author

marco44 commented Feb 5, 2018

Yes, I will, of course. And doc I presume. I just wanted to know if what I started made sense...

@theory
Copy link
Owner

theory commented Feb 5, 2018

Looks okay to me, though I agree that it could get annoying if the output of pg_get_constraintdef changes between releases.

@marco44
Copy link
Contributor Author

marco44 commented Feb 5, 2018

Yes. It could be the same problem using consrc/confkey anyway… I don't see a smarter way out of this.

@theory
Copy link
Owner

theory commented Feb 5, 2018

Agreed, but maybe check with pgsql-hackers.

@marco44
Copy link
Contributor Author

marco44 commented Feb 6, 2018

Just added the doc. No response from -hackers yet.

@marco44
Copy link
Contributor Author

marco44 commented Feb 27, 2018

Still nothing on hackers :)

So I guess no one has a much better idea

Copy link
Owner

@theory theory left a comment

Choose a reason for hiding this comment

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

Hi @marco44. Could you add some tests for the new function, please? You can add them to unique.sql, check.sql, pktap.sql, and fktap.sql in test/sql (and the corresponding *.out tests in test/expected). Thank you!

RETURNS TEXT AS $$
SELECT is(md5 ( pg_get_constraintdef(oid)), $5, $6)
FROM pg_constraint
WHERE connamespace=(SELECT oid FROM pg_namespace WHERE nspname=$1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe just use $1::regnamespace. Or for that matter, maybe the function should just accept regnamespace. At some point we should really have function versions that accept reg* pseudo-types instead of forcing people to segregate schema from object name.

@nasbyj
Copy link
Collaborator

nasbyj commented Nov 14, 2019

Please merge current master as well; it has much improved automated testing.

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.

None yet

3 participants