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

Use named types in pg_tapgen generated function body tests #34

Conversation

deathwish
Copy link
Contributor

Currently, function body MD5 tests look up the specialization of the function being tested using the raw OIDs of it's arguments. These OIDs are only consistent between databases for built-in types. This means functions using extension types (I.E. hstore) will have differing argument OIDs on different machines, causing tests to fail. Worse, due to the structure of the query the test is not run, meaning only a plan failure results.

This PR corrects both issues in what I believe is a reasonable way for the majority of cases. Unlike the more sophisticated function introspection tools available today, oidvectortypes is available on all supported PG versions. And while plan failures will still result if the schema is not present, other direct failures will invariably result from such a situation. As such, I felt keeping the query simple was worth a moderate compromise in function.

The duplicate name elimination is just a bonus, the only function change is shorter output. Included as I feel it makes it clearer that this test does not count overloads of the same function name.

Also reorder the joins and conditions generated for these tests
in order to avoid a plan failure if the given function does not exist.

Cumulatively, this fixes theory#21.
@theory
Copy link
Owner

theory commented Sep 6, 2020

Is oidvectortypes() more portable than pg_get_function_identity_arguments()?

bin/pg_tapgen Outdated Show resolved Hide resolved
@deathwish
Copy link
Contributor Author

It appears that pg_get_function_identity_arguments was introduced in 8.4, as it is not present in the 8.3 documentation. By contrast oidvectortypes seems to have been present in the 7.x series, so I would say it is notably more portable.

@theory
Copy link
Owner

theory commented Sep 12, 2020

Yeah but pgTAP supports 9.1 and later (as of v1.1.0).

Also improve the implementation of distinct function name selection
for functions_are.
@deathwish
Copy link
Contributor Author

The documentation on pgtap.org is still for 1.0 and the pgTAP 1.1 README.md still reflects a minimum PostgreSQL version requirement of 8.4, I think some confusion is forgivable.

That said, my experiments suggest that pg_get_function_identity_arguments is not the right tool as it returns any names used for parameters. For example, it would return s text, y varchar(10) rather than text, varchar(10). This does not align with the definition of the regprocedure type, which appears to be the other available argument-formatting mechanism.

In turn, regprocedure has it's own problem for this use case: the TEXT format of the type is relative to the current user's search path. Canonicalization or extraction of the desired parameter type string are both possible, but either are significantly messier than simply calling oidvectortypes and getting the desired output.

As no further procedure parameter information functionality has been added to PostgreSQL to date, I think sticking with oidvectortypes until better-suited functionality is available seems reasonable.

@theory
Copy link
Owner

theory commented Oct 24, 2020

The documentation on pgtap.org is still for 1.0 and the pgTAP 1.1 README.md still reflects a minimum PostgreSQL version requirement of 8.4, I think some confusion is forgivable.

That's fair. I've opened theory/pgtap#262 to clean out the old cruft.

As no further procedure parameter information functionality has been added to PostgreSQL to date, I think sticking with oidvectortypes until better-suited functionality is available seems reasonable.

Okay, thanks for digging into that.

@theory theory merged commit c7477db into theory:develop Oct 24, 2020
@deathwish deathwish deleted the BUGFIX-use_named_types_for_generated_proc_tests branch November 12, 2020 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants