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

Fix metadata functions getProcedures() and getFunctions() to ignore search_path #2174

Merged
merged 3 commits into from Jun 11, 2021

Conversation

sehrope
Copy link
Member

@sehrope sehrope commented Jun 7, 2021

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests?
  2. Does ./gradlew autostyleCheck checkstyleAll pass ?

Changes to Existing Features:

  • Does this break existing behaviour? If so please explain.

Fixes getProcedures() and getFunctions() to not filter by search_path when no schema pattern is specified.

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully run tests with your changes locally?

Closes #2173. See also discussion on #1633.

This PR changes those two metadata functions to ignore the search_path. Previously without a schema pattern they restricted the results to the active search path.

The rest of the changes update the tests to reflect that behavior and subsequently clean up and improve them a bit. The last commit adds some more detail to the getProcedures() test to validate the result set columns similar to how were were already validation the result set of getFunctions().

@sehrope sehrope changed the title Fix metadata viz Fix metadata functions getProcedures() and getFunctions() to ignore search_path Jun 7, 2021
@davecramer
Copy link
Member

Using try/catch in the tests will not allow me to backpatch this ... :(

@sehrope
Copy link
Member Author

sehrope commented Jun 7, 2021

Force pushed to add the PR details to the change log as well.

The main CI action only runs on the latest version of PG server. I have it running against the omni matrix with all the other server versions on my fork here: https://github.com/sehrope/pgjdbc/actions/runs/915786302. Locally it worked fine on all the versions I checked and looks like it's 2/3 done clearing the full matrix.

@sehrope
Copy link
Member Author

sehrope commented Jun 7, 2021

Which branch are you going to back patch into? I thought we had already completely dropped everything less than JDK 8.

@davecramer
Copy link
Member

42.2.21 is still what folks are using until we release 42.3.0 which 1) won't be stable and 2) doesn't seem to be imminent

@sehrope
Copy link
Member Author

sehrope commented Jun 7, 2021

Dang. I really thought we were done with those ancient JDKs. My fault for trying to improve the tests ;-)

The fix itself applies clean but the test won't work on the old JDK and without a change won't pass either as they expect search path changes to be reflected.

Speaking of which, how do we test this anyway? None of Actions or Travis CI runs those old JDKs and I don't have either 6 or 7 installed right now either. If you think it's worthwhile to backpatch this I can get it running and "ancient-ize" the tests so it applies uniformly. IIRC, with the gradle changes we'd have to build on 8+ targeting 1.6 as gradle itself won't run on there either.

…rch_path visibility

Previously if no schema pattern was specified then a default filter was added to getFunctions()
and getProcedures() to restrict the returned set to only those that are visible to the user
via search_path.

This removes that filter so that calling either function with a null or empty schema pattern
will return all functions or procedures.
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.

getProcedures, getFunctions should find all procedures/functions if schema is null
2 participants