Add metadata to stored procedures for 3rd party libs

[mkurz]

Hi @rbygrave!

You may remember the problem we, from the Play Framework, had in

• [2.7.x][5.0.x] Evolution fails for auto-generated script contains create procedures script playframework/play-ebean#166

The problem is that Play splits sql scripts by ; meaning that any stored procedures that usually contains ; in their body will be split into weird statements, making the sql script fail. We do have an escape mechanism by using two ;;, which avoids the string getting split and the double ;; will end up as a single ; in the final statement.

The problem is that when ebean (re)generates its ddl scripts it will override existing once, so each time people would have to replace ; with ;; which is annoying. Even worse, lots of people don't even know what's going on and just see an exception without having a clue why ebean generated ddl scripts don't apply with Play evolutions and just open issues like "ebean doesn't work", "play-ebean doesn' work", etc.

Now I think the solution to this problem is actually quite easy:
If you could add meta data to your ddl scripts, like shown in this pull request, we could easily parse that meta data and make Play evolution ignore the semicolon within such a "meta data block".
The naming of the meta data comments I came up with is currently:

-- !Play-Evolutions: KEEP SEMICOLONS START
-- !Play-Evolutions: KEEP SEMICOLONS END

but I am totally open for other naming.

What do you think about that? IMHO this shouldn't hurt ebean but 3rd party libraries would benefit from finally being able to correctly handle your ddl scripts.

Thanks!

[xabolcs]

Ohh, I hoped you already have a solution, but this PR is just an issue, with a code example, isn't it?

Just reading the title and the description I tought you would implement a new entrypoint for 3rd party libs in the ddl generation code to programatically put those BEGIN & END markers in the ddl payload.

(Just to note: I'm just a follower, I don't even use Ebean. 😅 But the solution you came up is very interesting. Subscribed to the issue.)

[mkurz]

but this PR is just an issue, with a code example, isn't it?

No, this is not just an issue, it's also an actual pull request with the change required to make this work. The only thing that needs to be discussed is the naming of the "meta data".

Just reading the title and the description I tought you would implement a new entrypoint for 3rd party libs in the ddl generation code to programatically put those BEGIN & END markers in the ddl payload.

No, this isn't necessary, my solution is much simpler and does its job perfectly.

(Just to note: I'm just a follower, I don't even use Ebean. 😅 But the solution you came up is very interesting. Subscribed to the issue.)

You come here from the Play Framework issue?

[xabolcs]

You come here from the Play Framework issue?

I was browsing the activity of the repo with OctoDroid and noticed that the lead developer of Play! Framework opened a PR with an interesting title. 🙂

[rbygrave]

Well, I think there are 2 other alternatives to consider.

One option would be to just copy Ebean's DdlParser code - https://github.com/ebean-orm/ebean-ddl-runner/blob/main/src/main/java/io/ebean/ddlrunner/DdlParser.java#L55-L61 ... with the benefit that this would work with not just ebean migrations but any DDL scripts that people want to parse and execute.

A second option is to adjust the Play parser to support the "more standard" delimiters and I'm pretty sure there are only the 2 cases needed to be supported for identifying the start and end which is:

• delimiter $$ and $$ [for MySql, MariaDB, Postgres etc]
• contains PROCEDURE and GO [for MS SQL Server]

So these approaches have some benefit as I see it. Have these options been considered?

[rbygrave]

The problem is that when ebean (re)generates its ddl scripts it will override existing once, so each time people would have to replace ; with ;; which is annoying.

I'm not sure if there is a misunderstanding here but Ebean has 2 types of DDL generation:
(A) DB Migrations - https://ebean.io/docs/db-migrations/#generate
(B) "create-all.sql" / "drop-all.sql" - https://ebean.io/docs/ddl-generation/#create-all

Ebean will regenerate the "create-all.sql" / "drop-all.sql" every time but to be clear these are never intended to be used for db migrations but instead for general local development running tests. People should not be treating these as migration scripts.

Ebean has a separate tool to generate db migrations (based on a "diff" of the model). This tool is generally explicitly run to generate the next db migration script (and model change). When we want to create a db migration we should use this tool and this does not re-generate or override any ddl scripts etc.

Play splits sql scripts by ;

To me that sounds like it does not support MS SQL Server GO based scripts.