You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@fineract.apache.org by Yemdjih Kaze Nasser <ka...@gmail.com> on 2020/08/23 13:12:14 UTC
Adopting GenerationType.TABLE for ID generation in
LoanRepaymentScheduleInstallment entity.
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.
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?
Thanks,
Regards,
Y. K. Nasser
Re: Adopting GenerationType.TABLE for ID generation in
LoanRepaymentScheduleInstallment entity.
Posted by Yemdjih Kaze Nasser <ka...@gmail.com>.
I ran into some trouble with LoanRepaymentScheduleInstallment entity and
the new ID generation strategy which I've Implemented was a last resort
solution as most of the things I tried over weeks were unfruitful.
I will propose this generation strategy with Openjpa in a PR so we can
further this discussion there.
On Tue, Aug 25, 2020, 15:50 Michael Vorburger <mi...@vorburger.ch> wrote:
> On Tue, Aug 25, 2020 at 4:42 PM Yemdjih Kaze Nasser <ka...@gmail.com>
> 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 <mi...@vorburger.ch> wrote:
>>
>>> On Sun, Aug 23, 2020 at 2:12 PM Yemdjih Kaze Nasser <
>>> kazenasser@gmail.com> 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
>>>>
>>>
Re: Adopting GenerationType.TABLE for ID generation in
LoanRepaymentScheduleInstallment entity.
Posted by Michael Vorburger <mi...@vorburger.ch>.
On Tue, Aug 25, 2020 at 4:42 PM Yemdjih Kaze Nasser <ka...@gmail.com>
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 <mi...@vorburger.ch> wrote:
>
>> On Sun, Aug 23, 2020 at 2:12 PM Yemdjih Kaze Nasser <ka...@gmail.com>
>> 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
>>>
>>
Re: Adopting GenerationType.TABLE for ID generation in
LoanRepaymentScheduleInstallment entity.
Posted by Yemdjih Kaze Nasser <ka...@gmail.com>.
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?
On Tue, Aug 25, 2020, 15:26 Michael Vorburger <mi...@vorburger.ch> wrote:
> On Sun, Aug 23, 2020 at 2:12 PM Yemdjih Kaze Nasser <ka...@gmail.com>
> 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
>>
>
Re: Adopting GenerationType.TABLE for ID generation in
LoanRepaymentScheduleInstallment entity.
Posted by Michael Vorburger <mi...@vorburger.ch>.
On Sun, Aug 23, 2020 at 2:12 PM Yemdjih Kaze Nasser <ka...@gmail.com>
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
>