meteor / meteor-feature-requests

A tracker for Meteor issues that are requests for new functionality, not bugs.
Other
89 stars 3 forks source link

Minimongo: make hint: $natural available on client, too #413

Open jankapunkt opened 3 years ago

jankapunkt commented 3 years ago

As far as I can see from the docs, there is no hint: $natural option available on the client.

Is there a reason why? From my understanding it simply reverses the search order from latest to oldest. If I get this right we simply would have to support a optional backwards count in this line in minimongo/cursor.js:

    this.collection._docs.forEach((doc, id) => {
      const matchResult = this.matcher.documentMatches(doc);

      if (matchResult.result) {
        if (options.ordered) {
          results.push(doc);

          if (distances && matchResult.distance !== undefined) {
            distances.set(id, matchResult.distance);
          }
        } else {
          results.set(id, doc);
        }
      }

      // Override to ensure all docs are matched if ignoring skip & limit
      if (!applySkipLimit) {
        return true;
      }

      // Fast path for limited unsorted queries.
      // XXX 'length' check here seems wrong for ordered
      return (
        !this.limit ||
        this.skip ||
        this.sorter ||
        results.length !== this.limit
      );
    });

However, I am not deep enough into this file yet to see potential sideeffects. Anyone with experience on minimongo internals here?

filipenevola commented 3 years ago

@jankapunkt I believe it was not included because nobody asked.

radekmie commented 3 years ago

I believe I have some experience with minimongo internals. Let's start with the MongoDB note on $natural first:

The $natural parameter returns items according to their natural order within the database. This ordering is an internal implementation feature, and you should not rely on any particular ordering of the documents.

And the natural order definition:

The order in which the database refers to documents on disk. This is the default sort order. See $natural and Return in Natural Order.

Now, this is quite unspecified for minimongo, as documents have no arrival nor publish date. It could be added but I see no real value in it - it'll impact all minimongo users and only a few would benefit from it. This means we could simply .reverse() the result as @jankapunkt suggested:

     if (this.sorter) {
       results.sort(this.sorter.getComparator({distances}));
-    }
+    // Unspecified `$natural` and `$natural: -1` are equivalent to no reordering.
+    } else if (this.naturalHint === 1) {
+      results.reverse();
+    }

Other things to do: correct this test, this piece of documentation, and this note.


What is your use case, @jankapunkt? I've never actually used $natural but a createdAt (or _id of ObjectID type) and an index to achieve this behavior instead.

jankapunkt commented 3 years ago

I am using to retrieve documents from the newest without having to use sort. So basically I get the latest n Docs via $natural set to -1 and limit to n. In my naive thinking I thought that this is way performant than sorting the whole collection.

Edit: this mostly happens on the server but I wrote some isomorphic Code where splitting Server/Client because of one line seems to me very inefficient.

radekmie commented 3 years ago

@jankapunkt I'd still suggest an index then. $natural won't use indexes except _id, and top N is one of the best use cases of secondary indexes. In other words, $natural wins if and only if there is no query or the query matches only _id field. Theoretically, it may be slower for minimongo, as it works in filter-sort-slice manner (no "fancy" top N implementation), but if it's a problem, there are probably too many documents in the browser anyway.

Also, keep in mind that $natural order may not be preserved for imported data, like test servers initialized with live data dump.