ExportXMLWordPrintableJSON

    • Type: Bug
    • Resolution: Unresolved
    • Priority: Major - P3
    • None
    • Affects Version/s: 8.0.4, 9.0.2, 8.0.32, 9.1.0-rc1023
    • Component/s: None
    • Query Execution
    • ALL
    • Hide
      db.w.drop();
      db.w.insertMany([0, 1, 2, 3, 5, 8, 13, 21].map((x, i) => (\{_id: i, x: x, v: 1})));
      
      function run(lo, hi) {
        return db.w.aggregate([
          \{$setWindowFields: {sortBy: {x: 1},
            output: \{s: {$sum: "$v", window: {documents: [lo, hi]}}}}},
          \{$sort: {_id: 1}}
        ]).toArray().map(d => d.s);
      }
      
      run(-2147483647, 0)   // [1,2,3,4,5,6,7,8]   correct: running count
      run(-2147483648, 0)   // [1,1,1,1,1,1,1,1]   WRONG: expected [1,2,3,4,5,6,7,8]
      run(-2147483649, 0)   // error: "Numeric document-based bounds must be an integer"
      

      Expected: documents: [-2147483648, 0] returns the same result as [-2147483647, 0] and ["unbounded", 0] on this 8-document partition, which is [1,2,3,4,5,6,7,8].
      Actual: it returns [1,1,1,1,1,1,1,1], the same as documents: [0, 0]. There is no error and no warning.

       

      Show
      db.w.drop(); db.w.insertMany([0, 1, 2, 3, 5, 8, 13, 21].map((x, i) => (\{_id: i, x: x, v: 1}))); function run(lo, hi) {   return db.w.aggregate([     \{$setWindowFields: {sortBy: {x: 1},       output: \{s: {$sum: "$v" , window: {documents: [lo, hi]}}}}},     \{$sort: {_id: 1}}   ]).toArray().map(d => d.s); } run(-2147483647, 0)   // [1,2,3,4,5,6,7,8]   correct: running count run(-2147483648, 0)   // [1,1,1,1,1,1,1,1]   WRONG: expected [1,2,3,4,5,6,7,8] run(-2147483649, 0)   // error: "Numeric document-based bounds must be an integer" Expected: documents: [-2147483648, 0] returns the same result as [-2147483647, 0] and ["unbounded", 0] on this 8-document partition, which is [1,2,3,4,5,6,7,8] . Actual: it returns [1,1,1,1,1,1,1,1] , the same as documents: [0, 0] . There is no error and no warning.  
    • None
    • None
    • None
    • None
    • None
    • None
    • None

      Summary

      A $setWindowFields document-based window whose lower bound is exactly -2147483648 (INT32_MIN) is accepted and silently evaluated as a different window. The removable-document executor computes abs(_lowerBound). For INT_MIN that is undefined behaviour, and in practice it yields INT_MIN, a negative number. The "has the left side of the window filled yet?" test is therefore always true, so the executor removes a document on every step. A running $sum/$count/$push/$avg/$min/$stdDevPop collapses to the current row only.

      The value is one step away from correct behaviour on each side. -2147483647 works, -2147483648 is wrong, and -2147483649 is rejected by the parser. The one int32 value that the parser accepts and the executor cannot handle is the one that returns wrong results.

      Observed on master (9.1.0 nightly, gitVersion e5ad5e50d389), identical on 9.0.2 and 8.0.32

      Data and pipeline are as in Steps To Reproduce (8 documents, $sum: "$v" with v = 1).

      documents:[ -2147483647              , 0]  $sum -> [1,2,3,4,5,6,7,8]
      documents:[ -2147483648              , 0]  $sum -> [1,1,1,1,1,1,1,1]     <<< wrong
      documents:[ NumberInt(-2147483648)   , 0]  $sum -> [1,1,1,1,1,1,1,1]     <<< wrong
      documents:[ NumberLong(-2147483648)  , 0]  $sum -> [1,1,1,1,1,1,1,1]     <<< wrong
      documents:[ NumberDecimal(-2147483648),0]  $sum -> [1,1,1,1,1,1,1,1]     <<< wrong
      documents:[ -2147483649              , 0]  $sum -> ERROR: Numeric document-based bounds must be an integer
      
      documents:["unbounded", 0]                 $sum -> [1,2,3,4,5,6,7,8]
      documents:[-2147483647, 2147483647]        $sum -> [8,8,8,8,8,8,8,8]
      documents:[-2147483648, 2147483647]        $sum -> [8,7,6,5,4,3,2,1]     <<< wrong, expected all 8
      documents:[-2147483647, -1]                $sum -> [0,1,2,3,4,5,6,7]
      documents:[-2147483648, -1]                $sum -> [0,0,0,0,0,0,0,0]     <<< wrong, expected [0,1,...,7]
      

      Every accumulator that uses the removable document executor is affected. The comparison below is lower bound -2147483647 versus -2147483648, upper bound 0, over x = 0,1,2,3,5,8,13,21:

      $count     [1,2,3,4,5,6,7,8]                 vs  [1,1,1,1,1,1,1,1]       wrong
      $avg       [0,0.5,1,1.5,2.2,3.17,4.57,6.625] vs  [0,1,2,3,5,8,13,21]     wrong
      $min       [0,0,0,0,0,0,0,0]                 vs  [0,1,2,3,5,8,13,21]     wrong
      $push      (lengths) [1,2,...,8]             vs  [1,1,...,1]             wrong
      $addToSet  (sizes)   [1,2,...,8]             vs  [1,1,...,1]             wrong
      $stdDevPop [0,0.5,0.82,...,6.73]             vs  [0,0,0,0,0,0,0,0]       wrong
      $max       [0,1,2,3,5,8,13,21]               vs  same                    (right only because x is ascending)
      $first     [0,0,...]                         vs  same                    not affected (different executor)
      

      explain() shows the window runs in the classic $_internalSetWindowFields stage under the default internalQueryFrameworkControl: "trySbeRestricted". The stage is still classic with trySbeEngine, and the result is the same.

      Root cause

      src/mongo/db/pipeline/window_function/window_bounds.cpp:167-172: the parser accepts any value for which Value::integral() is true, i.e. anything representable as int32, so INT_MIN gets through:

      auto parseInt = [](Value v) -> int {
          uassert(ErrorCodes::FailedToParse,
                  "Numeric document-based bounds must be an integer",
                  v.integral());
          return v.coerceToInt();
      };
      

      src/mongo/db/pipeline/window_function/window_function_exec_removable_document.cpp:83 (_lowerBound is int, header line 60; getCurrentPartitionIndex() returns int):

          // For a negative lower bound we can start removing every time only after we have seen
          // documents to fill the left side of the window.
          if (_lowerBound >= 0 || _iter.getCurrentPartitionIndex() > abs(_lowerBound)) {
              removeFirstValueIfExists();
          }
      

      abs(INT_MIN) is undefined behaviour. On these builds it evaluates to INT_MIN, so index > abs(_lowerBound) is index > -2147483648, which is always true. removeFirstValueIfExists() then runs on every update(), the window never grows beyond what initialize() loaded for the first row, and its size stays fixed. That explains every row in the tables above:

      1. [INT_MIN, 0]: the window holds only the current document, the same as [0, 0].
      2. [INT_MIN, INT_MAX]: initialize() loads the whole partition and one document is removed per row, giving [8,7,...,1], the same as [0, "unbounded"].
      3. [INT_MIN, -1]: one document is added and one removed per row, so the window is always empty.

      Related latent overflow (same bounds, no wrong result observed)

      partition_iterator.cpp:89-90 computes auto docDesired = _indexOfCurrentInPartition + index; with two int}}s. With an upper bound of {{2147483647 (accepted), this overflows as soon as the current index is >= 1. That is also UB. Today it happens to wrap negative and hit the docDesired < 0 early return, so results are correct, but it is worth fixing together, e.g. by computing in int64_t.

      Suggested fix

      Do not negate the bound in int. For example:

          if (_lowerBound >= 0 ||
              static_cast<int64_t>(_iter.getCurrentPartitionIndex()) > -static_cast<int64_t>(_lowerBound)) {
      

      Alternatively, reject -2147483648 in window_bounds.cpp the same way -2147483649 is already rejected. The first option keeps the documented "any integer" contract. A regression test can be a jstest asserting that documents: [-2147483648, 0] gives the same output as documents: ["unbounded", 0]. Building with UBSan (-fsanitize=undefined) should also flag the abs call on this input.

      Why it matters

      INT_MIN is not an exotic value for a client to send. It is Integer.MIN_VALUE / int.MinValue / math.MinInt32, a common "as far back as possible" sentinel, and the result of negating a large positive bound. The server accepts the pipeline, and the result looks plausible (a running total becomes the per-row value), so nothing downstream can detect it. The neighbouring value one lower is rejected, so a user who sees no error has no reason to suspect a problem.

            Assignee:
            Unassigned
            Reporter:
            Ke N/A (EXT)
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: