Fix min_by/max_by(x, y, n) (facebookincubator#8566)

Summary:
Pull Request resolved: facebookincubator#8566

Same as bug in min/max(x, n) fixed in facebookincubator#8311, min_by/max_by(x, y, n) also breaks the
assumption of incremental window aggregation because their extractValues() methods
has a side effect of clearing the accumulator.

This diff fixes this issue by making the extractValues() methods of min_by/max_by(x, y, n)
not clear the accumulators.

Since Presto's min_by/max_by have the same bug (prestodb/presto#21653). This fix
will make Velox's min_by/max_by behave differently from Presto when used in Window
operation, until prestodb/presto#21653 is fixed.

This diff fixes facebookincubator#8138.

Reviewed By: bikramSingh91

Differential Revision: D53139892

fbshipit-source-id: 1323f22196e22554c0d880d20584a4ee4059b64c

velox/docs/develop/aggregate-functions.rst

@@ -283,6 +283,9 @@ initialize all accumulators.
 The author can also optionally define a `destroy` function that is called when
 *this* accumulator object is destructed.
 
+Notice that `writeIntermediateResult` and `writeFinalResult` are expected to not
+modify contents in the accumulator.
+
 addInput
 """"""""
 
@@ -365,6 +368,9 @@ behavior.
 On the other hand, the C++ function signatures of `addInput`, `combine`,
 `writeIntermediateResult`, and `writeFinalResult` are different.
 
+Same as the case for default-null behavior, `writeIntermediateResult` and
+`writeFinalResult` are expected to not modify contents in the accumulator.
+
 addInput
 """"""""
 
@@ -605,6 +611,7 @@ After implementing the addRawInput() method, we proceed to adding logic for extr
 .. code-block:: c++
 
       // Extracts partial results (used for partial and intermediate aggregations).
+      // This method is expected to not modify contents in accumulators.
       // @param groups Pointers to the start of the group rows.
       // @param numGroups Number of groups to extract results from.
       // @param result The result vector to store the results in.
@@ -625,7 +632,8 @@ Next, we implement the extractValues() method that extracts final results from t
 
 .. code-block:: c++
 
-      // Extracts final results (used for final and single aggregations).
+      // Extracts final results (used for final and single aggregations). This method
+      // is expected to not modify contents in accumulators.
       // @param groups Pointers to the start of the group rows.
       // @param numGroups Number of groups to extract results from.
       // @param result The result vector to store the results in.