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

Add support for .populate({ select: [] }) #5549

Closed
anasyb opened this issue Mar 31, 2015 · 64 comments
Closed

Add support for .populate({ select: [] }) #5549

anasyb opened this issue Mar 31, 2015 · 64 comments

Comments

@anasyb
Copy link

anasyb commented Mar 31, 2015

The select is being ignored.. getting back the full records instead of just the titles.

Model.find().populate(assoc.alias, { select: ['title']});

Not sure if its only on mongo?

UPDATE @dmarcelino: changed this issue into a feature request.

@tjwebb
Copy link
Contributor

tjwebb commented Mar 31, 2015

I did not know you could select only certain attributes. Can you point to the docs on this?

@devinivy
Copy link

Possibly related to #509, which will be a part of 0.11.

@anasyb
Copy link
Author

anasyb commented Mar 31, 2015

I have been reading the code which seems to be written as if this is the behaviour... populate.js and deferred.js

The only documentation I found is here

Maybe it's still work in progress?

@anasyb
Copy link
Author

anasyb commented Mar 31, 2015

Do you guys know when that _insertJoinedResults which contains childRow = _.pick(childRow, select); is supposed to be called? I can't seem to be able to catch it with a breakpoint!

@anasyb anasyb changed the title Bug: populate sub-criteria isn't doing anything Should populate sub-criteria do anything? Mar 31, 2015
@dmarcelino
Copy link
Member

