[SERVER-30664] Extend ExpressionWithPlaceholder to accept top-level match expressions Created: 15/Aug/17 Updated: 30/Oct/23 Resolved: 30/Aug/17 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Internal Code, Querying |
| Affects Version/s: | None |
| Fix Version/s: | 3.5.13 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Kyle Suarez | Assignee: | Nicholas Zolnierz |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||
| Sprint: | Query 2017-09-11 | ||||||||
| Participants: | |||||||||
| Description |
|
Currently, ExpressionWithPlaceholder only allows us to parse MatchExpressions for which there exists a single top-level field name. We must extend it to allow top-level match expressions as well so that it can accept any MatchExpression generated by the JSON Schema parser. |
| Comments |
| Comment by Githook User [ 30/Aug/17 ] |
|
Author: {'name': 'Nick Zolnierz', 'username': 'nzolnierzmdb', 'email': 'nicholas.zolnierz@mongodb.com'}Message: |
| Comment by Tess Avitabile (Inactive) [ 21/Aug/17 ] |
|
Agreed that we don't need an actual string to be the default placeholder, since the placeholder is not used beyond matching the array filter against the identifiers. I just mean that conceptually, in the JSON schema case, we should still match the MatchExpression against the BSONElement as if it is wrapped in a BSONObj with a single field. |
| Comment by David Storch [ 21/Aug/17 ] |
|
tess.avitabile, I don't think there should be a concept of a specific string placeholder, such as i. Instead, I think the update code should fail if there is no placeholder, since it cannot correlate the array filter with the paths in the update expression. For JSON Schema, on the other hand, we can accept any placeholder string if the expression itself has no placeholder. |
| Comment by Kyle Suarez [ 18/Aug/17 ] |
|
Kyle to determine if we indeed only need those two pieces to work. |
| Comment by Tess Avitabile (Inactive) [ 18/Aug/17 ] |
|
That seems reasonable to me. When ExpressionWithPlaceholder has an empty query, we can just say that it uses a default placeholder i, so then we treat the BSONElement <elem> as if it is the BSONObj {i: <elem>} matching against the empty query {}. However, we would need to modify ParsedUpdate to check that no array filter has the default placeholder. |
| Comment by David Storch [ 18/Aug/17 ] |
|
Per our discussion from this morning I think there might only be two action items here:
|
| Comment by Tess Avitabile (Inactive) [ 18/Aug/17 ] |
|
I am not sure this will behave sensibly with the array filter semantics. When an ExpressionWithPlaceholder is an array filter, we rely on the fact that there is a single top-level field name i so that we can match an array element <elem> as if it is wrapped in an object {i: <elem>}. If we allow top-level match expressions in ExpressionWithPlaceholder, I am concerned that we will end up with the same language ambiguity we get with $pull match expressions, where the behavior is different depending on whether we are matching against an object or a scalar. |