Filtering not working at sub-queries level

[roosmaa]

When working with more advanced queries (QueryCombination), it seems that the filter property is only being applied at the final results level, instead of sub-query level as the type definitions would suggest. There's no mention of this restriction on filter in the docs either.

For example, the following query wouldn't do any filtering at all:

myIndex.search({
  combineWith: 'OR',
  queries: [{
    queries: ["term1"],
    filter: (doc) => doc.type === 'foo',
  }, {
    queries: ["term2"],
    filter: (doc) => doc.type === 'bar',
  }],
});

[lucaong]

Hi @roosmaa , thanks for opening this issue and for your pull request.

You are absolutely right that the fact that filter is only considered at the top level of a QueryCombination is never mentioned in the docs. It should have been documented, because filter operates on the final results, not on the RawResult of each sub-query, so in principle it only makes sense at the top level. Adding this possibility for sub-queryes means to compute "final" results even for each sub-query, only to call filter, which might add some non-negligible overhead.

That said, you implemented it in a way that makes sense: you only incur in the overhead if a filter was specified within the sub-query, otherwise things behave like before. This might be a good way to go.

Allow me some time to think about all the implications, and how to best proceed :) In the meanwhile thanks a lot for your contribution.

[roosmaa]

Some more food for thought for you.

I think currently the top level SearchOptions (from search(_, opts)) are passed down the call-stack and merged into the individual queries. As it is now, in my PR if the top-level filter was specified it would also execute down in the executeQuerySpec.

There could be a distinction made for the filter passed in as SearchOptions vs filter in the QueryCombination. The top-level one passed in as SearchOption would be executed at search() level (for the final results), and the QueryCombination filters would be executed at executeQuerySpec level (for the intermediate ones). That would require a tiny change of not passing down the filter from the SearchOptions to the underlying executeQuerySpec calls. (And of course properly documenting this nuance/distinction somewhere)

[lucaong]

Yes, while all other options cascade from top level to nested levels if not specified, the filter option acts differently and should probably not cascade. As in: it only needs to be applied at the level where it is specified (as filtering both at the sub-query level and also at the query level would be useless duplication). This is true at any level, should one have a nested query combination of more than two levels.

In other words, at any level in the QueryCombination tree, if a query has a filter and several sub-queries without a filter, the sub-queries should be executed, their results combined, and then the filter should be applied (as opposed to each sub-query executing the filtering before combination of the results).

This avoids duplicated computation, and as far as I can see would also be completely backward compatible (as in, any case where the filter was specified at the top level, which is the only supported case at the moment, would behave exactly the same, both in terms of results, and in terms of performance).

[roosmaa]

Sounds good 👍 I updated the PR to do the filtering at QueryCombination-level in executeQuery (instead of executeQuerySpec).

[roosmaa]

@lucaong just a small nudge (if/when you have time)