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

$lookup as reference #63

Closed
wants to merge 1 commit into from
Closed

Conversation

Redsandro
Copy link

$lookups are safe. No need to clone.

As @kofrasa said:

forgot to remove the clone. must fix

$lookups are safe. No need to clone.

As @kofrasa [said](kofrasa#61 (comment)):
> forgot to remove the clone. must fix
@codecov-io
Copy link

codecov-io commented Sep 6, 2017

Codecov Report

Merging #63 into development will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           development      #63   +/-   ##
============================================
  Coverage        97.53%   97.53%           
============================================
  Files               52       52           
  Lines             1178     1178           
============================================
  Hits              1149     1149           
  Misses              29       29
Impacted Files Coverage Δ
lib/operators/pipeline/lookup.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fb3650...b5be652. Read the comment docs.

@kofrasa
Copy link
Owner

kofrasa commented Sep 6, 2017

I have some changes ready to go that do more optimizations. Basically, iterating the indexes can also be removed by putting the objects put in the hash buckets directly.

@Redsandro
Copy link
Author

Redsandro commented Sep 6, 2017

Ok. I can remove this?

This was an attempt to help speeding an npm release without the clone along for me to play with. 😎
I didn't know you were already working on more optimizations.

@kofrasa kofrasa closed this in 332acb0 Sep 6, 2017
@Redsandro Redsandro deleted the patch-1 branch September 7, 2017 09:42
@Redsandro
Copy link
Author

Thanks!

One question: hash[k] should be an object?

@kofrasa
Copy link
Owner

kofrasa commented Sep 7, 2017

The mongo docs do not specify what happens when the join field is not matched. Returning a collection seems reasonable.

@Redsandro
Copy link
Author

Redsandro commented Sep 7, 2017

You are correct. My mistake. I have tunnel vision. I often match one and $unwind.

The good news, if I am correct, is that I can use a custom operator to create $lookupOne based on $lookup, that always adds a single document to a field, so I don't have to $unwind.

@kofrasa
Copy link
Owner

kofrasa commented Sep 7, 2017

👍
I think you will discover that custom operators will afford you all the flexibility you need.

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.

3 participants