I know waterline supports "populate where": .populate('taxis', { medallion: { '<': 2 }}) (find.populate.where.js#L38) but I think "populate select" would be a new feature.

@devinivy
Copy link

devinivy commented Apr 1, 2015

If someone thinks this isn't full answered, feel free to reopen. But as of now, there's no select criteria.

@devinivy devinivy closed this as completed Apr 1, 2015
@atiertant
Copy link

shoud be a feature request

@anasyb
Copy link
Author

anasyb commented Apr 24, 2015

subscribers: sorry for the buzz :P nothing interesting to read.
Although this was about select, I just wanted to add reference to this issue for anyone landing here... few other things you can do with populate sub-criteria:
https://github.com/balderdashy/waterline/issues/334

@dmarcelino dmarcelino changed the title Should populate sub-criteria do anything? Add support for .populate({ select: [] }) Apr 24, 2015
@dmarcelino
Copy link
Member

Thanks @anasyb, that's valuable information. Some of it is actually in the docs (https://github.com/balderdashy/waterline-docs/blob/master/query.md#populate) but sort is not mentioned. Care to do a PR? 😃 :

Meanwhile at @atiertant's request I've changed this issue into a feature request and reopened it.

@dmarcelino dmarcelino reopened this Apr 24, 2015
@andriussev
Copy link

I have created a simple project to test the issue and it seems that waterline with the disk adapter (the default one) does work great with populate's selects.
Switching it out to mysql removes the functionality and the select attribute is ignored.

@dmarcelino
Copy link
Member

@nonpc, can you submit a patch to waterline-adapter-tests? There is already a find.populate.where.js suite of tests, so perhaps add a find.populate.select.js in https://github.com/balderdashy/waterline-adapter-tests/tree/master/interfaces/associations/manyToMany

@andriussev
Copy link

I have actually mostly fixed the issue but I don't feel good enough to do a pull request.
The fix is also not in the correct file since the query to join (it's just a seperate select) has "SELECT *..." hardcoded.

Changing the beginning of the query to:
https://github.com/nonpc/waterline-sequel/blob/8c7eada04f4dbe6e43ca3d88b4183e264bd0e06f/sequel/where.js#L192

Most of the logic resides here, a lot of copy-pasteing done from the select.js file:
https://github.com/nonpc/waterline-sequel/blob/8c7eada04f4dbe6e43ca3d88b4183e264bd0e06f/sequel/where.js#L234
Changes go until line 271.

Mind you, I've only tested it on MySQL.

I am actually not a fan of doing these seperate queries on simple joins so I myself am going to change all of the populate() to .query() and do all the joins myself until (if ever) all this gets fully solved.

@dmarcelino
Copy link
Member

I see, if you do find a fix you're comfortable with please let us know, thanks.

@ghost
Copy link

ghost commented Jul 1, 2015

👍 this would be a good feature

@atiertant
Copy link

@nonpc the problem is you will need to correct it in 3 times :

  • verify that populate add correct select in joins passed to adapter or PR waterline to correct it
  • PR waterline-adapter-test a failling test.
  • PR waterline-sequel to correct the hardcoded * .

@dmarcelino would you be ok with this ? do you think it's a feature or a bug ?
as select is a criteria and populate takes a criteria parameter,i think it's a bug ...

@dmarcelino
Copy link
Member

The first 2 are definitely right. I'm not sure about "PR waterline-sequel to correct the hardcoded * ." as I'm not aware of such hardcode. I'll trust your judgement there.

Feature or bug... definitions are not always easy... :) Given that only recently select was documented and added to waterline-adapter-tests (balderdashy/waterline-adapter-tests#71: 26 days ago) I'll vote for feature.

@barroudjo
Copy link

Just to let you guys know: this feature will come in handy !

@iamvajid
Copy link

Can't we use the methods like :toJSON or other custom methods on models to override the returning result ? And will this work on an array of result ?

@shamasis
Copy link

This would significantly improve performance for deletions of highly nested populations.

@shamasis
Copy link

I've even tried modifying deffered.prototype.populate to send correct selects in the joins that it builds internally. Still no solution.

Is select on joins supported at all?

@atiertant
Copy link

@shamasis i think modifying deffered.prototype.populate is the best way but this hasn't been tested at all in any adapter so the we need :

  • modifying deffered.prototype.populate to add correct select in joins passed to adapter.
  • PR waterline-adapter-test to test that select in joins are correctly used.
  • correct adapters ...

@shamasis
Copy link

@atiertant I've already implemented the correct join selects on a fork of waterline we maintain. Almost all adapters we tested does not respect the select criteria in joins. That's what I am working next. Will post updates and send PR when I can get around to it.

@devinivy
Copy link

That would be great. If there is a common pattern to convert the adapters, do share!

@sailsbot
Copy link

Thanks for posting, @anasyb. I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message. On the other hand, if you are still waiting on a patch, please:

  • review our contribution guide to make sure this submission meets our criteria (only verified bugs with documented features, please; no questions, commentary, or bug reports about undocumented features or unofficial plugins)
  • create a new issue with the latest information, including updated version details with error messages, failing tests, and a link back to the original issue. This allows GitHub to automatically create a back-reference for future visitors arriving from search engines.

Thanks so much for your help!

@devinivy devinivy reopened this Sep 27, 2015
@rasekar2000
Copy link

+1

1 similar comment
@Tronix117
Copy link

👍

@particlebanana
Copy link
Contributor

@particlebanana
Copy link
Contributor

This has been merged along with official support for .select() and will be published in the next version of Waterline (0.12.0).

@didil
Copy link

didil commented Mar 18, 2016

👍

@alexventuraio
Copy link

Excellent I'll love it.
@particlebanana Can you please share an example please, I'm working with sails v0.12.1. I would like to testing it.

@xueenda
Copy link

xueenda commented May 6, 2016

I think there is another bug for this select. I have a query need to populate two objects like

  .populate('partner', { select: ['_id', 'name', 'avatar']})
  .populate('user', { select: ['_id', 'name', 'avatar']})

Here is the response:
{
"partner": {
"name": "Jessica",
"avatar": "http://www.movenoticias.com/wp-content/uploads/2015/11/Jessica-Alba-850x594.jpg",
"id": "56be42c3c282eaecad0f53f3"
},
"user": {},
...
}

The user object isn't populate. But when I remove the select from populating user, it will return the full user object.

Can someone take a look? I am install sails.js and waterline both.

"sails": "~0.12.3",
"waterline": "^0.12.1"

@atiertant
Copy link

@xueenda if you want some help, you should post your models definitions and some sample to reproduce bug... which adapter do you use? did you tryed with an other one?

@billyshena
Copy link

@particlebanana Any news about supporting .select() with a new version of Sails?

@driftman
Copy link

driftman commented Aug 1, 2016

@billyshena i just tried with populate and it is working like a charm v 0.12.3 populate('relation', { select: ['wantedFields', ...] });

@asergey87
Copy link

+1

@DavidVeloso
Copy link

@particlebanana Pls, Any news about supporting .populate({ select: [] }) with Sails v0.12 (sails-hook-orm) ?

@khalilifar
Copy link

Still no support for this ?

@netr0m
Copy link

netr0m commented May 18, 2017

As @driftman pointed out, it works perfectly.
Model.find().populate('relationship', { select: ['attribute', 'attribute', 'attribute']}).exec(...)

@rstrange1992
Copy link

@mortea15 I am using Sails v0.12.13 with the following code
Page.find(query, select).populate('structures', { select: ['structure_id'] }).exec(function(err, result) { and the select in the populate does not work, it seems to be ignored.

@netr0m
Copy link

netr0m commented May 24, 2017

@rstrange1992 as far as I can tell, it doesn't work with MySQL, and perhaps other DBs. I switched to MongoDB, which works fine.

@rstrange1992
Copy link

@mortea15 It doesn't seem to be working with Postgresql either, just so that other people know.

@atiertant
Copy link

it should work with MYSQL, Postgresql and others with the adapter offshore-sql

@anshumanr
Copy link

anshumanr commented Feb 10, 2018

This still does not work for mysql on sails 0.12.14

@luizcarraro
Copy link

same as @anshumanr

@jevho
Copy link

jevho commented Jan 14, 2019

Same for for mysql on sails 0.12.14
Can be waterline updated separately to fix this?

@raqem raqem transferred this issue from balderdashy/waterline May 15, 2019
@abrantes01
Copy link

Hello guys, any news on this issue ?

@tskweres
Copy link

is this supported?

@TahaBoulehmi
Copy link

What happened to this feature? Is it supported? It doesn't work on Sails 1.2.3.

@TahaBoulehmi
Copy link

TahaBoulehmi commented Nov 19, 2019

I made some tests. It works currently only with a population of one-to-many relationship.

I made a pull request to solve the one-to-one relationship issue:
balderdashy/waterline#1613

@marcoshmendes
Copy link

marcoshmendes commented Mar 3, 2020

I made some tests. It works currently only with a population of one-to-many relationship.

I made a pull request to solve the one-to-one relationship issue:
#1613

Any news about it?

@TahaBoulehmi
Copy link

TahaBoulehmi commented Mar 3, 2020

I made some tests. It works currently only with a population of one-to-many relationship.
I made a pull request to solve the one-to-one relationship issue:
#1613

Any news about it?

I'm still waiting for someone from the sails team to review the pull request. If you really need to solve the issue immediately just update lib/waterline/utils/query/forge-stage-two-query.js like this:

balderdashy/waterline@682f1e3

I did it in most of my projects and until now I didn't face any problems with the fix. But as the pull request has not yet been accepted, please make sure to have a backup.

@MattGarnettWelsh
Copy link

Do the limitations in Waterline currently stand at:

  • We can only use select sub criteria in populates for one-to-many relationships.
  • There is no support with this feature for many-to-many.
  • The current version (although written as supported in lib/waterline/utils/query/forge-stage-two-query.js) doesn't work as per comments above for one-to-one relationships?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests