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

BUG: Batch executions silently accept SELECT statements #970

Closed
jorsol opened this issue Sep 29, 2017 · 3 comments
Closed

BUG: Batch executions silently accept SELECT statements #970

jorsol opened this issue Sep 29, 2017 · 3 comments

Comments

@jorsol
Copy link
Member

jorsol commented Sep 29, 2017

Working on #935, I have just found a bug in batch execution, the driver silently accept SELECT statements in executeBatch(), but the specification clearly reads:

The method executeBatch throws a BatchUpdateException if any of the commands in
the batch fail to execute properly or if a command attempts to return a result set.

The JavaDoc for executeBatch:

Throws BatchUpdateException (a subclass of SQLException) if one of the commands sent to the database fails to execute properly or attempts to return a result set.

And the Java Tutorials:

You should not add a query (a SELECT statement) to a batch of SQL commands because the method executeBatch, which returns an array of update counts, expects an update count from each SQL statement that executes successfully. This means that only commands that return an update count (commands such as INSERT INTO, UPDATE, DELETE) or that return 0 (such as CREATE TABLE, DROP TABLE, ALTER TABLE) can be successfully executed as a batch with the executeBatch method.

The bug was introduced in #503, while it should not be a big deal, the specification is very clear about this, and must be corrected.

@vlsi
Copy link
Member

vlsi commented Oct 5, 2017

Do you mean this change?
https://github.com/pgjdbc/pgjdbc/pull/503/files#diff-90f32da311526dc658f812dba0399924R54

Feel free to submit a PR that reverts that line -- we'll se which tests would be affected.

PS. The documentation is weird since "statements in 'return generated keys' mode are allowed to return results".

@jorsol
Copy link
Member Author

jorsol commented Oct 6, 2017

Yes, that should do the trick...

@davecramer
Copy link
Member

seems like this doesn't have any traction. @jorsol let me know if you think it should be reopened

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

No branches or pull requests

3 participants