Repository: pgjdbc/pgjdbc
Pull request #1645 (2019-12-10 14:36:53)

Test case and documentation for calling stored procedures with transaction control.
Add documentation and unit test for calling stored procedures that use transaction control. Update existing callproc doc to refer to functions and procedures. Add error code returned from server if stored proc with transaction control is called from inside a transaction. 

### All Submissions:

* [Yes] Have you followed the guidelines in our [Contributing](https://github.com/pgjdbc/pgjdbc/blob/master/CONTRIBUTING.md) document? Yes
* [Yes] Have you checked to ensure there aren't other open [Pull Requests](../../pulls) for the same update/change? Yes

1. [yes] Does your submission pass tests?
2. [yes] Does mvn checkstyle:check pass ?
3. [yes] Have you added your new test classes to an existing test suite in alphabetical order?

* [no] Does this break existing behaviour? If so please explain.
* [yes] Have you written new tests for your core changes, as applicable? new test, not core change.
* [yes] Have you successfully run tests with your changes locally?



Patch:
diff --git a/docs/documentation/head/binary-data.md b/docs/documentation/head/binary-data.md
index bbd36f1e..6a4fcd2b 100644
--- a/docs/documentation/head/binary-data.md
+++ b/docs/documentation/head/binary-data.md
@@ -3,7 +3,7 @@ layout: default_docs
 title: Chapter 7. Storing Binary Data
 header: Chapter 7. Storing Binary Data
 resource: media
-previoustitle: Chapter 6. Calling Stored Functions
+previoustitle: Chapter 6. Calling Stored Functions and Procedures
 previous: callproc.html
 nexttitle: Chapter 8. JDBC escapes
 next: escapes.html
diff --git a/docs/documentation/head/callproc.md b/docs/documentation/head/callproc.md
index bf5e14a5..d5eb1950 100644
--- a/docs/documentation/head/callproc.md
+++ b/docs/documentation/head/callproc.md
@@ -1,7 +1,7 @@
 ---
 layout: default_docs
-title: Chapter 6. Calling Stored Functions
-header: Chapter 6. Calling Stored Functions
+title: Chapter 6. Calling Stored Functions and Procedures
+header: Chapter 6. Calling Stored Functions and Procedures
 resource: media
 previoustitle: Creating and Modifying Database Objects
 previous: ddl.html
@@ -11,9 +11,24 @@ next: binary-data.html
 
 **Table of Contents**
 
-* [Obtaining a `ResultSet` from a stored function](callproc.html#callproc-resultset)
-	* [From a Function Returning `SETOF` type](callproc.html#callproc-resultset-setof)
-	* [From a Function Returning a refcursor](callproc.html#callproc-resultset-refcursor)
+* [Obtaining a `ResultSet` from a stored function](callproc.html#callfunc-resultset)
+	* [From a Function Returning `SETOF` type](callproc.html#callfunc-resultset-setof)
+	* [From a Function Returning a refcursor](callproc.html#callfunc-resultset-refcursor)
+* [Calling stored procedure with transaction control](callproc.html#call-procedure-example)
+
+PostgreSQL™ supports two types of stored objects, functions that can return a
+result value and - starting from v11 - procedures that can perform transaction
+control. Both types of stored objects are invoked using `CallableStatement` and
+the standard JDBC escape call syntax `{call storedobject(?)}`. The
+`escapeSyntaxCallMode` connection property controls how the driver transforms the
+call syntax to invoke functions or procedures.
+
+The default mode, `select`, supports backwards compatibility for existing
+applications and supports function invocation only. This is required to invoke
+a void returning function. For new applications, use
+`escapeSyntaxCallMode=callIfNoReturn` to map `CallableStatement`s with return
+values to stored functions and `CallableStatement`s without return values to
+stored procedures.
 
 <a name="call-function-example"></a>
 **Example 6.1. Calling a built in stored function**
@@ -22,15 +37,15 @@ This example shows how to call a PostgreSQL™ built in function, `upper`, which
 simply converts the supplied string argument to uppercase.
 
 ```java
-CallableStatement upperProc = conn.prepareCall("{? = call upper( ? ) }");
-upperProc.registerOutParameter(1, Types.VARCHAR);
-upperProc.setString(2, "lowercase to uppercase");
-upperProc.execute();
-String upperCased = upperProc.getString(1);
-upperProc.close();
+CallableStatement upperFunc = conn.prepareCall("{? = call upper( ? ) }");
+upperFunc.registerOutParameter(1, Types.VARCHAR);
+upperFunc.setString(2, "lowercase to uppercase");
+upperFunc.execute();
+String upperCased = upperFunc.getString(1);
+upperFunc.close();
 ```
 
-<a name="callproc-resultset"></a>
+<a name="callfunc-resultset"></a>
 # Obtaining a `ResultSet` from a stored function
 
 PostgreSQL's™ stored functions can return results in two different ways. The
@@ -38,7 +53,7 @@ function may return either a refcursor value or a `SETOF` some datatype.  Depend
 on which of these return methods are used determines how the function should be
 called.
 
-<a name="callproc-resultset-setof"></a>
+<a name="callfunc-resultset-setof"></a>
 ## From a Function Returning `SETOF` type
 
 Functions that return data as a set should not be called via the `CallableStatement`
@@ -61,7 +76,7 @@ rs.close();
 stmt.close();
 ```
 
-<a name="callproc-resultset-refcursor"></a>
+<a name="callfunc-resultset-refcursor"></a>
 ## From a Function Returning a refcursor
 
 When calling a function that returns a refcursor you must cast the return type of
@@ -94,17 +109,17 @@ stmt.close();
 // We must be inside a transaction for cursors to work.
 conn.setAutoCommit(false);
 
-// Procedure call.
-CallableStatement proc = conn.prepareCall("{? = call refcursorfunc() }");
-proc.registerOutParameter(1, Types.OTHER);
-proc.execute();
-ResultSet results = (ResultSet) proc.getObject(1);
+// Function call.
+CallableStatement func = conn.prepareCall("{? = call refcursorfunc() }");
+func.registerOutParameter(1, Types.OTHER);
+func.execute();
+ResultSet results = (ResultSet) func.getObject(1);
 while (results.next())
 {
     // do something with the results.
 }
 results.close();
-proc.close();
+func.close();
 ```
 
 It is also possible to treat the refcursor return value as a cursor name directly.
@@ -116,9 +131,43 @@ you are free to directly use cursor commands on it, such as `FETCH` and `MOVE`.
 
 ```java
 conn.setAutoCommit(false);
-CallableStatement proc = conn.prepareCall("{? = call refcursorfunc() }");
-proc.registerOutParameter(1, Types.OTHER);
+CallableStatement func = conn.prepareCall("{? = call refcursorfunc() }");
+func.registerOutParameter(1, Types.OTHER);
+func.execute();
+String cursorName = func.getString(1);
+func.close();
+```
+
+<a name="call-procedure-example"></a>
+**Example 6.5. Calling a stored procedure
+
+This example shows how to call a PostgreSQL™ procedure that uses transaction control.
+
+```java
+// set up a connection
+String url = "jdbc:postgresql://localhost/test";
+Properties props = new Properties();
+... other properties ...
+// Ensure EscapeSyntaxCallmode property set to support procedures if no return value
+props.setProperty("escapeSyntaxCallMode", "callIfNoReturn");
+Connection con = DriverManager.getConnection(url, props);
+
+// Setup procedure to call.
+Statement stmt = con.createStatement();
+stmt.execute("CREATE TEMP TABLE temp_val ( some_val bigint )");
+stmt.execute("CREATE OR REPLACE PROCEDURE commitproc(a INOUT bigint) AS '"
+    + " BEGIN "
+    + "    INSERT INTO temp_val values(a); "
+    + "    COMMIT; "
+    + " END;' LANGUAGE plpgsql");
+stmt.close();
+
+// As of v11, we must be outside a transaction for procedures with transactions to work.
+con.setAutoCommit(true);
+
+// Procedure call with transaction
+CallableStatement proc = con.prepareCall("{call commitproc( ? )}");
+proc.setInt(1, 100);
 proc.execute();
-String cursorName = proc.getString(1);
 proc.close();
 ```
diff --git a/docs/documentation/head/index.html b/docs/documentation/head/index.html
index 70ef62fd..dce2deae 100644
--- a/docs/documentation/head/index.html
+++ b/docs/documentation/head/index.html
@@ -86,16 +86,17 @@
 								</dl>
 							</dd>
 
-							<dt>6. <a href="callproc.html">Calling Stored Functions</a></dt>
+							<dt>6. <a href="callproc.html">Calling Stored Functions and Procedures</a></dt>
 							<dd>
 								<dl>
-									<dt><a href="callproc.html#callproc-resultset">Obtaining a ResultSet from a stored function</a></dt>
+									<dt><a href="callproc.html#callfunc-resultset">Obtaining a ResultSet from a stored function</a></dt>
 										<dd>
 											<dl>
-												<dt><a href="callproc.html#callproc-resultset-setof">From a Function Returning SETOF type</a></dt>
-												<dt><a href="callproc.html#callproc-resultset-refcursor">From a Function Returning a <span class="type">refcursor</span></a></dt>
+												<dt><a href="callproc.html#callfunc-resultset-setof">From a Function Returning SETOF type</a></dt>
+												<dt><a href="callproc.html#callfunc-resultset-refcursor">From a Function Returning a <span class="type">refcursor</span></a></dt>
 											</dl>
 										</dd>
+									<dt><a href="callproc.html#call-procedure-example">Calling stored procedure with transaction control</a></dt>
 								</dl>
 							</dd>
 
diff --git a/pgjdbc/src/main/java/org/postgresql/util/PSQLState.java b/pgjdbc/src/main/java/org/postgresql/util/PSQLState.java
index ca4de8ba..21ff5c85 100644
--- a/pgjdbc/src/main/java/org/postgresql/util/PSQLState.java
+++ b/pgjdbc/src/main/java/org/postgresql/util/PSQLState.java
@@ -68,6 +68,8 @@ public enum PSQLState {
   INVALID_SQL_STATEMENT_NAME("26000"),
   INVALID_AUTHORIZATION_SPECIFICATION("28000"),
 
+  INVALID_TRANSACTION_TERMINATION("2D000"),
+
   STATEMENT_NOT_ALLOWED_IN_FUNCTION_CALL("2F003"),
 
   INVALID_SAVEPOINT_SPECIFICATION("3B000"),


Test patch:
diff --git a/pgjdbc/src/test/java/org/postgresql/jdbc/NoColumnMetadataIssue1613.java b/pgjdbc/src/test/java/org/postgresql/jdbc/NoColumnMetadataIssue1613Test.java
similarity index 87%
rename from pgjdbc/src/test/java/org/postgresql/jdbc/NoColumnMetadataIssue1613.java
rename to pgjdbc/src/test/java/org/postgresql/jdbc/NoColumnMetadataIssue1613Test.java
index 750623ac..65d3eb98 100644
--- a/pgjdbc/src/test/java/org/postgresql/jdbc/NoColumnMetadataIssue1613.java
+++ b/pgjdbc/src/test/java/org/postgresql/jdbc/NoColumnMetadataIssue1613Test.java
@@ -23,7 +23,7 @@ import java.sql.Statement;
  * @author Ivy (ivyyiyideng@gmail.com)
  *
  */
-public class NoColumnMetadataIssue1613 extends BaseTest4 {
+public class NoColumnMetadataIssue1613Test extends BaseTest4 {
   @Override
   @Before
   public void setUp() throws Exception {
@@ -34,7 +34,7 @@ public class NoColumnMetadataIssue1613 extends BaseTest4 {
   @Test
   public void shouldBeNoNPE() throws Exception {
     Statement statement = con.createStatement();
-    statement.executeQuery("INSERT INTO test_no_column_metadata values (1)");
+    statement.execute("INSERT INTO test_no_column_metadata values (1)");
     ResultSet rs = statement.executeQuery("SELECT x FROM test_no_column_metadata x");
     assertTrue(rs.next());
   }
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/Jdbc2TestSuite.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/Jdbc2TestSuite.java
index 004f1b61..4e01958d 100644
--- a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/Jdbc2TestSuite.java
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/Jdbc2TestSuite.java
@@ -13,6 +13,7 @@ import org.postgresql.core.ParserTest;
 import org.postgresql.core.ReturningParserTest;
 import org.postgresql.core.v3.V3ParameterListTests;
 import org.postgresql.jdbc.DeepBatchedInsertStatementTest;
+import org.postgresql.jdbc.NoColumnMetadataIssue1613Test;
 import org.postgresql.jdbc.PgSQLXMLTest;
 import org.postgresql.jdbc.PrimitiveArraySupportTest;
 import org.postgresql.test.core.JavaVersionTest;
@@ -73,6 +74,7 @@ import org.junit.runners.Suite;
     LruCacheTest.class,
     MiscTest.class,
     NativeQueryBindLengthTest.class,
+    NoColumnMetadataIssue1613Test.class,
     NotifyTest.class,
     OidToStringTest.class,
     OidValueOfTest.class,
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc3/Jdbc3TestSuite.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc3/Jdbc3TestSuite.java
index 2693cf7f..a059d5fa 100644
--- a/pgjdbc/src/test/java/org/postgresql/test/jdbc3/Jdbc3TestSuite.java
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc3/Jdbc3TestSuite.java
@@ -24,6 +24,7 @@ import org.junit.runners.Suite;
     Jdbc3CallableStatementTest.class,
     Jdbc3SavepointTest.class,
     ParameterMetaDataTest.class,
+    ProcedureTransactionTest.class,
     ResultSetTest.class,
     SendRecvBufferSizeTest.class,
     SqlCommandParseTest.class,
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc3/ProcedureTransactionTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc3/ProcedureTransactionTest.java
new file mode 100644
index 00000000..e1262df9
--- /dev/null
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc3/ProcedureTransactionTest.java
@@ -0,0 +1,169 @@
+/*
+ * Copyright (c) 2019, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.test.jdbc3;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import org.postgresql.PGProperty;
+import org.postgresql.core.ServerVersion;
+import org.postgresql.jdbc.EscapeSyntaxCallMode;
+import org.postgresql.test.TestUtil;
+import org.postgresql.test.jdbc2.BaseTest4;
+import org.postgresql.util.PSQLState;
+
+import org.junit.Test;
+
+import java.sql.CallableStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Properties;
+
+public class ProcedureTransactionTest extends BaseTest4 {
+
+  @Override
+  protected void updateProperties(Properties props) {
+    super.updateProperties(props);
+    PGProperty.ESCAPE_SYNTAX_CALL_MODE.set(props, EscapeSyntaxCallMode.CALL_IF_NO_RETURN.value());
+  }
+
+  @Override
+  public void setUp() throws Exception {
+    super.setUp();
+    Statement stmt = con.createStatement();
+    if (TestUtil.haveMinimumServerVersion(con, ServerVersion.v11)) {
+      stmt.execute("create temp table proc_test ( some_val bigint )");
+      stmt.execute(
+          "CREATE OR REPLACE PROCEDURE mycommitproc(a INOUT bigint) AS 'BEGIN INSERT INTO proc_test values(a); commit; END;' LANGUAGE plpgsql");
+      stmt.execute(
+          "CREATE OR REPLACE PROCEDURE myrollbackproc(a INOUT bigint) AS 'BEGIN INSERT INTO proc_test values(a); rollback; END;' LANGUAGE plpgsql");
+      stmt.execute(
+          "CREATE OR REPLACE PROCEDURE mynotxnproc(a INOUT bigint) AS 'BEGIN INSERT INTO proc_test values(a); END;' LANGUAGE plpgsql");
+    }
+  }
+
+  @Override
+  public void tearDown() throws SQLException {
+    Statement stmt = con.createStatement();
+    if (TestUtil.haveMinimumServerVersion(con, ServerVersion.v11)) {
+      stmt.execute("drop procedure mycommitproc(a INOUT bigint) ");
+      stmt.execute("drop procedure myrollbackproc(a INOUT bigint) ");
+      stmt.execute("drop procedure mynotxnproc(a INOUT bigint) ");
+      stmt.execute("drop table proc_test ");
+    }
+    stmt.close();
+    super.tearDown();
+  }
+
+  @Test
+  public void testProcWithNoTxnControl() throws SQLException {
+    assumeMinimumServerVersion(ServerVersion.v11);
+    assumeCallableStatementsSupported();
+    CallableStatement cs = con.prepareCall("call mynotxnproc(?)");
+    int val = 1;
+    cs.setInt(1, val);
+    cs.execute();
+    TestUtil.closeQuietly(cs);
+
+    cs = con.prepareCall("select some_val from proc_test where some_val = ?");
+    cs.setInt(1, val);
+    ResultSet rs = cs.executeQuery();
+
+    assertTrue(rs.next());
+    assertTrue(rs.getInt(1) == val);
+
+    TestUtil.closeQuietly(rs);
+    TestUtil.closeQuietly(cs);
+  }
+
+  @Test
+  public void testProcWithCommitInside() throws SQLException {
+    assumeMinimumServerVersion(ServerVersion.v11);
+    assumeCallableStatementsSupported();
+    CallableStatement cs = con.prepareCall("call mycommitproc(?)");
+    int val = 2;
+    cs.setInt(1, val);
+    cs.execute();
+    TestUtil.closeQuietly(cs);
+
+    cs = con.prepareCall("select some_val from proc_test where some_val = ?");
+    cs.setInt(1, val);
+    ResultSet rs = cs.executeQuery();
+
+    assertTrue(rs.next());
+    assertTrue(rs.getInt(1) == val);
+
+    TestUtil.closeQuietly(rs);
+    TestUtil.closeQuietly(cs);
+  }
+
+  @Test
+  public void testProcWithRollbackInside() throws SQLException {
+    assumeMinimumServerVersion(ServerVersion.v11);
+    assumeCallableStatementsSupported();
+    CallableStatement cs = con.prepareCall("call myrollbackproc(?)");
+    int val = 3;
+    cs.setInt(1, val);
+    cs.execute();
+    TestUtil.closeQuietly(cs);
+
+    cs = con.prepareCall("select some_val from proc_test where some_val = ?");
+    cs.setInt(1, val);
+    ResultSet rs = cs.executeQuery();
+
+    assertFalse(rs.next());
+
+    TestUtil.closeQuietly(rs);
+    TestUtil.closeQuietly(cs);
+  }
+
+  @Test
+  public void testProcAutoCommitTrue() throws SQLException {
+    con.setAutoCommit(true);
+    testProcAutoCommit();
+  }
+
+  @Test
+  public void testProcAutoCommitFalse() throws SQLException {
+    // setting autocommit false enables application transaction control, meaning JDBC driver issues a BEGIN
+    // as of PostgreSQL 11, Stored Procedures with transaction control inside the procedure cannot be
+    // invoked inside a transaction, the procedure must start the top level transaction
+    // see: https://www.postgresql.org/docs/current/plpgsql-transactions.html
+    con.setAutoCommit(false);
+    try {
+      testProcAutoCommit();
+      fail("Should throw an exception");
+    } catch (SQLException ex) {
+      //2D000 invalid_transaction_termination
+      assertTrue(ex.getSQLState().equalsIgnoreCase(PSQLState.INVALID_TRANSACTION_TERMINATION.getState()));
+      con.rollback();
+    }
+
+  }
+
+  private void testProcAutoCommit() throws SQLException {
+    assumeMinimumServerVersion(ServerVersion.v11);
+    assumeCallableStatementsSupported();
+    CallableStatement cs = con.prepareCall("call mycommitproc(?)");
+    int val = 4;
+    cs.setInt(1, val);
+    cs.execute();
+    TestUtil.closeQuietly(cs);
+
+    cs = con.prepareCall("select some_val from proc_test where some_val = ?");
+    cs.setInt(1, val);
+    ResultSet rs = cs.executeQuery();
+
+    assertTrue(rs.next());
+    assertTrue(rs.getInt(1) == val);
+
+    TestUtil.closeQuietly(rs);
+    TestUtil.closeQuietly(cs);
+  }
+
+}