-
Notifications
You must be signed in to change notification settings - Fork 5.7k
SERVER-5044: accumulator for stdev #409
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
Conversation
Hi base698 (Justin?), Before we can accept any pull request, we have two main requirements. (1) reference a SERVER ticket, (2) make sure you've signed the contributor agreement. This is explained in CONTRIBUTING.rst in the root of our source tree. I see you already have the SERVER ticket taken care of, so that's great! Now we need the contributor agreement. The form is at http://www.10gen.com/legal/contributor-agreement Even though it's not "required" in the form, please make sure you specify your GitHub username. This will help us verify compliance quickly in the future. Thank you for contributing to MongoDB, and I apologize for the delay in getting back to you. |
I did the agreement. Thanks! |
Thanks Justin! I see the updated contributors agreement. Looks good now. |
Hi Justin, sorry for the delay. Looking at your diff I see some a style issue:
Please fix these per our style guide (http://www.mongodb.org/about/contributors/kernel-code-style/). You can have Git highlight leading tabs by setting
Mathias (our aggregation lead) also had some comments about the code itself. Did you notice the Gist link in SERVER-5044 to an algorithm for computing avg, variance, stddev in one pass? I think this is the approach he is looking for. |
I also wanted to do it in one pass, and think I asked about that in the JIRA issue. Missed that link. I can change it to no literal tabs easy. How many is default for you guys? Will look at implementing it as the gist. |
Our kernel code style guide specifies 4 spaces per indentation. http://www.mongodb.org/about/contributors/kernel-code-style/#basics |
Hey Justin, I see that you pushed some whitespace fixes about a week ago, but I'm not sure what these other merge commits are. Maybe you wanted to rebase instead against master? Also, are you still thinking of reimplementing this with the other algorithm per the Gist? |
Hi Justin, It's been a month since we last discussed this issue. I am going to close this pull request due to inactivity, but if you want to reimplement per our discussion above, just reopen the pull request and submit a new implementation. Or open a new pull request, whichever is easier. SERVER-5044 will stay open until we get an implementation of $stdDev and friends. |
Improve the error message if no columns are specified to a projection. closes mongodb#409.
This is how we try to indicate in example code that error handling is needed without cluttering up the code too much. refs mongodb#409
…eviously, column groups added after all columns appeared in a column group would not be populated correctly. refs mongodb#409
…e of population data, canonical in database textbooks. refs mongodb#409
* allow empty column lists in projection cursors; * do another pass over the schema documentation and example code; * add docs for the variable-length argument lists for WT_CURSOR:{g,s}et_{key,value}. closes mongodb#409
We still rely on C language type promotion for values passed in, including careful handling of long vs int values. refs mongodb#409
Trying to enable $stdev for aggregate. Seems to work, but needs tests and some clean up.
https://jira.mongodb.org/browse/SERVER-5044