Arrayness of dotted path after inclusion projection is incorrect

XMLWordPrintableJSON

    • Query Optimization
    • ALL
    • v9.0
    • 200
    • None
    • None
    • None
    • None
    • None
    • None
    • None

      The dependency graph reports prefix of $project path as non-array after the $project stage.
      This affects the following simple cases:

      [{$project: {"a.b": {$literal: 1}}}]
      [{$project: {"a.b": "$z"}}]
      

      The prefix "a" is reported as non-array AFTER the $project.

      This is wrong: an inclusion projection applies the computed field element-wise:

      {a: [{}, {}]} becomes {a: [

      {b: 1}

      , {b: 1}]} and a is still an array.

      TEST_F(PipelineDependencyGraphTest, Test) {
          setPipeline("[{$project: {'a.b': {$literal: 1}}}]");
          runTest([&] {
              ASSERT_TRUE(graph->canPathBeArray(nullptr, "a"));
          });
      }
      

      This was discovered using the PBT test from SERVER-134177 which is not merged yet.

      Consequence

      Only the MATCH_PUSHDOWN optimization rule and JOO consume this today.

      The incorrectly recorded arrayness after the $project can result in incorrect inference about a complex rename later, which can lead to missed documents or additional documents after $match.

      db.c.drop();
      db.c.insertOne({a: [{}, {}], z: 1});
      db.c.aggregate([{$project: {"a.x": "$z", z: 1}}, {$project: {"a.b": "$z"}}, {$match: {$expr: {$eq: ["$a.b", 1]}}}]);
      

      Returns {a: [

      { b: 1 }

      , { b: 1 }]} when it shouldn't.

      It requires two $project stages on the same prefix or $project+$set to work.
      The first $project will result in the incorrect inference for the prefix, and the second $project/$set can then use that incorrect inference to decide the rename is simple.

      [{$project: {"a.x": "$z", z: 1}}, // wrong inference: 'a' is not array
       {$project: {"a.b": "$z"}}, // wrong inference: if 'a' is not array, this is a simple rename
       {$match: {$expr: {$eq: ["$a.b", 1]}}}]
        | | |
        v v v
      [{$project: {"a.x": "$z", z: 1}},
       {$match: {$expr: {$eq: ["$z", 1]}}}, // array traversal is lost
       {$project: {"a.b": "$z"}}]
      

      Example document:

      {a: [{}, {}], z: 1}
      

      $expr is not needed. Here's an example which breaks, because the array is empty and the array-traversal the $project does does not change the document.

      [{$project: {"a.x": "$z", z: 1}}, {$project: {"a.b": "$z"}}, {$match: {"a.b": 1}}]
      | | |
      v v v
      [{$project: {"a.x": "$z", z: 1}}, {$match: {"z": 1}}, {$project: {"a.b": "$z"}}]
      

      Example document:

      {a: [], z: 1}
      

      There are no elements to traverse into, so both projections leave the document unchanged and "a.b" is missing. The correct answer is no match, but the rewritten predicate sees z: 1 and returns the document.

      Root cause
      This has been present in the original inclusion projection handling in the graph. Some logic was added in SERVER-128465 but only addressed a subset, the general issue remained because in SERVER-128465 there is a loss of precision in the ScopeId parentScope that was added to declareField when the prior field is unknown to the graph.

      Fix
      A targeted fix is to guard this case and report maybe-array conservatively, and therefore prevents any pushdown in this case. We didn't do any pushdown past rename of this kind before 8.3 anyways.

      A general fix requires the new field lookup code from SERVER-131449 which is in-progress and is a larger change.

      Mitigation
      This code is guarded by the featureFlagImprovedDepsAnalysis IFR flag only, which is enabled by default. The arrayness-related feature flags are not required for this bug.

      There is no query knob.

      The graph inference is incorporated into the MATCH_PUSHDOWN when that flag is enabled, causing the incorrect rewrite.

            Assignee:
            Vesko Karaganev
            Reporter:
            Vesko Karaganev
            Votes:
            0 Vote for this issue
            Watchers:
            9 Start watching this issue

              Created:
              Updated: