Mongo explain results incorrectly returning an object instead of an array of objects #7442

Activity

1. mtrezza commented on Jun 20, 2021

   

   mtrezza

   on Jun 20, 2021

   Last edited by mtrezza

   Member

   More actions

   Thanks for opening this issue. Is this an enhancement or a bug?
2. cbaker6 commented on Jun 20, 2021

   

   cbaker6

   on Jun 20, 2021

   Last edited by cbaker6

   ContributorAuthor

   More actions

   I would label this as a bug. The reason for this is in JS because the language isn’t strongly typed, but from an implementation standpoint, explain should have always returned an array since every other query returns an array when using find. The bug was discovered because of its use in the Swift SDK which is strongly typed https://community.parseplatform.org/t/explain-result-in-parseswift/1557/2?u=cbaker6
3. mtrezza commented on Jun 20, 2021

   

   mtrezza

   on Jun 20, 2021

   Member

   More actions

   Isn't the ObjC SDK also strongly typed? How is that solved there?
4. cbaker6 commented on Jun 20, 2021

   

   cbaker6

   on Jun 20, 2021

   ContributorAuthor

   More actions

   Isn't the ObjC SDK also strongly typed? How is that solved there?

   I can check, but from memory, I don’t think the Objc SDK has the ability to explain
5. mtrezza commented on Jun 20, 2021

   

   mtrezza

   on Jun 20, 2021

   Member

   More actions

   I think you are right, I just looked through it.
6. mtrezza commented on Jun 20, 2021

   

   mtrezza

   on Jun 20, 2021

   Last edited by mtrezza

   Member

   More actions

   This will break SDKs such as the JS SDK.

   Do you mean that a change will inevitable include a breaking change for the JS SDK? Or is there anything broken currently in the JS SDK?

   I assume you mean the breaking change of

   const query = new Parse.Query('_User');
   query.explain();
   const result = await query.find();
   

   returning an array instead of an object?

   In addition, this brings up another issue in the JS SDK which doesn't support explain for first queries.

   Is this related to results not being an array, or a completely separate issue?
7. cbaker6 commented on Jun 20, 2021

   

   cbaker6

   on Jun 20, 2021

   Last edited by cbaker6

   ContributorAuthor

   More actions

   Do you mean that a change will inevitable include a breaking change for the JS SDK? Or is there anything broken currently in the JS SDK?

   Yes, plus, any dev who uses Mongo and explain will have to modify their code no matter if they use JS or or REST directly. I think the Flutter SDK might use explain also in which it will probably break. The only SDK that won’t break is the Swift SDK because I never conformed to the bug

   Is this related to results not being an array, or a completely separate issue?

   Kind of, first should automatically take the first index of results on the client side. I added a test on this for the server which is why I say the PR in the JS SDK is needed. Using find should return an array.

   The easiest way to compare is looking at the implementation in Swift:

   First:
   https://github.com/parse-community/Parse-Swift/blob/f877b37bf4c4536d41b600791d6cbbae328c7575/Sources/ParseSwift/Types/Query.swift#L1572-L1583

   Find:
   https://github.com/parse-community/Parse-Swift/blob/f877b37bf4c4536d41b600791d6cbbae328c7575/Sources/ParseSwift/Types/Query.swift#L1564
8. mtrezza commented on Jun 20, 2021

   

   mtrezza

   on Jun 20, 2021

   Last edited by mtrezza

   Member

   More actions

   Before we weigh breaking all SDKs vs. making the Swift SDK conform to the "bug" - would it be possible at all to make the Swift SDK conform?
9. cbaker6 commented on Jun 20, 2021

   

   cbaker6

   on Jun 20, 2021

   Last edited by cbaker6

   ContributorAuthor

   More actions

   Before we weigh breaking all SDKs vs. making the Swift SDK conform to the "bug" - would it be possible at all to make the Swift SDK conform?

   Doing this would break the Postgres implementation in which I setup correctly on the server side #6506. In its current form (before this PR), only Mongo or Postgres explain can work on strongly typed SDKs, not both as the Mongo explain was originally coded incorrectly
10. cbaker6 commented on Jun 20, 2021

    

    cbaker6

    on Jun 20, 2021

    Last edited by cbaker6

    ContributorAuthor

    More actions

    My point is, the Mongo storage adaptor is disobeying the correct sending of results, particularly when using JSON.

    when developing the Swift SDK I never conformed to bugs as long as I knew it was a bug originally. Of course, I could have introduced bugs which should be fixed over time.
11. mtrezza commented on Jun 20, 2021

    

    mtrezza

    on Jun 20, 2021

    Last edited by mtrezza

    Member

    More actions

    Yes, I understand your point, it makes sense to improve these things. I am trying to understand the implication of such a change. If it breaks only developer code, then this is just another breaking change. But if it breaks SDKs, we may need to prepare these SDKs before making the change in the server. It would be interesting to know which SDKs exactly this would break.
12. cbaker6 commented on Jun 20, 2021

    

    cbaker6

    on Jun 20, 2021

    ContributorAuthor

    More actions

    Yes, I understand your point, it makes sense to improve these things. I am trying to understand the implication of such a change. If it breaks only developer code, then this is just another breaking change. But if it breaks SDKs, we may need to prepare these SDKs before making the change in the server. It would be interesting to know which SDKs exactly this would break.

    From memory, the JS, Swift (non-breaking), and Flutter SDKs have explain. I don’t think the others have it, but I’m on mobile, so we will have to confirm
13. cbaker6 commented on Jun 20, 2021

    

    cbaker6

    on Jun 20, 2021

    Last edited by cbaker6

    ContributorAuthor

    More actions

    For the JS SDK, this will not break the SDK itself, but breaks it's testcases. In addition, if devs were using find explain they will need to fix their code to use an array instead of an object.

    It seems the only SDKs that have the ability to explain are the JS and Swift SDKs. I’ve checked the obj-c, android, flutter, php, and dotNet and can’t find explain in any of them.
14. mtrezza commented on Jun 21, 2021

    

    mtrezza

    on Jun 21, 2021

    Member

    More actions

    Then I think we can go ahead with a PR and highlight the breaking change.
15. mtrezza

    added

    type:bugImpaired feature or lacking behavior that is likely assumedImpaired feature or lacking behavior that is likely assumed

    S4

    on Jun 21, 2021
16. mtrezza commented on Jun 21, 2021

    

    mtrezza

    on Jun 21, 2021

    Last edited by mtrezza

    Member

    More actions

    I classified this as a bug with severity S4 (small / trivial):

    • It does not impact any existing functionality, but can be considered a "conceptual flaw" with the effect that it does not allow to easily implement explain in SDKs with strongly typed languages.
    • The workaround is to use the REST API and manually parse the response.

    👍React with 👍1Reacted by Corey
17. cbaker6 commented on Jun 22, 2021

    

    cbaker6

    on Jun 22, 2021

    ContributorAuthor

    More actions

    Another workaround is to write an afterFind trigger in Cloud Code to take the object explain returns and place it in array. This should allow explain to work work with strongly typed SDKs like the Swift SDK.
18. mtrezza

    removed

    severity:low

    on Jan 17, 2022