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

SQLServer: no exception thrown when statement fails #718

Closed
rekonvald opened this issue Mar 19, 2014 · 7 comments
Closed

SQLServer: no exception thrown when statement fails #718

rekonvald opened this issue Mar 19, 2014 · 7 comments

Comments

@rekonvald
Copy link

@rekonvald rekonvald commented Mar 19, 2014

We faced with such problem. When one of patch commands fails to execute properly(not first command), flyway does not throw any exceptions, and marks this patch as executed in schema_version table.
It can be reproduced in such way: just create patch with statements(should fail on ) to flyway-samle project and run it.
INSERT INTO test_user (name1) VALUES ('test');
INSERT INTO test_user (name1) VALUES ('test');
Patch should fail but it doesn't.
We investigated this problem and found reason of bug. On executing statement, JdbcTemplate uses execute method. But it would be better to use http://docs.oracle.com/javase/7/docs/api/java/sql/Statement.html#executeBatch%28%29

rekonvald added a commit to rekonvald/flyway that referenced this issue Mar 20, 2014
…s to execute properly flyway dosn't throw any exception as if patch was executed successfully
@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented Mar 23, 2014

Thanks for the report? Which database are you using? Why would executeBatch() fix this?

@rekonvald
Copy link
Author

@rekonvald rekonvald commented Mar 24, 2014

I use MSSQL.We also wondered why query passes without exceptions even if script is incorrect. The problem occured when we tried to create foreign key with wrong data type. Flyway ran patch, foreign key was not added to the table, but patch was marked as executed in schema_version table. Then I tried to use execute batch, because according to the spec it 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.
Further research showed that execute does not throw exception if first query in the patch ran successfully, as far as I see it happens if foreign key problem occurs.
Did you try to reproduce this bug adding new patch to flyway-samle project?

@rekonvald
Copy link
Author

@rekonvald rekonvald commented Mar 24, 2014

It happens only with MSSQL, for hsqldb and mysql it is ok.

@rekonvald
Copy link
Author

@rekonvald rekonvald commented Mar 25, 2014

Could you please advice, what should be done to make it work in MSSQL? If we do a local changes, we will not have possibility to update flyway, because updated version will still contain this bug....

@axelfontaine axelfontaine added this to the Flyway 3.1 milestone Apr 15, 2014
@cc-rock
Copy link
Contributor

@cc-rock cc-rock commented Jun 13, 2014

I am facing exactly the same problem, using FlyWay 3.0 with SQL Azure (microsoft JDBC driver).
It seems that if you execute multiple sql commands in the same jdbc statement (which happens if you don't separate each sql command with GO), and an error occurs in some DML command (for example INSERT) that is not the first one in the batch, then the MS jdbc driver does not throw any exception if you don't call getMoreResults() on the jdbc statement, until the result of the failing command is retrieved.
Hope this helps.

@cc-rock
Copy link
Contributor

@cc-rock cc-rock commented Jun 18, 2014

I wrote a fix for this issue and sent a pull request:
#780

@axelfontaine axelfontaine changed the title If one of the commands sent to the database fails to execute properly flyway dosn't throw any exception as if patch was executed successfully SQLServer: no exception thrown when statement fails Aug 3, 2014
@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented Aug 3, 2014

PR Merged. Thanks Carlo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants