horizon
horizon copied to clipboard
rethinkdb
Add a `.without` or `.pluck` term to the horizon query language
Numerous users have reported issues with sending large documents over the wire. The nosql-y way to do things is to jam as much into a table as possible since only single document actions are atomic, but we need to give them relief from sending everything over the wire.
Problem is... we currently have an invariant where everything streamed to horizon is a full document with an id etc. I think we should probably allow plucking, but forbid plucking out the id field so it can always be mapped back to a db document. This would still allow people to shoot themselves in the foot by accidentally writing back a plucked document (and wiping out fields) but that sounds like an app bug, not something we should prevent at the platform level.
Would it be unreasonable to tag plucked documents (say by adding something special to the id field, which is supposed to be opaque anyway), and then when people write them back with replace we don't remove fields that weren't plucked out initially?
Is there anyway we could merge these into a single command for terseness?
Would it be unwise having an API similar to $project?
horizon('vehicles').project({
'year': true,
'brand': true,
'previous_owners': false // An avoided giant array of documents
}).fetch().subscribe((result) => {
console.log(result);
})
@mlucy I thought about this, and it's definitely do-able (we have Symbols in js now). I sort of fell on the side of it being too much work to keep a developer from making a programming mistake. If you pluck a large document and then use .replace instead of .update, that's just a regular oversight kind of bug. I don't think preventing it is probably worth a ton of trickery in horizon unless it becomes a big problem
Horizon currently doesn't have an equivalent to ReQL's update, does it?
Did we decide not to have update because of concerns about optimistic updates being harder to implement if you don't have the full new document when doing a write?
@dalanmiller I think the project style makes sense. Especially since we probably want to move towards the graphQL style of query where you're explicit about which fields you want to get back. Having worked with rest apis in the past, it's nice to know exactly what's fields are coming in your query and project
@danielmewes We have update, (https://github.com/rethinkdb/horizon/blob/next/client/src/ast.js#L316) it appears to be missing from the docs though!
@dalanmiller Maybe I don't understand how $project works, but it seems to me that it's identical to our pluck, just with a more verbose syntax. The only field you can suppress in $project is the _id field, since that's otherwise included by default.
It might be reasonable to just add pluck, it definitely gets used way more than without.
Ahh yup @danielmewes, the only thing I wish it had is the use case where I only provide falses, I'd want .pluck to act as a filter rather than having to explicitly list out everything I do want.
This is useful where I want everything but one field, and in this case .pluck is more verbose than $project. Everywhere I want one or a couple fields out of many in the document .pluck is superior.
@dalanmiller I don't think $project supports that. If you only specify falses, you'll get an empty document (except for _id if you don't explicitly false that too).
I agree with @mlucy that pluck probably covers most use cases.
I can't come up with a good syntax that combines both pluck and without into one command right now. We could do what $project does with the additional property that if all specifications are false, then we behave like without. But that might be slightly surprising, and also makes the syntax in the pluck case more annoying.
One thing I think we should change from raw pluck is the ability to force fields to exist even if they aren't in the original document. So:
.pluck('a', 'b', 'c')
If field b doesn't exist, it should be added
I stand corrected, I meant projection in find or findOne queries.
The
can be one of the following include or exclude values:
- 1 or true to include. The findOne() method always includes the _id field even if the field is not explicitly specified in the projection parameter.
- 0 or false to exclude.
The projection argument cannot mix include and exclude specifications, with the exception of excluding the _id field.
If we change the functionality of .pluck at all I think we should rename it to not be confusing. "Pluck" by itself sounds like you are grabbing specific fields from the whole. If we do any modification or assurance that a field exists, I'm in favor or renaming to .project or maybe .shape?
@deontologician how do you come up with a default value for b without some sort of schema definition on the collection?
@marshall007 I guess we could either do null or something else... Hmm. Maybe I'll take this back. I think pluck is probably explicit enough and we can deal with schema like issues separately. without is the one where you're left wondering which fields will come through, so it's probably fine
@deontologician I think you have the right idea though, something like .pluck('a', 'b', 'c').defaults({ b: true }) might be pretty handy.
The thing is, when a field is missing in JS evaluating it defaults to undefined, so I don't see any benefit of automagically adding the missing fields, unless it's with something like @marshall007 proposed, with a separate defaults method chained.
I'm rather new here -- but -- the idea of introducing a new .defaults() method seems .. wrong? ES6 introduced default values for various things (most notably function arguments) .. it would seem like a good idea, IF we want default values, to follow somehow in the same footsteps? Or at least in the same vein .. Just seems like we can figure out a syntax for .pluck() that would allow the defaults to be specified within that without having to add a new chained method ..
Maybe allow either an array of attributes or object set of them and in the object set have the default? So ..
.pluck({ a : "default_string", b : 2 , c : "default_new_string", d : undefined } ) ... or .pluck( 'a', 'b', 'c') which would default to undefined ??
Just an idea ..
I like the conciseness of @sjmcdowall's {} style defaults but that seems to mean explicitly requiring all values to have a default so I prefer @marshall007's idea of adding the defaults.
That said, a pluck, and with defaults, is definitely valuable. @deontologician's initial observation is spot on with large documents.
Once plugins land I know I'd want to figure out how to lodash style "deep gets" so we could do
.pluck('a.foo', 'b.bar', 'c[2].baz')
