search

mtanneryd / ef-bulk-operations

Bulk operations for Entity Framework 6
Apache License 2.0
80 stars 30 forks source link

fix for insert in table with computed column #30

Closed hzahradnik closed 4 years ago

hzahradnik commented 4 years ago

We found another problem with this library when using for our project. When a table has a computed column, the current version tries to insert this column, which of course fails as it is not allowed to insert a specific value to a computed column.

As a solution, the call to select the properties now filters a column, if IsStoreGeneratedComputed is true. To test this, I refactored the context I added in the previous PR #29 and added a computed column. As a consequence I had to reorder the existing migrations.

Unfortunately, I added the changes onto #29, instead of adding it onto your master. I looked into rebasing, but as I use the same context, it would be rather difficult. If you want, I can try, but for now I leave it like it is and rebase it, if you merge #29.

If you need anything else, you can ask us for help. We are eager to help.

Edit: As you can see in the commits, I have wrongly used identity column. To avoid any misunderstandings, the fix is for computed columns.

mtanneryd commented 4 years ago

Thanks for the feedback. I'll try to have a look at this asap and also try getting the next release out. I am currently the only one in my family not dealing with the flu so I've been a bit busy taking care of wife and kids, but things are looking up now so I'm slowly getting back to normal.

Måns Tånneryd Tånneryd IT AB Barrskogsvägen 19 186 53 Vallentuna +46-705140093 https://se.linkedin.com/in/manstanneryd

https://github.com/mtanneryd/ef6-bulk-operations https://github.com/mtanneryd/ef6-bulk-operations

Den ons 18 mars 2020 kl 08:28 skrev hzahradnik notifications@github.com:

We found another problem with this library when using for our project. When a table has a store identity, the current version tries to insert this column, which of course fails as it is not allowed to insert a specific value to an identity column.

As a solution, the call to select the properties now filters a column, if IsStoreGeneratedComputed is true. To test this, I refactored the context I added in the previous PR #29 https://github.com/mtanneryd/ef6-bulk-operations/pull/29 and added a identity column. As a consequence I had to reorder the existing migrations.

Unfortunately, I added the changes onto #29 https://github.com/mtanneryd/ef6-bulk-operations/pull/29, instead of adding it onto your master. I looked into rebasing, but as I use the same context, it would be rather difficult. If you want, I can try, but for now I leave it like it is and rebase it, if you merge #29 https://github.com/mtanneryd/ef6-bulk-operations/pull/29.

If you need anything else, you can ask us for help. We are eager to help.

You can view, comment on, or merge this pull request online at:

https://github.com/mtanneryd/ef6-bulk-operations/pull/30 Commit Summary

  • add test case to show issue with primary key mapped to different column name
  • add test case for non existing select of model with different primary key column name
  • fix for issue #23
  • Refactored migrations folder, add migration to InvoiceContext
  • Add test for store identity column
  • fix for BulkInsertAll on table with identity column

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mtanneryd/ef6-bulk-operations/pull/30, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2BSROYALZZGKKUOQVLAGDRIBZYBANCNFSM4LOJJH7Q .

Brandinga commented 4 years ago

no hurry! you are doing a great job on this library. thanks (head of hzahradnik)

mtanneryd commented 4 years ago

Hi!

I've started working on this now. The error you got only happens when you have a computed column in a table with a user generated primary key. I already skip the computed columns when inserting into tables with database generated keys but forgot to deal with them in other tables. However, the way we avoid the computed columns today is less optimal than the way you suggested so I'll refactor the code using your suggested solution. There are some other refactorings being made also so instead of merging your changes from your branch I'll simply add them to my current development branch. I also plan to add the following to the readme file.

"Fixed a bug when using computed columns in tables without identity primary keys (reported and resolved by https://github.com/hzahradnik)"

If you prefer not to be mentioned in the README please let me know.

Måns Tånneryd Tånneryd IT AB Barrskogsvägen 19 186 53 Vallentuna +46-705140093 https://se.linkedin.com/in/manstanneryd

https://github.com/mtanneryd/ef6-bulk-operations https://github.com/mtanneryd/ef6-bulk-operations

Den ons 18 mars 2020 kl 10:30 skrev Brandinga notifications@github.com:

no hurry! you are doing a great job on this library. thanks (head of hzahradnik)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mtanneryd/ef6-bulk-operations/pull/30#issuecomment-600500450, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2BSRPLI57LUEGHUX54KVDRICEGBANCNFSM4LOJJH7Q .

mtanneryd commented 4 years ago

Working on the fix but got into some unexpected issues with InvoiceItem table. I'll keep you posted.

/Måns

Måns Tånneryd Tånneryd IT AB Barrskogsvägen 19 186 53 Vallentuna +46-705140093 https://se.linkedin.com/in/manstanneryd

https://github.com/mtanneryd/ef6-bulk-operations https://github.com/mtanneryd/ef6-bulk-operations

Den mån 23 mars 2020 kl 09:01 skrev Måns Tånneryd mans@tanneryd.se:

Hi!

I've started working on this now. The error you got only happens when you have a computed column in a table with a user generated primary key. I already skip the computed columns when inserting into tables with database generated keys but forgot to deal with them in other tables. However, the way we avoid the computed columns today is less optimal than the way you suggested so I'll refactor the code using your suggested solution. There are some other refactorings being made also so instead of merging your changes from your branch I'll simply add them to my current development branch. I also plan to add the following to the readme file.

"Fixed a bug when using computed columns in tables without identity primary keys (reported and resolved by https://github.com/hzahradnik)"

If you prefer not to be mentioned in the README please let me know.

Måns Tånneryd Tånneryd IT AB Barrskogsvägen 19 186 53 Vallentuna +46-705140093 https://se.linkedin.com/in/manstanneryd

https://github.com/mtanneryd/ef6-bulk-operations https://github.com/mtanneryd/ef6-bulk-operations

Den ons 18 mars 2020 kl 10:30 skrev Brandinga notifications@github.com:

no hurry! you are doing a great job on this library. thanks (head of hzahradnik)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mtanneryd/ef6-bulk-operations/pull/30#issuecomment-600500450, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2BSRPLI57LUEGHUX54KVDRICEGBANCNFSM4LOJJH7Q .

hzahradnik commented 4 years ago

As we already successfully use the latest version, this PR should be obsolete.

Thank you, great project. 👍