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

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

Merged

Conversation

Chilldev
Copy link
Contributor

@Chilldev Chilldev commented Sep 4, 2024

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

Copy link

Thanks for opening this pull request!

@Chilldev Chilldev changed the title Add failing distinct query test to address issue #8804. Fix: Add failing distinct query test to address issue #8804. Sep 4, 2024
@Chilldev Chilldev changed the title Fix: Add failing distinct query test to address issue #8804. fix: Add failing distinct query test to address issue #8804. Sep 4, 2024
spec/helper.js Outdated Show resolved Hide resolved
@mtrezza mtrezza changed the title fix: Add failing distinct query test to address issue #8804. fix: Parse.Query.distinct fails due to invalid aggregate stage 'hint' Oct 14, 2024
@mtrezza
Copy link
Member

mtrezza commented Oct 14, 2024

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

@mtrezza
Copy link
Member

mtrezza commented Oct 15, 2024

Interesting, could you explain the last commit?

Copy link

codecov bot commented Oct 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Chilldev
Copy link
Contributor Author

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

@mtrezza
Copy link
Member

mtrezza commented Oct 16, 2024

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?

@Chilldev
Copy link
Contributor Author

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.

find(
query,
{
skip,
limit,
sort,
keys,
maxTimeMS,
readPreference,
hint,
caseInsensitive,
explain,
comment,
} = {}
) {
// Support for Full Text Search - $text
if (keys && keys.$score) {
delete keys.$score;
keys.score = { $meta: 'textScore' };
}
return this._rawFind(query, {
skip,
limit,
sort,
keys,
maxTimeMS,
readPreference,
hint,
caseInsensitive,
explain,
comment,
}).catch(error => {

_rawFind(
query,
{
skip,
limit,
sort,
keys,
maxTimeMS,
readPreference,
hint,
caseInsensitive,
explain,
comment,
} = {}
) {
let findOperation = this._mongoCollection.find(query, {
skip,
limit,
sort,
readPreference,
hint,
comment,
});

@mtrezza mtrezza merged commit 5f66c6a into parse-community:alpha Oct 22, 2024
29 of 30 checks passed
parseplatformorg pushed a commit that referenced this pull request Oct 22, 2024
# [7.4.0-alpha.4](7.4.0-alpha.3...7.4.0-alpha.4) (2024-10-22)

### Bug Fixes

* `Parse.Query.distinct` fails due to invalid aggregate stage 'hint' ([#9295](#9295)) ([5f66c6a](5f66c6a))
@parseplatformorg
Copy link
Contributor

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

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Oct 22, 2024
parseplatformorg pushed a commit that referenced this pull request Dec 23, 2024
# [7.4.0-beta.1](7.3.0...7.4.0-beta.1) (2024-12-23)

### Bug Fixes

* `Parse.Query.distinct` fails due to invalid aggregate stage 'hint' ([#9295](#9295)) ([5f66c6a](5f66c6a))
* Security upgrade cross-spawn from 7.0.3 to 7.0.6 ([#9444](#9444)) ([3d034e0](3d034e0))
* Security upgrade fast-xml-parser from 4.4.0 to 4.4.1 ([#9262](#9262)) ([992d39d](992d39d))
* Security upgrade node from 20.14.0-alpine3.20 to 20.17.0-alpine3.20 ([#9300](#9300)) ([15bb17d](15bb17d))

### Features

* Add support for MongoDB 8 ([#9269](#9269)) ([4756c66](4756c66))
* Add support for PostGIS 3.5 ([#9354](#9354)) ([8ea3538](8ea3538))
* Add support for Postgres 17 ([#9324](#9324)) ([fa2ee31](fa2ee31))
* Upgrade @parse/push-adapter from 6.7.1 to 6.8.0 ([#9489](#9489)) ([286aa66](286aa66))
parseplatformorg pushed a commit that referenced this pull request Dec 23, 2024
# [7.4.0](7.3.0...7.4.0) (2024-12-23)

### Bug Fixes

* `Parse.Query.distinct` fails due to invalid aggregate stage 'hint' ([#9295](#9295)) ([5f66c6a](5f66c6a))
* Security upgrade cross-spawn from 7.0.3 to 7.0.6 ([#9444](#9444)) ([3d034e0](3d034e0))
* Security upgrade fast-xml-parser from 4.4.0 to 4.4.1 ([#9262](#9262)) ([992d39d](992d39d))
* Security upgrade node from 20.14.0-alpine3.20 to 20.17.0-alpine3.20 ([#9300](#9300)) ([15bb17d](15bb17d))

### Features

* Add support for MongoDB 8 ([#9269](#9269)) ([4756c66](4756c66))
* Add support for PostGIS 3.5 ([#9354](#9354)) ([8ea3538](8ea3538))
* Add support for Postgres 17 ([#9324](#9324)) ([fa2ee31](fa2ee31))
* Upgrade @parse/push-adapter from 6.7.1 to 6.8.0 ([#9489](#9489)) ([286aa66](286aa66))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released-alpha Released as alpha version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parse-server V6.3.1 query.distinct("score") throw ParseError: Invalid aggregate stage 'hint'
3 participants