Support composite primary keys

[benjamincanac]

Proposed changes

Support composite primary keys for updates and deletes.

Instead of updating how works static get primaryKey() to handle arrays and breaking everything, I added two new methods static get updatePrimaryKey() and deletePrimaryKey().

During update / delete the query is built accordingly to those two new methods. If it's an array, it allows you to save with composite primary keys.

This PR references #167.

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

• I have read the CONTRIBUTING doc
• Lint and unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works.
• I have added necessary documentation (if appropriate)

Further comments

Can you tell me what you think of this and if it's the way to go?

Also can you tell me where I can add tests for this feat?

[thetutlage]

Hello @benjamincanac

Thanks for sharing the PR :)

I believe, we can use the primaryKey getter directly without introducing new properties. If the value of primaryKey is an array, we perform the multiple where logic.

Regarding breaking change, no existing project have been returning array from this getter, so it's safe to re-use the same getter.

Testing

The tests will go inside https://github.com/adonisjs/adonis-lucid/blob/develop/test/unit/lucid.spec.js and here's how you can construct them.

1. Create a new table inside https://github.com/adonisjs/adonis-lucid/blob/develop/test/unit/helpers/index.js#L195 which relies on composite keys. Also make sure to drop the table after tests
2. I believe a CRUD operation in 4-5 tests will cover the use cases for composite keys.

[benjamincanac]

Hi @thetutlage

Thanks for your answer, I'm gonna close the PR and make another one with valid changes!

[RomainLanz]

Hey @benjamincanac! 👋

Any news on this?

[benjamincanac]

Hi @RomainLanz,

Actually I ended up by not doing it, the code was way too complex than with just an ID as a primary key.

[yariksav]

Need this enhancement

[scal-lider]

Hello people,

Any new on this proposal?

List of related issues and discussion about this that I found:

• Issues Closed
  • Wrong SQL Insert Statement of Multiple Columns Primary Key #167
  • Support composite primary keys #382
  • Model relationship for Composite Keys ? #404
• Discussions: answer is Lucid do not support it for now
  • #187
  • #2224