Consider removing MatchExpression::path() in favor of fieldRef() or similar

    • Type: Improvement
    • Resolution: Unresolved
    • Priority: Major - P3
    • None
    • Affects Version/s: None
    • Component/s: None
    • None
    • Query Optimization
    • None
    • 3
    • TBD
    • None
    • None
    • None
    • None
    • None
    • None

      MatchExpression::path() does not distinguish between (1) an empty path ("") and (2) no path at all. For example:

      // Case 1: $type with an empty path has path() = ""
      find({"": {$type: 'long'}})
      
      // Case 2: $and is a non-PathMatchExpression and has path() = ""
      find({$and: [...]})
      
      // Case 2: $type without a path has path() = ""
      find({a: {$elemMatch: {$type: 'long'}}}) 

      While rewriting a query, it's a common pattern to check `expr->path().empty()` to figure out if we're in case 2. For example, we may do this when walking a MatchExpression tree to build a full/dotted path for a leaf expression. However, this also excludes case 1, often accidentally. This can lead to bugs due to misunderstanding which path in a document a particular MatchExpression applies to.

      In SERVER-65494, we introduced PathMatchExpression::optPath() to disambiguate, but it seems to be underused, possibly because it's only available for PathMatchExpressions. We already have MatchExpression::fieldRef(); I wonder if we could standardize on that.

            Assignee:
            Unassigned
            Reporter:
            Hana Pearlman
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: