search

TylerBrock / mongo-hacker

MongoDB Shell Enhancements for Hackers
tylerbrock.github.io/mongo-hacker
MIT License
1.79k stars 235 forks source link

New "count doc(ument)s" shell helper/shell command #128

Closed pvdb closed 9 years ago

pvdb commented 9 years ago

@TylerBrock - this pull request is basically to see if there's an appetite for my "count documents" shell helper / shell command, which I find myself using on a very frequent basis... the attached screenshot explains better than words can how it works, but it is akin to the "show collections" command, and borrows heavily from it for its implementation.

If you think this shell command is a useful addition to mongo-hacker then please accept the PR, or else I'm willing to refactor the code a bit first, as it can definitely be DRY-ed up quite a bit (but I thought I'd check first to see if you're interested in it)

Also, let me know if you have a better suggestion for the name ("count collections" maybe, even though that's not quite correct, or else "count rows" but that's probably too SQL-ish :smile:)

mongo_hacker_count_documents

PS - notice how the "count documents" command - on purpose - skips the system collections... we could have separate "count system documents" and "count application documents" commands, but again it's probably best to see if there's an appetite for this first... thanks!

TylerBrock commented 9 years ago

Ohh, I really like this. Thank you! Nice work! I think skipping the system collections is the appropriate behavior. The command name seems fine to me as well. Cool.

Ideally we could merge this into the show collections display where it would look something like: <collection name> size: xx.xxMB/yy.yyMB documents: 1,234

I don't think it would add too much time to show collections itself for small numbers of collections as there is no filter being applied BUT it would still add significant cost to displaying large amounts of collection data (where people have hundreds or thousands of collections). So, that being said, I think this is the correct approach.

My one question is, was there no commify() like function already in the mongodb shell codebase? It wouldn't surprise me if it was missing but just wanted to see if you had looked.

TylerBrock commented 9 years ago

Also, can you add an entry to the enhancements section of the README.md? Please give yourself some credit in there if you do.

pvdb commented 9 years ago

Hey @TylerBrock

Great to hear that you like the idea!

Ideally we could merge this into the show collections display where it would look something like: <collection name> size: xx.xxMB/yy.yyMB documents: 1,234

That's a possibility I considered as well, but the attached screenshot shows a possible extension to the count documents command that I'm currently working on, ie. retain state and also display the delta since the last count for each collection...

I think that if that extension every makes it in, adding it all to the show collections command may overload the output of that command with too many data points, making it much harder to grok...

Thoughts?

count_with_state

pvdb commented 9 years ago

My one question is, was there no commify() like function already in the mongodb shell codebase? It wouldn't surprise me if it was missing but just wanted to see if you had looked.

I didn't look into the mongodb shell codebase, but I did however try my luck with Number.prototype.toLocaleString() which is supposed to do exactly that, ie. commify a number, but I couldn't get it to work properly in the mongodb shell for some reason.

Now that I know you're willing to accept the PR though, I'll have another go at it, as it is indeed a bit of an eyesore :smile:

pvdb commented 9 years ago

Also, can you add an entry to the enhancements section of the README.md? Please give yourself some credit in there if you do.

Done in 130af08 ... how does that look?

TylerBrock commented 9 years ago

Looks great. Why don't we start with this and merge it in?

pvdb commented 9 years ago

Hey @TylerBrock,

Looks great. Why don't we start with this and merge it in?

Sounds good... I looked at the mongo source code, as you suggested, but couldn't immediately find something that I could use instead of my handcrafted commify() function, but I'll keep looking and will issue a separate pull request if I do find something suitable.

:shipit:

pvdb commented 9 years ago

W00t w00t! Cheers, @TylerBrock! :smile:

TylerBrock commented 9 years ago

Thank you for contributing. I'll probably cut the next release soon.