Skip to content

Commit

Permalink
fix: getProcedures returns only procedures (not functions) for psql11+ (
Browse files Browse the repository at this point in the history
#1723)

* fix: getProcedures returns only procedures (not functions) for psql11+

Currently the PGDatabaseMetaData.getProcedures() method returns both
procedures and functions. This made sense prior to PostgreSQL11, but now
actual procedures can be created instead of just functions, and pgjdbc
has an added getFunctions() method to distinguish between the two. For
only PostgreSQL version 11 and onward, this will make getFunctions()
return only functions and getProcedures() return only procedures.

I believe this closes #1340 and #1629, which both seem to essentially be
the same thing?

* only create procedure in tests for psql versions 11 and higher

* testGetProcedures now checks for functions or procedures depending on psql version

* only test 'create procedure' for psql 11+
  • Loading branch information
MSGoodman committed Feb 27, 2020
1 parent 0faf9ce commit 5fbe046
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 18 deletions.
Expand Up @@ -1037,6 +1037,10 @@ public ResultSet getProcedures(String catalog, String schemaPattern, String proc
+ " LEFT JOIN pg_catalog.pg_class c ON (d.classoid=c.oid AND c.relname='pg_proc') "
+ " LEFT JOIN pg_catalog.pg_namespace pn ON (c.relnamespace=pn.oid AND pn.nspname='pg_catalog') "
+ " WHERE p.pronamespace=n.oid ";

if (connection.haveMinimumServerVersion(ServerVersion.v11)) {
sql += " AND p.prokind='p'";
}
if (schemaPattern != null && !schemaPattern.isEmpty()) {
sql += " AND n.nspname LIKE " + escapeQuotes(schemaPattern);
} else {
Expand Down
Expand Up @@ -459,42 +459,45 @@ List<String> getFunctionNames(DatabaseMetaData databaseMetaData, String schemaPa

@Test
public void testGetProcedures() throws SQLException {
String executeGranted = TestUtil.haveMinimumServerVersion(hidingCon, ServerVersion.v11) ? "execute_granted_insert_procedure" : "execute_granted_add_function";
String noGrants = TestUtil.haveMinimumServerVersion(hidingCon, ServerVersion.v11) ? "no_grants_insert_procedure" : "no_grants_add_function";

List<String> proceduresWithHiding =
getProcedureNames(hidingDatabaseMetaData, "high_privileges_schema");
assertThat(proceduresWithHiding,
hasItem("execute_granted_add_function"));
hasItem(executeGranted));
assertThat(proceduresWithHiding,
not(hasItem("no_grants_add_function")));
not(hasItem(noGrants)));

List<String> proceduresWithNoHiding =
getProcedureNames(nonhidingDatabaseMetaData, "high_privileges_schema");
assertThat(proceduresWithNoHiding,
hasItems("execute_granted_add_function", "no_grants_add_function"));
hasItems(executeGranted, noGrants));

proceduresWithHiding =
getProcedureNames(hidingDatabaseMetaData, "low_privileges_schema");
assertThat(proceduresWithHiding,
hasItem("execute_granted_add_function"));
hasItem(executeGranted));
assertThat(proceduresWithHiding,
not(hasItem("no_grants_add_function")));
not(hasItem(noGrants)));

proceduresWithNoHiding =
getProcedureNames(nonhidingDatabaseMetaData, "low_privileges_schema");
assertThat(proceduresWithNoHiding,
hasItems("execute_granted_add_function", "no_grants_add_function"));
hasItems(executeGranted, noGrants));

// Or should the the function names not be returned because the schema is not visible?
proceduresWithHiding =
getProcedureNames(hidingDatabaseMetaData, "no_privileges_schema");
assertThat(proceduresWithHiding,
hasItem("execute_granted_add_function"));
hasItem(executeGranted));
assertThat(proceduresWithHiding,
not(hasItem("no_grants_add_function")));
not(hasItem(noGrants)));

proceduresWithNoHiding =
getProcedureNames(nonhidingDatabaseMetaData, "no_privileges_schema");
assertThat(proceduresWithNoHiding,
hasItems("execute_granted_add_function", "no_grants_add_function"));
hasItems(executeGranted, noGrants));

}

