When script contains more than one statement with a failing one

[djeanprost]

Hello,

I met a problem with a script that contains 2 statements : a correct create table script and an incorrect insert script.
The create table is correct, but the insert script couldn't work because of data cast impossible to be executed.

As there is an error executing the script, none of the two statement where applied against the database, but mybatis reporded this script with success.

I'm using SQL Server with the latest jdbc driver.
The only workaround I found is to set send_full_script to false, so that each statement is sent on its own, and in this case, mybatis reports correctly as failure because of the 2nd incorrect statement.

I can't explain the way mybatis works in this case.

[h3adache]

A cast as in CAST(xx as yy)?

[djeanprost]

the script is like this. The problem comes with the 2nd statement where the 'NULL' value should have been NULL instead.
When executing this script, mybatis sees it as success although there was a problem during execution preventing to apply the 2nd statement because of the data type problem.
If I change the default value of send_full_script from true to false, the behaviour is finally correct : no script is passed and mybatis reports as failure because of the 2nd one.

-- Migration SQL that makes the change goes here.

create table dbo.Mytable (
   code                 char(14)  not null,
   dateEffet            datetime             not null,
   dateFinEffet         datetime             null,
   constraint C_PK_Mytable primary key  (code)
);

	
INSERT INTO dbo.Mytable (code, dateEffet, dateFinEffet)
VALUES 
	
	('CODE1', '01/01/2008','NULL'),
('CODE2', '01/01/2008','NULL');

[djeanprost]

I wonder if the problem could be in org.apache.ibatis.jdbc.ScriptRunner.executeStatement(String) where multiple "resultsets" are not handled correctly. For instance, java.sql.Statement.execute(String) returns only the result of the first statement, but to get the others statements results, some more code is needed.

[harawata]

It seems that we need to call java.sql.Statement#getMoreResults() to let MS SQL Server throw the exception.
mssql-jdbc team says it follows the JDBC spec.
microsoft/mssql-jdbc#367

It may be possible to adjust Migrations behavior.

@h3adache ,
We chatted about copying ScriptRunner to Migrations, but how about moving it out of core and maintain it as an independent library?
This way, the core can use this library as a test dependency and we can avoid having duplicated code.

[h3adache]

Where are you thinking of @harawata? Seem a bit extreme to have just a single file as a 'common'.

[harawata]

@h3adache ,
I was thinking about moving ScriptRunner, SqlRunner and RuntimeSqlException to a new repo like 'mybatis-script-runner' or 'mybatis-tools' because they do not depend on MyBatis and are used only by tests in the core.
Anyway, it was just an idea and maybe you are right about being extreme, so let's leave the idea for now.

The issue I would like to solve is that Migrations releases being blocked by the core, so if you are OK with copying ScriptRunner to Migrations, that is good enough as a workaround.

[h3adache]

Sorry for the late response @harawata! I think a mybatis-tools would be great but most of the tools that I can think of that would be most handy would be built on top of and to 'enhance' the user experience with mybatis itself. I'm definitely open to the idea of a common or tools type library. I wonder what else the other devs might find handy to split out

[harawata]

Hi @djeanprost ,
Just FYI, this has been fixed in the core.
It should be possible to verify the fix by replacing MyBatis JAR in 'lib' directory and edit the startup script (bin/migrate).
Thank you for taking the time to report this!

And thank you for the comment, @h3adache !
Let's keep that option open.

[djeanprost]

Hello @harawata
I just tested with https://oss.sonatype.org/content/repositories/snapshots/org/mybatis/mybatis/3.5.0-SNAPSHOT/mybatis-3.5.0-20180515.194833-33.jar instead of "C:\bin\mybatis-migrations-3.3.3\lib\mybatis-3.4.6.jar", and my first test fails : mybatis still exit with a success return code instead of failing.

I will double check my configuration setup.
Can you confirm the mybatis.jar version I used contains your fix ?
Thank you

[harawata]

Thanks for testing, @djeanprost .
Yes, that version should work as you expect.
Here are a few things you should check.

• The original mybatis-3.4.6.jar must be removed from the lib directory. The JARs in that directory are added to the classpath.

• This one would be obvious, but as you're on Windows, the startup script you need to edit is bin/migrate.cmd.

  @@ -72,7 +72,7 @@
   
   if "%REPO%"=="" set REPO=%BASEDIR%\lib
   
  -set CLASSPATH="%BASEDIR%"\etc;"%REPO%"\mybatis-3.4.6.jar;"%REPO%"\mybatis-migrations-3.3.4.jar
  +set CLASSPATH="%BASEDIR%"\etc;"%REPO%"\mybatis-3.5.0-20180515.194833-33.jar;"%REPO%"\mybatis-migrations-3.3.4.jar
   
   set ENDORSED_DIR=lib
   if NOT "%ENDORSED_DIR%" == "" set CLASSPATH="%BASEDIR%"\%ENDORSED_DIR%\*;%CLASSPATH%

[djeanprost]

@harawata
Well, I followed your instructions and it's now ok. I hadn't expected that jars in lib are actually added twice in classpath :

• once by endorsed mecanism
• once by being explicitely added

So after removing the old jar in the lib directory, my script fails as expected because of my error in my script.
Thank you for helping. I think I will close this issue, and wait for the next release of the project.