Can't create multiple stored procedures with transactions in setup script for MS-SQL

[GregJohnStewart]

Describe the bug

I'm working to import a schema from an existent MS-SQL database, and have devservices handle setting up that test instance and use a specified .sql file to set it up.

However, it seems that there is no way to have more than one CREATE PROCEDURE statement present in that file, as it is treated as one batch. The GO keyword is also not available in this setup script, making it impossible to setup the schema I need for testing.

This is further confirmed as the case when the script is pasted into MS SQM Server Management Studio; the first CREATE PROCEDURE statement has an error noting that multiple cannot exist in the same batch.

Possible solutions, from my thoughts:

• Somehow recognize the GO keyword, or other way to separate batches in the same file
• Support multiple scripts specified in quarkus.datasource.devservices.init-script-path, and execute each in order

Expected behavior

An SQL Script that has multiple CREATE PROCEDURE commands should be able to used

Actual behavior

Error message:

Caused by: org.testcontainers.ext.ScriptUtils$ScriptStatementFailedException: Script execution failed (import.sql:234): CREATE PROCEDURE testProcedureOne ( @Correlation_Id VARCHAR(100) ) AS BEGIN

How to Reproduce?

Example setup script:

import.sql

CREATE TABLE foo;
...

CREATE PROCEDURE testProcedureOne 
(@Process AS VARCHAR(50))
AS
BEGIN
        BEGIN TRY
        BEGIN TRANSACTION -- having this transaction causes issues
        SELECT * FROM foo;

        COMMIT TRANSACTION
    END TRY
    BEGIN CATCH
        ROLLBACK TRANSACTION
        SELECT * FROM foo;

    END CATCH
END;

CREATE PROCEDURE testProcedureTwo
(@Process AS VARCHAR(50))
AS
BEGIN
    SELECT * FROM foo;
END;

Output of uname -a or ver

MINGW64_NT-10.0-19044 DEVDP-AM027 3.3.6-bec3d608-341.x86_64 2023-02-22 08:29 UTC x86_64 Msys

Output of java -version

openjdk version "11.0.18" 2023-01-17 LTS

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.16.6

Build tool (ie. output of mvnw --version or gradlew --version)

Maven

Additional information

No response

[quarkus-bot[bot]]

/cc @DavideD (hibernate-reactive), @Sanne (hibernate-orm,hibernate-reactive), @gavinking (hibernate-reactive), @gsmet (hibernate-orm), @mswatosh (db2), @yrodiere (hibernate-orm)

[geoand]

Hi,

Can you please attach a sample application which we can use for checking the problematic behavior and potential fixes?

Thanks

[yrodiere]