Expand Down
Expand Up @@ -40,8 +40,14 @@ public void setUp() throws Exception {
TestUtil.createTable(conn, "sercoltest", "a serial, b int");
TestUtil.createSchema(conn, "hasfunctions");
TestUtil.createSchema(conn, "nofunctions");
TestUtil.createSchema(conn, "hasprocedures");
TestUtil.createSchema(conn, "noprocedures");
TestUtil.execute("create function hasfunctions.addfunction (integer, integer) "
+ "RETURNS integer AS 'select $1 + $2;' LANGUAGE SQL IMMUTABLE", conn);
if (TestUtil.haveMinimumServerVersion(conn, ServerVersion.v11)) {
TestUtil.execute("create procedure hasprocedures.addprocedure() "
+ "LANGUAGE plpgsql AS $$ BEGIN SELECT 1; END; $$", conn);
}
}

@After
Expand All @@ -50,6 +56,8 @@ public void tearDown() throws Exception {
TestUtil.dropTable(conn, "sercoltest");
TestUtil.dropSchema(conn, "hasfunctions");
TestUtil.dropSchema(conn, "nofunctions");
TestUtil.dropSchema(conn, "hasprocedures");
TestUtil.dropSchema(conn, "noprocedures");
TestUtil.closeDB(conn);
}

Expand Down Expand Up @@ -122,26 +130,68 @@ public void testGetFunctionsInSchema() throws SQLException {
}

@Test
public void testGetProceduresInSchema() throws SQLException {
public void testGetProceduresInSchemaForFunctions() throws SQLException {
// Due to the introduction of actual stored procedures in PostgreSQL 11, getProcedures should not return functions for PostgreSQL versions 11+

DatabaseMetaData dbmd = conn.getMetaData();
Statement statement = conn.createStatement();

// Search for procedures in schema "hasfunctions" (which should expect a record only for PostgreSQL < 11)
ResultSet rs = dbmd.getProcedures("", "hasfunctions",null);
assertTrue(rs.next());
Boolean recordFound = rs.next();
if (TestUtil.haveMinimumServerVersion(conn, ServerVersion.v11)) {
assertEquals("PostgreSQL11+ should not return functions from getProcedures", recordFound, false);
} else {
assertEquals("PostgreSQL prior to 11 should return functions from getProcedures", recordFound, true);
}

Statement statement = conn.createStatement();
statement.execute("set search_path=hasfunctions");
// Search for procedures in schema "nofunctions" (which should never expect records)
rs = dbmd.getProcedures("", "nofunctions",null);
recordFound = rs.next();
assertFalse(recordFound);

// Search for procedures by function name "addfunction" within schema "hasfunctions" (which should expect a record for PostgreSQL < 11)
statement.execute("set search_path=hasfunctions");
rs = dbmd.getProcedures("", "","addfunction");
assertTrue(rs.next());
recordFound = rs.next();
if (TestUtil.haveMinimumServerVersion(conn, ServerVersion.v11)) {
assertEquals("PostgreSQL11+ should not return functions from getProcedures", recordFound, false);
} else {
assertEquals("PostgreSQL prior to 11 should return functions from getProcedures", recordFound, true);
}

// Search for procedures by function name "addfunction" within schema "nofunctions" (which should never expect records)
statement.execute("set search_path=nofunctions");
rs = dbmd.getProcedures("", "","addfunction");
assertFalse(rs.next());
recordFound = rs.next();
assertFalse(recordFound);

statement.execute("reset search_path");
statement.close();
rs = dbmd.getProcedures("", "nofunctions",null);
assertFalse(rs.next());
}

@Test
public void testGetProceduresInSchemaForProcedures() throws SQLException {
// Only run this test for PostgreSQL version 11+; assertions for versions prior would be vacuously true as we don't create a procedure in the setup for older versions
if (TestUtil.haveMinimumServerVersion(conn, ServerVersion.v11)) {
DatabaseMetaData dbmd = conn.getMetaData();
Statement statement = conn.createStatement();

ResultSet rs = dbmd.getProcedures("", "hasprocedures", null);
assertTrue(rs.next());

rs = dbmd.getProcedures("", "nofunctions", null);
assertFalse(rs.next());

statement.execute("set search_path=hasprocedures");
rs = dbmd.getProcedures("", "", "addprocedure");
assertTrue(rs.next());

statement.execute("set search_path=noprocedures");
rs = dbmd.getProcedures("", "", "addprocedure");
assertFalse(rs.next());

statement.close();
}
}

@Test
Expand Down

0 comments on commit 5fbe046

Please sign in to comment.