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

feature request for generating flyway SQLs (edge case?) #955

Open
hohwille opened this issue Jul 19, 2019 · 6 comments
Open

feature request for generating flyway SQLs (edge case?) #955

hohwille opened this issue Jul 19, 2019 · 6 comments
Assignees

Comments

@hohwille
Copy link
Member

I created CobiGen templates to generate Flyway SQLs and therefore facing several edge cases that I could not solve with CobiGen:
According to our conventions the result must be generated into:
src/main/resources/db/migration/«version»/V«sequence»__«description».sql

While I can easily use static parts and variables to build «description» for my demands, I fail to fill the other two placeholders:

  • «version» would be extremely tricky to figure out: actually it IMHO needs to be the first two digits of the version number from the current POM (so in case of 1.2.3-alpha1-SNAPSHOT it would be 1.2.)
  • «sequence» would be the next available sequence number so requires to scan for all existign Vxxxx__* files and get the maximum + 1.

Please also note that we could change the guidelines in devon4j to switch from sequnces to timestamps for flyway SQLs. We already got feedback from a project that this would be best practice also for merging feature branches, but we never catched this up. Then IMHO it would be relatively easiy but IMHO still cobigen does not yet support a current timestamp variable.

I just want to open this discussions for us to plan and think beyond. I am not requesting these enhancements now and am entirely open to reject them as well to consider them.

@hohwille
Copy link
Member Author

Did we ever consider adding SQL support (mergers)?

@jdiazgon
Copy link
Member

Somehow related to #860. Several user have asked me why CobiGen does not generate SQL files. I think this could be a good opportunity to add this.

Did we ever consider adding SQL support (mergers)?

No as far as I know, maybe @maybeec can shed some light in here.

I just want to open this discussions for us to plan and think beyond.

I also agree that this is very tricky. In my opinion, we should implement some naive solutions because it seems too much efforts for following properly the devon4j SQL naming conventions.

My naive solution would be:

  • Create one SQL file for tables creation statements called V0005__Create_Tables
  • Create one SQL file for adding sample data to the table V0006__Insert_Data
  • Use the current text-merger if possible, so that new SQL code will be appended at the end of each of those files.

@hohwille
Copy link
Member Author

  • text-merger should be fine. Otherwise generic override merger could also be used.
  • I would suggest to create individual SQL file per Entity as otherwise we will never be able to properly merge. Also this is the only way that makes sense for src/main/resources/db/migration as otherwise we would modify existing SQLs after they have already been released what is an absolute no-go for flyway.
  • For src/test/resources/db/test if would be suitable however to create a single SQL but merging this would also be very tricky. Mabe it would therefore be easier to do the same for test and main.
  • From CobiGen point of view I would suggest to generate timestamp based filenames and file an issue in devon4j asking to change suggestions in documentation as well as this would then be easy to solve and obviously makes sense.

BTW: I could easily provide my little work for #860 that I already did.

@jdiazgon
Copy link
Member

jdiazgon commented Jul 22, 2019

I would suggest to create individual SQL file per Entity

Okay, I agree.

Maybe it would therefore be easier to do the same for test and main.

Yes, from our user's point of view this makes more sense.

The only thing I'm not very convinced about are timestamps. My concerns are:

  • Timestamps look ugly to me, an SQL file like V22072019__«description».sql seems a bit weird.
  • Our users are already used to «sequence».

I think I would leave the file name decision to the user. What I mean is, let's generate an SQL with a big sequence value like V10000__«entityName», then our user should decide whether he wants to edit the file name.

@maybeec
Copy link
Member

maybeec commented Jul 22, 2019

Please also note that we could change the guidelines in devon4j to switch from sequnces to timestamps for flyway SQLs. We already got feedback from a project that this would be best practice also for merging feature branches, but we never catched this up.

This is quite strange. I would have thought that this makes more trouble as you might end up in execution of your SQLs in a non-intended sequence in case of merging brings up a maybe related sql script, which does not break but changes another sql script added to the top.
For me the "conflict" on the numbering was always an additional quality gate, which was good to have.

Did we ever consider adding SQL support (mergers)?

There was no intention to do so. Hibernate brings SQL generators for each SQL dialect.
https://tools.jboss.org/features/hibernate.html
You can generate your entities from DB. Generate SQL from DB, Generate SQL from entities, generate DB from entities. Therefore, I did not focus at all on SQL generation with CobiGen.
It would be even hard to re-implement hibernate entity configuration related to inheritance, embeddables, etc. to come up with the correct SQL files.

@github-actions
Copy link

Stale topic. Please negotiate closing or discussing the relevance of this ticket.

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

No branches or pull requests

5 participants