search

theory / tap-parser-sourcehandler-pgtap

TAP::Parser::SourceHandler::pgTAP
http://search.cpan.org/dist/TAP::Parser::SourceHandler::pgTAP
11 stars 13 forks source link

Use named types in pg_tapgen generated function body tests #34

Closed deathwish closed 3 years ago

deathwish commented 4 years ago

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.

theory commented 4 years ago

Is oidvectortypes() more portable than pg_get_function_identity_arguments()?

deathwish commented 3 years ago

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 commented 3 years ago

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

deathwish commented 3 years ago

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 commented 3 years ago

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.