parse-server icon indicating copy to clipboard operation
parse-server copied to clipboard

fix: `Parse.Query.distinct` fails due to invalid aggregate stage 'hint'

Open Chilldev opened this issue 1 year ago • 7 comments

Pull Request

Issue

Closes: #8804

Approach

When directAccess is set to true and a distinct query is made within an eachBatch query an Error: Invalid aggregate stage 'hint'. is thrown.

Tasks

  • [ ] Add tests

Chilldev avatar Sep 04 '24 16:09 Chilldev

Thanks for opening this pull request!

So this test looks good, we just need to the fix for it.

mtrezza avatar Oct 14 '24 15:10 mtrezza

Interesting, could you explain the last commit?

mtrezza avatar Oct 15 '24 20:10 mtrezza

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.48%. Comparing base (dfd5a8e) to head (e654b92). Report is 4 commits behind head on alpha.

Additional details and impacted files
@@           Coverage Diff           @@
##            alpha    #9295   +/-   ##
=======================================
  Coverage   93.48%   93.48%           
=======================================
  Files         186      186           
  Lines       14811    14812    +1     
=======================================
+ Hits        13846    13847    +1     
  Misses        965      965           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 15 '24 20:10 codecov[bot]

Interesting, could you explain the last commit?

What happens is the Parse JS SDK sends the data directly to the AggregateRouter without it being sent over http (which removes undefined properties from the request). It has code where it sets hint with value undefined to query body by default.

So when that check is made it doesn't pass as hint is undefined and the property is never deleted from the object.

When it goes to the aggregatePipeline it finds it on the body and throws an error.

What I did was checking if the object has that property to make sure it gets deleted wether it has value or not and assign that property to the options object only if it's truthy.

https://github.com/parse-community/Parse-SDK-JS/blob/alpha/src%2FParseQuery.ts#L820-L827

Chilldev avatar Oct 16 '24 16:10 Chilldev

Does that mean that we missed something in https://github.com/parse-community/parse-server/commit/d0d30c4f1394f563724644a8fc81734be538a2c0? I mean is this PR treating merely the symptom, or is it the actual root cause that gets fixed?

mtrezza avatar Oct 16 '24 17:10 mtrezza

Does that mean that we missed something in d0d30c4? I mean is this PR treating merely the symptom, or is it the actual root cause that gets fixed?

Nothing is missing getPipeline handles it. And hint is expected to be provided that way for the query options.

https://github.com/parse-community/parse-server/blob/dfd5a8edbfc008f708c6ba02d5541ae8e513cd13/src/Adapters/Storage/Mongo/MongoCollection.js#L16-L47

https://github.com/parse-community/parse-server/blob/dfd5a8edbfc008f708c6ba02d5541ae8e513cd13/src/Adapters/Storage/Mongo/MongoCollection.js#L89-L111

Chilldev avatar Oct 17 '24 09:10 Chilldev

🎉 This change has been released in version 7.4.0-alpha.4

parseplatformorg avatar Oct 22 '24 18:10 parseplatformorg