This is most likely a limitation of the multi-line import script parser. Basically Hibernate ORM has to parse the import script in order to push the appropriate string into each JDBC statement (in your case, all the text from CREATE PROCEDURE to END;, and in this case it seems it parsed wrong.

I second that a reproducer would be useful.

Also, do you get the same error in Quarkus 3.0.0.CR2, which uses a different (hopefully better) parser?

Regarding the implementation, see hibernate.hbm2ddl.import_files_sql_extractor in the Hibernate ORM user guide; Quarkus sets that to multi-line by default, which means it uses MultiLineSqlScriptExtractor, which relies on this lexer and this parser.

[GregJohnStewart]

Quarkus 3.0.0.CR2 gives me a ton of compilation errors, complaining that javax.persistence package does not exist.. This would be my first time trying 3.x, so haven't looked into migration procedures yet.

I'm working on a reproducer now, but having the rather annoying issue of not being able to reproduce the issue in a simplistic example. Still hashing at it though

[geoand]

You need to replace javax with jakarta or use our tooling for upgrading

[GregJohnStewart]

So I have a poc going here: https://github.com/GregJohnStewart/quarkus-proofs/tree/main/32552-multi-stor-proc-poc

~~however, I've noticed that I cannot seem to get it to pick up the import script; it does not pull in the default import.sql, and when I specify a different script via quarkus.datasource.devservices.init-script-path, it says that the script cannot be found ~~ (resolved)

I have added a test that just prints out the existent tables, which can be used to verify that the script is read in as it should be.

[GregJohnStewart]

Got through the pathing issues I was having, but now can't reproduce the issue. It runs just fine...

This is really strange to me, as in the 'real' project, both stored procedures are fine on their own, and only have issues when both are included

[GregJohnStewart]

Found the issue! It appears that having a transaction within the stored procedure is what kicks off the error:

https://github.com/GregJohnStewart/quarkus-proofs/blob/main/32552-multi-stor-proc-poc/src/test/resources/db/importScript.sql#L26

[GregJohnStewart]

Also, Just swapped the code to Quarkus 3.0.0.CR2, no change in error

[yrodiere]

This is most likely a limitation of the multi-line import script parser. Basically Hibernate ORM has to parse the import script in order to push the appropriate string into each JDBC statement (in your case, all the text from CREATE PROCEDURE to END;, and in this case it seems it parsed wrong.

So that was completely wrong as I thought you were using quarkus.hibernate-orm.sql-load-script but you are using quarkus.datasource.devservices.init-script-path.

FWIW quarkus.hibernate-orm.sql-load-script seems to fare even worse with that script, but that's a different topic.

Back to quarkus.datasource.devservices.init-script-path... Quarkus doesn't do anything with that script and simply passes it to MSSQLServerContainer#withInitScript, which ultimately triggers a call to org.testcontainers.ext.ScriptUtils#splitSqlScript, which understands BEGIN/END, but unfortunately not COMMIT, so it doesn't correctly see that the BEGIN TRANSACTION block ends.

I don't think we can support multiple scripts in quarkus.datasource.devservices.init-script-path , as testcontainers only supports one such script. I'm not sure it would solve the problem anyway.

If we want to fix this, we'll need to submit a PR to https://github.com/testcontainers/testcontainers-java/tree/main/modules/database-commons to add BEGIN TRANSACTION/COMMIT TRANSACTION support.

[yrodiere]

FWIW this has been reported to the testcontainers project before: https://github.com/testcontainers/testcontainers-java/issues/6435

They basically recommend using container-specific initialization methods. Which I guess we could do instead of using withInitiScript, where supported. But while that's relatively easy with PostgreSQL or Mariadb, it seems with MS SQL Server it would require a custom image... Someone might be able to find an alternative with just volume mounts and a custom entrypoint though, I don't know.

[yrodiere]

Finally, a workaround for anyone with this problem: use either Flyway or Liquibase, which should be able to handle more complex init scripts.

[geoand]

@yrodiere so assume there is nothing to be done on our side?

If so, we can close the issue

[yrodiere]

Well I guess there's that:

They basically recommend using container-specific initialization methods. Which I guess we could do instead of using withInitiScript, where supported. But while that's relatively easy with PostgreSQL or Mariadb, it seems with MS SQL Server it would require a custom image... Someone might be able to find an alternative with just volume mounts and a custom entrypoint though, I don't know.

But I'm not sure we want to go there, as this will likely be fragile.

Submitting a PR to testcontainers would be the way to go, IMO. They didn't want to fix it themselves, but they may accept a PR if someone submits one and asks nicely, explaining this affects Quarkus and containers that don't have easy workarounds such as MS SQL Server.

Anyway, yes, nothing we can do in Quarkus proper. Closing.

[GregJohnStewart]

Unfortunate we can't fix it here, then. What's more unfortunate is that Flyway seems to have its own problems with the script (haven't tried liquibase). Still digging, thanks for the inputs

[Sanne]

FWIW, I understand the Testcontainers team wouldn't want to support such a thing. It's a pandora box, nearly impossible to being able to parse any SQL native dialect from any vendor.

In Hibernate we try hard but the intent is to have simple import scripts work, for convenience; we have to accept there's limitations. For more complex cases we have the option to use the simpler "dumb" parser which expects each statement to be one a new line; that's how most tools deal with this - the downside being that one can't maintain nicely formatted scripts.

[GregJohnStewart]

That's some good insight, but would it not be easy to just pass the whole script to the db itself? There is probably a detail I'm missing, but feels like (perhaps especially in MS-SQL's case), there are dialect differences that matter and ignoring it seems like an interesting choice.

I'd agree from a microservice/ Domain-driven design (with simpler schemas) this should be more of an edge case, but still hard to say one can't use dialect-specific syntax. You lose out on the advantages of that platform

[Sanne]

yeah I'd love to "pass the whole script to the db", but they don't typically expose a capability like that. It needs to go per-statement (typically - some DBs might have some custom facilities, but it's not in the JDBC standard).

[GregJohnStewart]

Made a TestContainers issue, fingers crossed. Thanks for the insight! https://github.com/testcontainers/testcontainers-java/issues/6917