Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: apply relationship constraints when using sub query in whereIn condition #1037

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

adamcikado
Copy link
Contributor

πŸ”— Linked issue

#1036

❓ Type of change

  • 🐞 Bug fix (a non-breaking change that fixes an issue)

πŸ“š Description

Resolves #1036

πŸ“ Checklist

  • I have linked an issue or discussion.

@adamcikado
Copy link
Contributor Author

adamcikado commented Jun 13, 2024

The pipeline errors do not seem to be related to this PR.

@thetutlage
Copy link
Member

I am little confused on how these changes fixes that issue. I can see that you have abstracted this inline logic https://github.com/adonisjs/lucid/pull/1037/files#diff-76f631b7273d470789eccc7dbf79045349516bc0178df8de8dfa4425d2be1dd4R233-R234 to its own function. But how does that fixes the issue.

PS: I have not scanned the entire source code yet, so maybe I am missing something :)

@adamcikado
Copy link
Contributor Author

@thetutlage The whereIn method uses transformValue that converts Chainable using following logic:

if (value instanceof Chainable) {
      value.applyWhere()
      return value.knexQuery
    }

So there is applyConstraints() missing for relationships. I've added a toKnex method to Chainable which calls applyWhere and returns knexQuery. I override this method in relation query builder (like other methods do), so that toKnex also calls applyConstraints.

@thetutlage
Copy link
Member

Okay, let me give it a run locally to ensure nothing else breaks.

@adamcikado
Copy link
Contributor Author

@thetutlage any luck with testing this?

In my opinion, it's important issue and should be fixed. We lost some production data because of this a while ago and had to restore the table. We had something like this in the codebase:

Table1.query()
.where('table2_id', record.related('table2').query().select('id'))
.delete()

Which deleted all records in the table.

@thetutlage thetutlage merged commit c131bc7 into adonisjs:develop Aug 8, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Relationship constraints are not being applied when using sub query in whereIn condition
2 participants