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

selectQueryBuilder.take messes up the query when using the ids parameter #5174

Closed
cuchi opened this issue Dec 3, 2019 · 0 comments · Fixed by #5178
Closed

selectQueryBuilder.take messes up the query when using the ids parameter #5174

cuchi opened this issue Dec 3, 2019 · 0 comments · Fixed by #5178

Comments

@cuchi
Copy link
Contributor

cuchi commented Dec 3, 2019

Issue type:

[ ] question
[x] bug report
[ ] feature request
[ ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[ ] mssql
[ ] mysql / mariadb
[ ] oracle
[x] postgres
[ ] cockroachdb
[ ] sqlite
[ ] sqljs
[ ] react-native
[ ] expo

TypeORM version:

[ ] latest
[ ] @next
[x] 0.2.20

Steps to reproduce or a small repository showing the problem:

This issue happens when I have something like:

const query = this.createQueryBuilder('employee')
// ... complex joins and stuff...
query.where('task.id IN (:...ids)', { ids: [foo, bar, etc] })

const result = await query.take(5).getMany()

This causes TypeORM to bahave unexpectedly, loading an empty result.

I can see in the generated query that it substitutes by :...ids with the parameters .take() uses internally.

I consider it a bug because it can frustrate a lot of unsuspecting people. Maybe I would rename it to something more obscure to the end user.

cuchi added a commit to cuchi/typeorm that referenced this issue Dec 4, 2019
cuchi added a commit to cuchi/typeorm that referenced this issue Dec 4, 2019
cuchi added a commit to cuchi/typeorm that referenced this issue Dec 6, 2019
pleerock pushed a commit that referenced this issue Dec 20, 2019
* Use a leading underscore on SelectQueryBuilder internal parameters

* Add test case for issue #5174

* Fix lint issues

* Improve test for issue #5174

* Use a different prefix for the former ...ids

* Remove .only
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 a pull request may close this issue.

1 participant