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
Function for inner and outer joins #688
Comments
Hi @michael-felden, so can you explain whether there are edge cases that your rework supports that the eqJoin doesn't? Or how is the "result" more like what I would expect from sql. It's too late to change the eqJoin prototype method signature and functionality unless we just add a new param(s)... and the method would have to support full backwards compatibility and unit tests. Current eqJoin hangs off resultset prototype and has implicit left data being the results in the resultset. As for the right data, eqJoin can accept raw data array, a collection or resultset. I also believe it supports left outer join and cross join. Let me know your usability concerns or if this is intended to be cleanup of internal code within that method. |
To demonstrate the difference here with eqJoin:
In this resultset Mary got only a dog. The cat is lost! Why not the other way round? How can I know? Or am I missing something? |
Sorry, I have not had much time to research this until now... I do see now the functionality difference you are describing and agree the current eqJoin functionality supports only single value 'match' on right. Perhaps we could add an 'outerJoin' or 'leftOuterJoin' method that behaves with the functionality you describe. Feel free to submit a pull request to incorporate your method into resultset class if you are still interested. We should probably have a unit test which demonstrates your expected usage. npm install and npm run test can be used locally to verify. Ideally it would be nice to support the same variable 'right' side where you can pass in Resultset, Collection, or doc array. If you do not feel comfortable with that I can add to my todo list but that might take a while to get to. |
For my current project there was a need for a multi collection join like in SQL. So I wrote a little class for that. That kind of solution I would engage in if could find some spare time. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
I think your code on the OP is broken, because Anyways, fixed and tested -- LokiJS-Forge/LokiDB#167 |
The result is what you would expect from sql.
Small changes to eqJoin would suffice.
What do you think?
The text was updated successfully, but these errors were encountered: