jplindstrom / p5-DBIx-Class-BatchUpdate

Perl module to update batches of DBIC rows with as few queries as possible
0 stars 1 forks source link

support for non-id field as primary key column name #6

Closed mohanprasadgutta closed 4 years ago

mohanprasadgutta commented 4 years ago

Hi @jplindstrom ,

Thanks for your efforts in writing this module for batch update. I would like to use this module for batch updating via DBIx-Class in my project. But it seems at present its accepting only id as primary key column name as documented. Usually we can have primary key column name different from id, i prepared patch to use non-id as primary key column name. I verified tests and non-id field test scenario 'pkid' is already covered in your test script and its passed with this patch. Could you please review this git patch(i zipped git patch file as github not allowing me to attach .patch files) and merge to main repo if possible.

Thanks Mohan

0001-Allow-non-id-field-name-as-primary-key.zip

jplindstrom commented 4 years ago

Hi Mohan,

Thanks for the patch! I'll have a look now.

jplindstrom commented 4 years ago

I think the module actually already does work with PKs named things other than "id".

I agree this isn't super obvious from the tests though, and I had to read a bit of code to figure out that it makes sense :)

The reason it works is that the DBIx::Class::Row->id attribute is a bit magical and always refers to the value(s) of the actual primary key, even if the PK column is called something else. And the actual update uses the ->pk_column. So in theory it should work.

Can you test the current version with your code base please? If you find that it doesn't in fact work, I've got a branch ready with a fix, but I'd prefer to not touch it since I don't have a code base available to actually try it out easily myself at the moment.

Please let me know what you find out.

mohanprasadgutta commented 4 years ago

Hi @jplindstrom , Thank you for quick reply. Yeah. I tried and It worked for non-id primary key. I was not aware that DBIx::Class::row->id magical stuff :). I think we can close this issue now. Thanks Mohan