Add custom script for database creation

[kvanover]

[wokket]

Thanks for looking into this @kvanover 🥳

I've made a few small comments above, but in general this looks very close to me!

[kvanover]

All should be resolved but I don't know if you're a fan of self-resolving or I can, so I let it open :)

[wokket]

Sorry mate I don't think I explained my issue with the test very well.

I'm looking to see the test confirm that we ran the script file, rather than the default behaviour. One way we could do that is by altering the script file to create a database with a different name to the one passed in on the config, but I'm open to any sort of check that confirms we ran the .sql, not the default create statement from ISyntax.

[kvanover]

OK, go it. Will adapt the test 👌

[kvanover]

Added a SupportCreateCustomScript for MariaDB, PostgreSQL and SqlServer.

[wokket]

Why the sudden decision to drop Oracle support?

[kvanover]

No drop, still in progress. Was a bit hesitant because Oracle was not with 'create database' syntax. That's why I first did the defaults and need to see what I can do for oracle. If you have ideas...

[wokket]

Oracle just uses different syntax, and shoudn't affect whether a suitably knowledgable user can pass in a custom script or not.

If you stayed with your original test that initialised the migrator with (say) FakeDb database name, but you populated the .sql file using the ISyntax create code like before, but with the dbase name DbaseItem253 as the token, then you could happily assert that FakeDb doesn't exist, while DbaseItem253 does.

[kvanover]

The main problem here is that it crashes because it does not find 'FakeDb' with the custom script change.

[wokket]

Mmmmm, that makes sense, I didn't see that coming!

Maybe your two-database plan is best then, but just write the ISyntax twice with the different DB names?

[kvanover]

Ok, did it a bit differently. catch the error but I verify that the database is created with the custom script. Oracle is now also included.

[kvanover]

Is there still something I need to do? Or do you not agree with the catch approach?

[wokket]

Sorry mate, I'm on holidays atm so internet access is limited.

I have pending reviews above to:

• remove all the docco etc indicating this feature is only available on specific DBMS's, as by my thinking it should apply to any target database?
• Remove the config flag about whether a DBMS supports a custom script file, as they all should?
• As a nit we also have a warning from the build regarding the unused exception var in the catch block of the test.

[kvanover]

no problem, just checking if it was ok. For the config flag, the custom create script is very strange for Sqlite. You are creating a database file based on the connection string. I don't see how we can integrate the custom 'create database' script. So If I would remove the flag, I still need to do something different for the SQL lite version. The rest is set to true.

[erikbra]

Hi, @kvanover !

Thank you very much for your contribution, I really appreciate it, and my apologies for not having the time to respond sooner to your PR.

I like the new feature, and I see the need for it. It's just something bugging me, that we get very, very many command-line options as we add features, and this makes the tool a bit confusing to use. How do you (and you, @wokket ), feel about generalising this concept, so that both create and drop, and possibly other of the "Syntax" commands can be customised, should the need be there?

Would it make sense to, instead of providing a command-line switch for just create, to be able to provide a --syntaxOverrides <folder> command-line switch, where we could create scripts for CreateDatabase, DropDatabase, and maybe others, should it be needed in the future?

Say I specify:

--syntaxOverrides ./scripts

and put a script, CreateDatabase.sql in that folder, we change grate so that if there is a specified folder, we use the scripts there instead of the hard-coded ones in the XXXSyntax (e.g. SqlServerSyntax, OracleSyntax) classes?

[kvanover]

also perfect for me. Will update the code to reflect the requirements

[wokket]

Sorry guys, a little late to the party here.

Everything else in grate is "files called what you like in specially named folders". Could we add a "CreateDataabase" folder to go alonside "AlterDatabase" etc? No new commandline settings, and consistency withe the rest of the process?

[erikbra]

Sorry for the no-reply season here on my part, guys. Real work(TM) has occupied me a lot lately. I haven't had the time to look deeply into this, but, intuitively I'm really a fan of your suggestion with a custom folder, @wokket . This also takes even a bit more of the database-specific stuff out of grate.

I'll look closer into this when I have a bit more time :)

[erikbra]

This is solved with #289 - available in release 1.5.0