Wrong value returned from Statement::getUpdateCount, after Statement::getMoreResults call

[lukaseder]

Try this code:

try (Statement s = connection.createStatement()) {
    System.out.println(s.execute("select 1 from RDB$DATABASE"));
    System.out.println(s.getUpdateCount());

    try (ResultSet rs = s.getResultSet()) {
        while (rs.next())
            System.out.println(rs.getInt(1));
    }

    System.out.println(s.getMoreResults());
    System.out.println(s.getUpdateCount());
}

It produces this output:

true
-1
1
false
0     <-- problem here

The last value should be -1, not 0

From the getMoreResults() Javadoc:

Moves to this Statement object's next result, returns true if it is a ResultSet object, andimplicitly closes any current ResultSetobject(s) obtained with the method getResultSet.

There are no more results when the following is true:
// stmt is a Statement object
((stmt.getMoreResults() == false) && (stmt.getUpdateCount() == -1))

From the getUpdateCount() Javadoc:

Returns: the current result as an update count; -1 if the current result is a ResultSet object or there are no more results

Debugging this a bit further, there seems to be an intermediary update count of 0 that is being returned:

try (Statement s = connection.createStatement()) {
    System.out.println(s.execute("select 1 from RDB$DATABASE"));
    System.out.println(s.getUpdateCount());

    try (ResultSet rs = s.getResultSet()) {
        while (rs.next())
            System.out.println(rs.getInt(1));
    }

    System.out.println(s.getMoreResults());
    System.out.println(s.getUpdateCount());
    System.out.println(s.getMoreResults());
    System.out.println(s.getUpdateCount());
}

This now produces:

true
-1
1
false
0
false
-1

Given the query, I don't see a good reason for this update count to be returned.

[mrotteveel]

Firebird does not have update counts as a separate result, so Jaybird must explicitly request an update count from the server at the appropriate time using a "statement information request". It is not something that the server automatically pushes to the client, and there is no way to detect that a statement does not produce an update count (i.e. did not execute any modifying DML). In Firebird, even selects can produce an update count (e.g. if it directly or indirectly references a selectable stored procedure with modifying DML).

So, to be able to return update counts at all, Jaybird uses a simple state-machine for the current statement execution, StatementResult. For a result set producing statement, the transition is RESULT_SET -> UPDATE_COUNT -> NO_MORE_RESULTS, so there is always an update count result, even if it is zero, and the statement in question would never modify anything.

In other words, this is a trade off between providing too little information (no update counts when there are), or too much information (providing a 0 update count, when the statement wouldn't do any modification).

[mrotteveel]

I do see that for DDL, the first result should probably be NO_MORE_RESULTS, and not UPDATE_COUNT like it does right now.

[lukaseder]

Interesting. I did think there must be a reason that isn't strictly a bug. Would you be interested in documenting this also on stack overflow? I could repeat my question on there, to make your useful answer more widely available.

[mrotteveel]

I was verifying my assumptions, and I found out that my belief that selectable stored procedures with modifying DML have a non-zero update count when requested is actually incorrect.

In other words, it is actually a bug. There is one complication, Firebird 5 will identify DML with a RETURNING clause as a select-statement (for backwards compatibility) to allow it returning multiple rows (as opposed to the single row it can produce in Firebird 4 and older), which means that to fix this, Jaybird itself will need to perform identification of the actual statement type by parsing the statement.

[mrotteveel]

Other wrinkle, an execute block producing a result set does have an update count

[lukaseder]

Excellent, thanks a lot for the analysis!

[mrotteveel]

I'm not going to fix this in Jaybird 4, as that version doesn't yet use the new parser introduced with #680, and backporting it, or using the old parser is too much work. I will do this for Jaybird 5.

[mrotteveel]

I'm also going to exclude other statements that don't produce an update count. I'm wondering what to do with -1 results for execute batch: keep them as is, or transform them to Statement.SUCCESS_NO_INFO (-2). What do you think @lukaseder?

[mrotteveel]

Forgot one option for executeBatch: transform them back to 0, just like needs to be done for executeUpdate.

[mrotteveel]

I forgot about server-side batch behaviour, there using EXECUTE PROCEDURE in a batch will result in an update count of 0, so for consistency, I'll also do that in emulated behaviour.

[lukaseder]

I'm also going to exclude other statements that don't produce an update count. I'm wondering what to do with -1 results for execute batch: keep them as is, or transform them to Statement.SUCCESS_NO_INFO (-2). What do you think @lukaseder?

Can you show an example?

[mrotteveel]

I have basically already decided to continue reporting 0 for these cases, as that matches the behaviour for the update counts generated by the server-side batch support of Firebird 4.0 and higher.

A very simple example (one for PreparedStatement, and the same for CallableStatement:

jaybird/src/test/org/firebirdsql/jdbc/BatchUpdatesTest.java

Lines 278 to 324 in 6c71bac

	@ParameterizedTest(name = "[{index}] useServerBatch = {0}")
	@ValueSource(booleans = { true, false })
	void testExecuteProcedureInPreparedStatementBatch(boolean useServerBatch) throws Exception {
	if (useServerBatch) {
	assumeServerBatchSupport();
	}
	try (Connection connection = createConnection(useServerBatch)) {
	connection.setAutoCommit(false);
	try (PreparedStatement pstmt = connection.prepareStatement("{call dummy_procedure(?)}")) {
	int rows = 5;
	IntStream.rangeClosed(1, 5).forEach(i -> {
	try {
	pstmt.setInt(1, i);
	pstmt.addBatch();
	} catch (SQLException e) {
	fail(e);
	}
	});
	int[] updateCounts = pstmt.executeBatch();
	int[] expectedUpdateCounts = new int[rows];
	assertArrayEquals(expectedUpdateCounts, updateCounts);
	}
	}
	}
	@Test
	void testExecuteProcedureInCallableStatementBatch() throws Exception {
	// NOTE: server batch not supported for callable statement
	try (Connection connection = getConnectionViaDriverManager()) {
	try (CallableStatement pstmt = connection.prepareCall("{call dummy_procedure(?)}")) {
	int rows = 5;
	IntStream.rangeClosed(1, 5).forEach(i -> {
	try {
	pstmt.setInt(1, i);
	pstmt.addBatch();
	} catch (SQLException e) {
	fail(e);
	}
	});
	int[] updateCounts = pstmt.executeBatch();
	int[] expectedUpdateCounts = new int[rows];
	assertArrayEquals(expectedUpdateCounts, updateCounts);
	}
	}
	}

Given this executes a stored procedure, the client-side emulated batch would - without intervention - produce update counts { -1, -1, -1, -1, -1 }, while the server-side batch (PreparedStatement only) would produce { 0, 0, 0, 0, 0 }.