#774 Remove FBProcedureCall "new" behaviour which is never used

src/docs/asciidoc/release_notes.adoc

@@ -1105,6 +1105,9 @@ there is no replacement
 ** `findOutParameter(String)` (protected);
 use `getAndAssertSingletonResultSet().findColumn(paramName)`;
 carefully check if that is the correct usage (the method was removed because the old usage within Jaybird resulted in mapping the wrong column)
+* `FBProcedureCall`
+** `mapOutParamIndexToPosition(int, boolean)`;
+use `mapOutParamIndexToPosition(int)` (equivalent to passing `true`), there is no replacement for the `false` behaviour
 
 The following methods had their visibility reduced:
 
@@ -1174,6 +1177,9 @@ there is no replacement
 there is no replacement
 ** `SQL_STATE_COMM_LINK_FAILURE` (`08S01`) -- it was unused;
 there is no replacement
+* `FBProcedureCall`
+** `OLD_CALLABLE_STATEMENT_COMPATIBILITY`;
+there is no replacement
 
 [#removal-of-deprecated-classes-packages-and-methods]
 === Removal of deprecated classes, packages and methods

src/main/org/firebirdsql/jdbc/FBProcedureCall.java

@@ -54,12 +54,6 @@ private static Vector<FBProcedureParam> cloneParameters(final Vector<FBProcedure
         return clonedParameters;
     }
 
-    /**
-     * <code>true</code> if the old callable statement compatibility mode should
-     * be used, otherwise - <code>false</code>. Current value - <code>true</code>.
-     */
-    public static final boolean OLD_CALLABLE_STATEMENT_COMPATIBILITY = true;
-
     private String name;
     // TODO Replace Vector with a List
     private Vector<FBProcedureParam> inputParams = new Vector<>();
@@ -137,58 +131,29 @@ private FBProcedureParam getParam(Collection<FBProcedureParam> params, int index
     }
 
     /**
-     * Map output parameter index to a column number of corresponding result
-     * set.
-     *
-     * @param index index to map.
+     * Map output parameter index to a column number of the corresponding result set.
      *
-     * @return mapped column number or <code>index</code> if no output parameter
-     * with the specified index found (assuming that {@link #OLD_CALLABLE_STATEMENT_COMPATIBILITY}
-     * constant is set to <code>true</code>, otherwise throws exception).
-     *
-     * @throws SQLException if compatibility mode is switched off and no
-     * parameter was found (see {@link #OLD_CALLABLE_STATEMENT_COMPATIBILITY}
-     * constant).
+     * @param index
+     *         index to map
+     * @return mapped column number or {@code index} if no output parameter with the specified index is found
+     * @throws SQLException
+     *         in current implementation: never, throws clause retained for compatibility and possibly future uses
      */
+    @SuppressWarnings("RedundantThrows")
     public int mapOutParamIndexToPosition(int index) throws SQLException {
-        return mapOutParamIndexToPosition(index, OLD_CALLABLE_STATEMENT_COMPATIBILITY);
-    }
-
-    /**
-     * Map output parameter index to a column number of corresponding result
-     * set.
-     *
-     * @param index index to map.
-     * @param compatibilityMode <code>true</code> if we should run in old compatibility mode.
-     *
-     * @return mapped column number or <code>index</code> if no output parameter
-     * with the specified index found and <code>compatibilityMode</code> is set.
-     *
-     * @throws SQLException if compatibility mode is switched off and no
-     * parameter was found.
-     */
-    public int mapOutParamIndexToPosition(int index, boolean compatibilityMode) throws SQLException {
-        int position = -1;
+        int position = 0;
 
         for (FBProcedureParam param : outputParams) {
             if (param != null && param.isParam()) {
                 position++;
 
                 if (param.getIndex() == index) {
-                    return position + 1;
+                    return position;
                 }
             }
         }
-
-        // hack: if we did not find the right parameter we return
-        // an index that was asked if we run in compatibility mode
-        // 
-        // we should switch it off as soon as people convert applications
-        if (compatibilityMode) {
-            return index;
-        } else {
-            throw new SQLException("Specified parameter does not exist", SQL_STATE_INVALID_DESC_FIELD_ID);
-        }
+        // For historic compatibility reasons we return the original requested index if there is no mapping
+        return index;
     }
 
     /**

src/test/org/firebirdsql/jdbc/escape/FBEscapedCallParserTest.java

@@ -52,6 +52,7 @@ class FBEscapedCallParserTest {
     private static final String CALL_TEST_8 = "EXECUTE PROCEDURE my_proc (UPPER(?), '11-dec-2001')";
     private static final String CALL_TEST_9 = "   \t  EXECUTE\nPROCEDURE  my_proc   (    UPPER(?), '11-dec-2001')  \t";
 
+    private final FBEscapedCallParser parser = new FBEscapedCallParser();
     private FBProcedureCall testProcedureCall;
     private final FBProcedureParam param1 = new FBProcedureParam(0, "?");
     private final FBProcedureParam param2 = new FBProcedureParam(1, "UPPER(?)");
@@ -78,17 +79,15 @@ public void setUp() throws SQLException {
 
     @Test
     void testProcessEscapedCall() throws Exception {
-        FBEscapedCallParser parser = new FBEscapedCallParser();
-
         FBProcedureCall procedureCall1 = parser.parseCall(CALL_TEST_5);
         procedureCall1.registerOutParam(1, Types.INTEGER);
         procedureCall1.getInputParam(2).setValue("test value");
         assertThrows(SQLException.class, () -> procedureCall1.registerOutParam(3, Types.CHAR),
                 "Should not allow registering param 3 as output, since it does not exist");
-
-        assertEquals(1, procedureCall1.mapOutParamIndexToPosition(1, false));
-        assertThrows(SQLException.class, () -> procedureCall1.mapOutParamIndexToPosition(2, false),
-                "Should not allow to obtain position when no compatibility mode is specified");
+        // Index 1 corresponds to the first mapped OUT parameter, so it returns 1
+        assertEquals(1, procedureCall1.mapOutParamIndexToPosition(1), "Should return mapped parameter");
+        // Index 2 does not correspond to a mapped OUT parameter, this returns the original value (2)
+        assertEquals(2, procedureCall1.mapOutParamIndexToPosition(2), "Should return unmapped parameter");
 
         FBProcedureCall procedureCall2 = parser.parseCall(CALL_TEST_5_1);
         procedureCall2.registerOutParam(1, Types.INTEGER);
@@ -112,6 +111,20 @@ void testProcessEscapedCall() throws Exception {
         verifyParseSql(procedureCall6);
     }
 
+    @Test
+    void testOutParameterMapping() throws Exception {
+        FBProcedureCall procedureCall = parser.parseCall(CALL_TEST_5);
+        procedureCall.getInputParam(1).setValue("test value");
+        procedureCall.registerOutParam(2, Types.INTEGER);
+        // Index 1 does not correspond to a mapped OUT parameter, this returns the original value (1)
+        assertEquals(1, procedureCall.mapOutParamIndexToPosition(1), "Should return unmapped parameter");
+        // Index 2 corresponds to the first mapped OUT parameter, so it returns 1 as well
+        assertEquals(1, procedureCall.mapOutParamIndexToPosition(2), "Should return mapped parameter");
+        // Index 3 does not correspond to a mapped OUT parameter (though it does correspond to a literal marked as OUT),
+        // this returns the original value (3)
+        assertEquals(3, procedureCall.mapOutParamIndexToPosition(3), "Should return unmapped parameter");
+    }
+
     private void verifyParseSql(FBProcedureCall procedureCall) throws SQLException {
         assertEquals(testProcedureCall.getSQL(false), procedureCall.getSQL(false),
                 String.format("Should correctly parse call.\n[%s] \n[%s]",