On Tue, Aug 25, 2020 at 4:42 PM Yemdjih Kaze Nasser <[email protected]> wrote:
> Thanks for your input Michael. > > I limited my application to the one entity that was most involved with my > problem. I can bring this out to OPENJPA if it is accepted. > > But, please clarify me on one thing. Are you suggesting I apply this for > all entities as the new generation strategy or just for the > LoanRepaymentScheduleInstallment entity? > Why would we need different ID Generation Strategies for different Entities? That doesn't make much sense to me, and I fear will only lead to future confusion. Why would only the LoanRepaymentScheduleInstallment entity require a GenerationType.TABLE ID Generation Strategy, but all other Fineract entities another one? That's... curious. Weird. I'm kind of reading between the lines that you probably ran into a particular issue with LoanRepaymentScheduleInstallment during your OpenJPA to EclipseLink migration. Perhaps whatever caused that could be fixed differently? On Tue, Aug 25, 2020, 15:26 Michael Vorburger <[email protected]> wrote: > >> On Sun, Aug 23, 2020 at 2:12 PM Yemdjih Kaze Nasser <[email protected]> >> wrote: >> >>> Hello all, >>> >>> I have a new strategy I have implemented in EclipseLink migration. I >>> faced a particular situation where the ID was not generated during the >>> persistence of an entity causing a lot of SQLIntegrityViolations. >>> To overcome this, I used GenerationType.TABLE for ID generation on the >>> entity that faced the most trouble with this >>> (LoanRepaymentScheduleInstallment entity). For damage control, it's the >>> only entity that has this change. >>> >>> I already have a sample of it on my PR at >>> https://github.com/apache/fineract/pull/928. >>> >> >> I think this is the kind of thing that would be easiest to review and >> discuss through a smaller PR proposing to gradually make just that >> particular change. >> >> I'm assuming that it would be technically feasible to use @Id >> @GeneratedValue(strategy = GenerationType.TABLE) with OpenJPA already as >> well, like EclipseLink. >> >> >>> It has been tested against the current IT tests we have and it seems to >>> work fine. >>> >>> Now I'd like to get your opinion on this, is this something that will >>> pose a problem in production, or can we roll with it? >>> >> >> Looking at this >> <https://github.com/apache/fineract/blob/0b76ffbc83a173eddfbbdf8af038f695e6ce9146/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanRepaymentScheduleInstallment.java>, >> my feedback would be that this shouldn't be done like that directly in >> an @Entity, but EITHER we should have a new AbstractPersistableCustom >> alternative equivalent (with GenerationType.TABLE), or... just even just >> change AbstractPersistableCustom directly, really. Because why would we >> need different ID Generation Strategies for different Entities? That >> doesn't make much sense, to me, and I fear will only lead to future >> confusion. >> >> BTW if we do do conclude that we want to change this, it would have some >> interesting data migration implications... ;-( >> >> Thanks, >>> Regards, >>> Y. K. Nasser >>> >>
