neumino / thinky

JavaScript ORM for RethinkDB
http://justonepixel.com/thinky/
Other
1.12k stars 128 forks source link

Store table indexes somewhere on the model #59

Closed mshick closed 10 years ago

mshick commented 10 years ago

I'd like to be able to create a dumber query interface, where filters can be built by field names, without the implementer necessarily knowing if the field is indexed or not.

Without a list of indexes stored somewhere on the model, I have no way of efficiently building a query like .orderBy() that can swap between .orderBy('createdAt') and .orderBy({ index: 'createdAt'}) depending on how that field was established in the model.

A naive implementation might just store the names that come into Model.ensureIndex(), a more sophisticated might store the results of an r.indexList() query.

neumino commented 10 years ago

You basically want orderBy to automatically use an index if one is available?

mshick commented 10 years ago

In this case, yes, though another use would be to swap a filter for a more efficient getAll() if the incoming params happened to be querying a field with an index.

It's similar to what the jugglingdb rethink adapter does here: https://github.com/fuwaneko/jugglingdb-rethink/blob/master/lib/rethink.js#L611-L619

I suppose it could be done auto-magically by Thinky, or, the necessary info could just be provided for an implementer (such as myself).

If Thinky were to abstract a little more away from Rethink, I could imagine a .where() method that dynamically generates a series of filters or a getAll() plus some filters, from some structured object (like what that adapter is doing).

Model.ensureIndex('published');
Model.where({ published: true, deleted: false })
---->
Model.getAll(true, {index: 'published'}).filter({deleted: false});
neumino commented 10 years ago

@mshick -- I'm a little reluctant in changing the behavior of the method from the driver. Adding a new method doesn't seem like a really good idea too (better than changing the behavior of the methods from the driver though).

This may make it in the official driver with https://github.com/rethinkdb/rethinkdb/issues/2356 @mshick -- feel free to chime on the rethinkdb issue.

mshick commented 10 years ago

Sure, I was just brainstorming.

Really the best solution for me would also be the most minimal -- simply storing the index names on the model so I have a better idea of what I'm working with later in the lifecycle.

My quick hack has been to provide a list of indexes in my Model module, iterate that list with Model.ensureIndex(), and then store that list as Model._indexes.

With this approach though, I worry about collisions with your own work, and the fact that this list isn't quite as bulletproof as running r.table().indexList() and storing the results (which might be the next step for my hack).

neumino commented 10 years ago

I can run table.indexList() when a table is ready, and add the indexes used by ensureIndex. If users don't manualy create indexes outside the code using thinky, it should be safe.

The only thing I'm kind of worried is that people may use thinky with orderBy("field") and at some point use a raw ReQL query with the driver and use the same expression, but not realize that they won't be using an index.

Somehow after a night of sleep, it doesn't that unreasonable now. I'll think a little more about it.

mshick commented 10 years ago

More art than science for sure. I totally understand you wanting to stay close to the rethink project, though as Thinky develops maybe the value-add will be the “sugar” it can lay on top of rethink itself, like you’ve already done with relations.

On May 7, 2014, at 12:00 PM, Michel notifications@github.com wrote:

I can run table.indexList() when a table is ready, and add the indexes used by ensureIndex. If users don't manualy create indexes outside the code using thinky, it should be safe.

The only thing I'm kind of worried is that people may use thinky with orderBy("field") and at some point use a raw ReQL query with the driver and use the same expression, but not realize that they won't be using an index.

Somehow after a night of sleep, it doesn't that unreasonable now. I'll think a little more about it.

— Reply to this email directly or view it on GitHub.

neumino commented 10 years ago

Release in 1.13.11.

filter and orderBy can be automatically optimized.