Allow queries based on nested fields

[deontologician]

Right now we only build indices/allow querying based on top level fields. This is a fairly arbitrary restriction, we should see if we can support nested fields in an elegant way that doesn't make the api too complicated

[marshall007]

I started looking into this. Currently we generate index names by joining on _ and thus have to escape _ and \ characters. Inevitably, in order to support nested fields we are going to have to come up with an additional escaped character (probably .).

Instead of doing that, I think as a first step we should consider generating index names by serializing the fields to JSON instead of joining on _.

Fields	Current	Proposed
[ 'foo', 'bar' ]	foo_bar	{"foo":true,"bar":true}
[ 'foo_bar', 'baz' ]	foo\_bar_baz	{"foo_bar":true,"baz":true}
[ 'foo.bar', 'baz' ]	N/A	{"foo":{"bar":true},"baz":true}

This makes the secondary index names much easier to consume (could even be done in ReQL!), helps us avoid future corner cases with escaping field names, and provides a bit more flexibility over _ delimited strings.

[marshall007]

We would either need to use something like json-stable-stringify or go ahead and make key ordering deterministic in RethinkDB as was briefly discussed in rethinkdb/rethinkdb#4636 (comments).

[deontologician]

Ah, that's pretty interesting

[Tryneus]

@marshall007 - I don't think we need deterministic key order, rather we need to know the order that the fields go into the index (this is important for matching queries to indexes on range gets). Therefore, I propose a slight modification to your suggestion:

Fields	Current	Proposed
[ 'foo', 'bar' ]	foo_bar	{"foo":0,"bar":1}
[ 'foo_bar', 'baz' ]	foo\_bar_baz	{"foo_bar":0,"baz":1}
[ 'foo.bar', 'baz' ]	N/A	{"foo":{"bar":0},"baz":1}

As you can see, instead of always being true, the value is now the offset of that field in the compound index. This would be completely deterministic, it would allow us to perform index matching, and it's slightly more concise and easier to read.

[mlucy]

@Tryneus -- I think you still need deterministic key order so that you always try to read from the index {"foo": 0, "bar": 1} instead of {"bar": 1, "foo": 0}.

[Tryneus]

@mlucy - the index is only created once, and we save the name in our internal structures we use for matching queries to indexes - we aren't "guessing" the index name on the fly.

[marshall007]

@Tryneus I like your proposal and I think we should do that if nothing else but for clarity. However, I see the ability to "guess" the index name on the fly as an advantage. The fewer internal tables and data structures we have to maintain the easier #120 becomes.

[32graham]

It looks like PR #650 says it resolves this issue. Does that mean I can now query on a nested property with .findAll?

I'm wanting to do something like this:

this.hz('appointments').findAll({ stylist: { id: this.stylistId }})

[marshall007]

@32graham IIRC this was released in 2.0 so that should work fine now. Let us know if you run into any issues.

[32graham]

@marshall007 I don't think finding by nested field is working.

My appointment data looks like this (I have added the stylistId at the root of an appointment to work around this issue but I would rather not have it there.

[
  {
    "id": 100,
    "startDateTime": "2016-01-01",
    "duration": 20,
    "stylist": {
      "id": 200,
      "name": "Sue",
    },
    "stylistId": 200
  }
]

When I try to filter to they nested stylist.id I don't get any results or errors. When I use the stylistId that is at the root of an appointment it works as expected.

this.hz("appointments")
  .findAll({ stylist: { id: this.stylistId }}) // prints nothing
  //.findAll({ stylistId: params.id}) // prints "something good"
  .fetch()
  .subscribe(
    items =>  console.log("something good"),
    error => console.log("something bad"));

[deontologician]

We added syntactical support in indexes, but it's not actually implemented as a feature.

On Sun, Nov 6, 2016, 13:59 Josh Graham [email protected] wrote:

@marshall007 https://github.com/marshall007 I don't think finding by nested field is working.

My appointment data looks like this (I have added the stylistId at the root of an appointment to work around this issue but I would rather not have it there.

[ { "id": 100, "startDateTime": "2016-01-01", "duration": 20, "stylist": { "id": 200, "name": "Sue", }, "stylistId": 200 } ]

When I try to filter to they nested stylist.id I don't get any results or errors. When I use the stylistId that is at the root of an appointment it works as expected.

this.hz("appointments") .findAll({ stylist: { id: this.stylistId }}) // prints nothing //.findAll({ stylistId: params.id}) // prints "something good" .fetch() .subscribe( items => console.log("something good"), error => console.log("something bad"));

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rethinkdb/horizon/issues/444#issuecomment-258714112, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAFVpCMyyN4zk3bKWBvb8sXNXkSs04qks5q7k2tgaJpZM4IipAt .

[32graham]

@deontologician For now is the best workaround just to put any id's you need to filter on at the root of the object?

Is this a feature the team would like to have added? Is it doable by a project outsider (someone not very familiar with the project)?

[deontologician]

I would say yes, that's the best workaround. We did want to add it, though I don't think it's the highest priority at the moment. PRs are welcome, though it may be a big feature given that it touches many parts of the code.

On Sun, Nov 6, 2016, 14:15 Josh Graham [email protected] wrote:

@deontologician https://github.com/deontologician For now is the best workaround just to put any id's you need to filter on at the root of the object?

Is this a feature the team would like to have added? Is it doable by a project outsider (someone not very familiar with the project)?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/rethinkdb/horizon/issues/444#issuecomment-258715197, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAFVlbN48eUTojZaQIhm1Y84odH79K4ks5q7lFxgaJpZM4IipAt .