Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow a query to sort based on an object value field #4805

Closed
acinader opened this issue Jun 1, 2018 · 10 comments · Fixed by #4806
Closed

Allow a query to sort based on an object value field #4805

acinader opened this issue Jun 1, 2018 · 10 comments · Fixed by #4806

Comments

@acinader
Copy link
Contributor

acinader commented Jun 1, 2018

Currently, if one tries to sort a query on a field in an object field, parse generates an error:

Type: Parse\ParseException
Code: 105
Message: Invalid field name: trendRankTopic.popular.

due to the check in the database controller here:

if (!SchemaController.fieldNameIsValid(fieldName)) {

This is not the same, though related to: #2113

An example would be an object:

{ sortField: { value: 10 } }

with a Query:

new Parse.Query('Test').addAscending('sortField.value').first()

generates the above error

@flovilmart
Copy link
Contributor

What error would do you expect? Or would you expect sorting to work on objects?

@acinader
Copy link
Contributor Author

acinader commented Jun 1, 2018

one sec and I'll open the pr with the failing test and my proposed solution :).

@flovilmart
Copy link
Contributor

Even better :)

@acinader
Copy link
Contributor Author

acinader commented Jun 7, 2018

@flovilmart any thoughts on #4806

I'm currently running our production on that branch which ain't a great long-term solution.

Anything I can explain as to why its an issue in the first place
or the solution I chose and why.

Some things I'd appreciate your considering: Is there a use case that this change would cause a breakage? is there a use case where my change will allow an introduction of 'bad schema' that could cause mayhem? Is there coverage I could write to test?

And finally, any suggestion on who I might ping to look at why Postgress is failing my test and what a solution might be?

@flovilmart
Copy link
Contributor

I don’t think it’s a bad idea to introduce sorting on nested keys, Postgres support could come later. It can be gated with it_only_db instead of a simple it.

I don’t believe it is something we may regret in then future, however, I’m quite not understanding the change / i believe it’s not the proper resolution :)

@acinader
Copy link
Contributor Author

acinader commented Jun 7, 2018

I'll gate the test and add more explanation to the pull request. I agree that it is low risk. Having looked at it, I do think its the right solution, however, I'm super mindful of the fact that my ability to reason about that code is pretty limited :). Thanks!

@flovilmart
Copy link
Contributor

CAN we add a small comment as for the change then in the code? So anyone that stumbled upon it is able to get a good idea of what’s going on?

@acinader
Copy link
Contributor Author

acinader commented Jun 7, 2018

done.

@wret
Copy link

wret commented Jul 21, 2018

Sorting based on object still not working on version 2.8.2.

@NicksonYap
Copy link

NicksonYap commented Oct 2, 2018

@wret
It is enabled in 2.8.4: "Allow sorting an object field (#4806)"
https://github.com/parse-community/parse-server/blob/2.8.4/CHANGELOG.md

However, it does not seem like it's supposed to order by related object's field

Tried this and has totally no effect:

      query.include('subzone');
      query.include('subzone.zone'); 
      query.addAscending('subzone.zone.title')  //no effect on result, no error
      query.addAscending('subzone.title') //no effect on result, no error
      query.addAscending('title') //sorts by title

@flovilmart @acinader
After upgrading from 2.8.2 to 2.8.4 query.ascending() no longer reports any error if the field does not exist
is it because of this sort by object field feature?

@parse-community parse-community deleted a comment from CodeWithOz Aug 2, 2022
@parse-community parse-community locked and limited conversation to collaborators Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants