samuelgozi/firebase-firestore-lite

Ordering on single field is failing

Closed this issue · 6 comments

I've the below order specified
orderBy: { field:'text', direction: 'descending' }

The response of the query is 400 error with below message

[{
  "error": {
    "code": 400,
    "message": "The query requires an index. You can create it here: https://console.firebase.google.com/v1/r/project/####/firestore/indexes?create_composite=####",
    "status": "FAILED_PRECONDITION"
  }
}]

When I tried to create the index with the url specified in the error message, the firebase is showing error saying that this is not qualified for composite index since there is only one field.

I already have the single field index for text field, still this error appears.

I checked code and seems like below is causing the problem.

orderBy(option) {
  option.push({ field: { fieldPath: '__name__' }, direction: 'ASCENDING' });
  return option;
},

Don't know why this __name__ order is always added, after removing this __name__ order, I get the valid success response with items ordered on text field as specified.

Could you please check.

@vivekweb2013 Thank you for taking the time to submit a bug report.

The __name__ tag is always added by the official SDK, so I added that too. But the reason they add it seems to be different than what I thought.

I thought that they add it because the database can return results in random order without specifying a filter, so to make sure the results are always returned in the same order they added the __name__(this is how you refer to the name of the document) field as a fallback.

Anyways, I'll take a look at it and hopefully fix it today.

@vivekweb2013 Thank you for taking the time to submit a bug report.

The __name__ tag is always added by the official SDK, so I added that too. But the reason they add it seems to be different than what I thought.

I thought that they add it because the database can return results in random order without specifying a filter, so to make sure the results are always returned in the same order they added the __name__(this is how you refer to the name of the document) field as a fallback.

Anyways, I'll take a look at it and hopefully fix it today.

Okay, thanks

I have a slightly unrelated question. Do you think it will be nicer to replace the directions from ascending/descending to asc/desc?

I have a slightly unrelated question. Do you think it will be nicer to replace the directions from ascending/descending to asc/desc?

Yes, It would be great if you change to the short strings asc/desc instead of ascending and descending

Ok, I found the issue.
I'm explaining it here mostly to document the reason I implement some kind of a "hack" instead of writing a long comment in the code.

So it seems like technically it's not a bug, this is how the database is supposed to work.
However, because the way the database works is a little bit "uncomfortable" then the official SDKs are doing some "hack" to make things easier for the devs and to make the results more consistent.

When making a query, the database might return results in inconsistent order(this is an assumption), and because of that, the official SDKs add a "hidden" orderBy set to order the results by __name__(the document's name) defaulting to ascending order, and I've done so too.

What I didn't know is that if you happen to add to your query a custom field to order by, then the hidden order of the __name__ field changes to match the order of your own fields.

So if you instructed your query to return results in descending order of the field test, then the hidden field __name__ order will be changed to descending too. And that way you won't have to create a new index.

However, it seems like the order of the hidden field will always match the order of the last custom field that you added to the order of the query.

NOTE: Just before posting this, I noticed that the docs specify that this behavior is enforced by the backend anyway, so it is completely safe to completely remove the hidden __name__ order... It never ceases to surprise me how much redundant/over-complicated code the official SDKs have... I should remind myself not to look at their code as a reference.

Anyways, a fix will be published under a new version in a few minutes, and also you will need to replace ascending/descending with asc/desc.

yes right, I think it is safe to remove __name__ as it is handled by backend by default.
I completely agree with you on fact that official SDK is full of redundancy & documentation issues.
Thanks for the fix I'll get the latest update.