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

Support pointer in distinct query #4471

Merged
merged 3 commits into from
Dec 30, 2017

Conversation

dplewis
Copy link
Member

@dplewis dplewis commented Dec 29, 2017

Per #2238 (comment)

Let me know if theres a better way to implement this.

@flovilmart
Copy link
Contributor

@dplewis The mongoObjectToParseObject method should already transform the pointers, perhaps just extract the logic: https://github.com/parse-community/parse-server/blob/master/src/Adapters/Storage/Mongo/MongoTransform.js#L1133

@dplewis
Copy link
Member Author

dplewis commented Dec 29, 2017

@flovilmart I extracted the logic because distinct returns a string like 'TestObject$ObjectId' which returns here. https://github.com/parse-community/parse-server/blob/master/src/Adapters/Storage/Mongo/MongoTransform.js#L1024

Edit: I see what you are saying

@flovilmart
Copy link
Contributor

Yeah, I mean what you can do is make a transformPointerString method in the MongoTransform.js module and use it at both places.

@vitaly-t
Copy link
Contributor

vitaly-t commented Dec 29, 2017

I generally do not like this line:

.catch(() => [])

It is dangerous, swallowing all possible errors like that.

Also, I probably would replace this block:

let qs = `SELECT DISTINCT ON ($1:raw) $2:raw FROM $3:name ${wherePattern}`;
if (isArrayField) {
   qs = `SELECT distinct jsonb_array_elements($1:raw) as $2:raw FROM $3:name ${wherePattern}`;
}

with this one:

const transformer = isArrayField ? 'jsonb_array_elements': 'ON';
const qs = `SELECT DISTINCT ${transformer}($1:raw) AS $2:raw FROM $3:name ${wherePattern}`;

@codecov
Copy link

codecov bot commented Dec 29, 2017

Codecov Report

Merging #4471 into master will decrease coverage by 0.12%.
The diff coverage is 92.3%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4471      +/-   ##
==========================================
- Coverage   92.98%   92.86%   -0.13%     
==========================================
  Files         118      118              
  Lines        8498     8376     -122     
==========================================
- Hits         7902     7778     -124     
- Misses        596      598       +2
Impacted Files Coverage Δ
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 96.09% <100%> (-1.66%) ⬇️
src/Adapters/Storage/Mongo/MongoTransform.js 85.4% <83.33%> (+0.04%) ⬆️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 97.07% <91.66%> (-0.09%) ⬇️
src/Adapters/Files/GridStoreAdapter.js 100% <0%> (ø) ⬆️
src/RestWrite.js 93.46% <0%> (ø) ⬆️
src/Adapters/Auth/meetup.js 89.47% <0%> (+5.26%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d773a5...6bdbc1d. Read the comment docs.

@dplewis
Copy link
Member Author

dplewis commented Dec 29, 2017

pushed-to-repo-travis-ci-build-passed

@vitaly-t
Copy link
Contributor

Can't see which PR made this one out-of-date, but requested a branch update, since it is required.

@dplewis
Copy link
Member Author

dplewis commented Dec 30, 2017

There were node packages that were merged

Copy link
Contributor

@vitaly-t vitaly-t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@dplewis dplewis merged commit 6143988 into parse-community:master Dec 30, 2017
@dplewis dplewis deleted the distinct-pointer branch December 30, 2017 03:32
@srameshr
Copy link
Contributor

srameshr commented Aug 24, 2018

@dplewis Does not work for me.

const query = new Parse.Query('MyClass');
query.include('content');
query.distinct('content.objectId');
query.find();

It just returns records with duplicate content column.

@flovilmart
Copy link
Contributor

Well, the single unit test that is exactly what you describe passes. So open a an issue with the full details as well as code examples and relevant logs

@dplewis
Copy link
Member Author

dplewis commented Aug 24, 2018

Do your objects have a field called “content.objectId”? Make sure you are querying the right field. Please open an issue with examples and I’ll look it over

@srameshr
Copy link
Contributor

srameshr commented Aug 24, 2018

content is a pointer field on MyClass. I need to fetch records with unique content pointer field. I just this approach, like fieldName.objectId in some other thread

@flovilmart
Copy link
Contributor

well @srameshr did you read the test file here

because what you're doing isn't how it's supposed to be used.

@srameshr
Copy link
Contributor

@flovilmart query.distinct('pointer') assuming pointer is the field name, so for me, the query would be query.distinct('content'), cos content is my field name. Even this did not work and also am I missing something?

@flovilmart
Copy link
Contributor

flovilmart commented Aug 24, 2018

did you notice that in the test it is:

const query = new Parse.Query(TestObject);
query.distinct('pointer').then((results) => {

});

and in your example:

const query = new Parse.Query('MyClass');
query.include('content');
query.distinct('content.objectId');
query.find();

Well if you don't see the differences now, I'm not sure how to help you.

@srameshr
Copy link
Contributor

Shit, ok got it. .distinct returns a promise with distinct values. My bad.

@flovilmart
Copy link
Contributor

Well, that's what's written in the test file. Next time consider reading what the maintainers share as well as perhpas the docs: https://docs.parseplatform.org/js/guide/#distinct

@srameshr
Copy link
Contributor

I heard you the first time.

UnderratedDev pushed a commit to UnderratedDev/parse-server that referenced this pull request Mar 21, 2020
* Support pointer in distinct query

* extract transform pointer string
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants