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

Projections #1310

Merged
merged 16 commits into from
Mar 17, 2016
Merged

Projections #1310

merged 16 commits into from
Mar 17, 2016

Conversation

particlebanana
Copy link
Contributor

DO NOT MERGE

This PR represents the initial work for projections on both the parent and child criteria. It finally brings official support for select and adds a .select() method to the deferred object. It also adds support for using select inside of criteria in a populate.

One thing to note is that any primary keys and foreign keys used in a join will always be selected. This is unavoidable due to the way we currently build up join queries and combine results in cross-adapter populates.

The first thing that needs to happen is tests in Waterline Adapter Tests to ensure things won't break across the officially supported adapters. There is a corresponding PR in Waterline Sequel that needs to be brought in for the SQL adapters and I'm sure there will be some slight tweaks that need to happen in the Mongo adapter.

Example Use:

// Deferred Object Style
User.find()
.select(['name', 'age'])
.where({
  age: {
    '>': 21
  }
})
.exec(function(err, users) {});
// Populate projections
User.find()
.select(['name', 'age'])
.populate('pets', { select: ['breed', 'name'] })
.exec(function(err, users) {});
// Callback style
User.find({ select: ['name', 'age'] }, function(err, users) {});

attributes = [attributes];
}

this._criteria.select = attributes;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should append to existing select and not replace it. Will allow to call select twice.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good call!

@shamasis
Copy link

For the populates, especially joins - WL-schema also needs to honour the select arrays. FYI. :-)

@particlebanana
Copy link
Contributor Author

@shamasis I have another PR for wl-sequel if that's what you meant. If not what does wl-schema need for this?

@sulihuang
Copy link

hello , now i can use .selet() and .populate('',{select: []}) ?

@cjy37
Copy link

cjy37 commented Mar 3, 2016

+1

@mikermcneil
Copy link
Member

One thing to note is that any primary keys and foreign keys used in a join will always be selected. This is unavoidable due to the way we currently build up join queries and combine results in cross-adapter populates.

👍

@particlebanana looks great man.

To get reaaallllll specific for a second, what happens if I:

  1. .select('*') / .populate('direwolves', { select: '*' })
  2. .select(['*']) / .populate('direwolves', { select: ['*'] })
  3. .select(['*', 'coatColor']) / .populate('direwolves', { select: ['*', 'coatColor'] })
  4. .select([]) / .populate('direwolves', { select: [] })

My thoughts are:

  1. not valid-- call cb with invalid criteria error (if we're feeling fancy later we can expand to ['*'] automatically once this is well-tested)
  2. not sure on this, because of the way it could affect driver-level stuff. I think we need to figure it out there first maybe.
  3. depends on 2 (but my intuition is to call cb with error)
  4. interpreted as only fetching the primary key and any necessary foreign keys.

@particlebanana
Copy link
Contributor Author

@mikermcneil started building out tests for projections in the adapter test suite and added the following checks.

balderdashy/waterline-adapter-tests#106

.select(['*']) / .populate('direwolves', { select: ['*'] })

Returns all fields for the record. Same as select * from 'foo'


.select('*') / .populate('direwolves', { select: '*' })

Gets normalized into select: ['*'] and returns all fields. This is done in the criteria normalize function.


.select(['*', 'coatColor']) / .populate('direwolves', { select: ['*', 'coatColor'] })

Gets normalized into select: ['*'] and returns all fields. If a star is used at all you will get all fields.


.select([]) / .populate('direwolves', { select: [] })

Only returns the primary key for the record.

particlebanana added a commit that referenced this pull request Mar 17, 2016
@particlebanana particlebanana merged commit 309ae26 into master Mar 17, 2016
@particlebanana particlebanana deleted the projections branch March 17, 2016 21:57
@Salakar
Copy link

Salakar commented Apr 11, 2016

Is this actually removing the unnecessary fields on waterline itself by modifying the returned data or is this doing projection queries on the db side (i.e mongo)?

@particlebanana
Copy link
Contributor Author

@Salakar this supports adapter level projections so it's up to the adapter to build them correctly. The latest version of Waterline adds this support and it's implemented in the sql adapters as well as mongo I believe.

@luislobo
Copy link
Contributor

I have an other case: if .select(false) or .select(undefined) it should just be treated as '*', that is, ignore the select. I have a caching function that could benefit of a select statement: CacheService.find(Model, criteria, populate, populateCriteria, select, sort). I could call it like: CacheService.find(User, {email:'luislobo@gmail.com'}), or CacheService.find(User, {email:'luislobo@gmail.com'},'groups',false,['email','name','groups'])

What do you think? From the code, I see that undefined or false is not handled. https://github.com/balderdashy/waterline/blob/master/lib/waterline/utils/normalize.js#L193

May be it's handled somewhere else?

@particlebanana
Copy link
Contributor Author

@luislobo we should add some tests for those cases first. But that sounds good.

@luislobo
Copy link
Contributor

luislobo commented May 2, 2016

I can create a PR for this

@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 population of one-to-many relationship.

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

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

Successfully merging this pull request may close these issues.

8 participants