Use a seperate memory pool for input vectors in ExpressionRunner

Summary:
At present, the expression runner utilizes the identical memory pool for both
the deserialization of input vectors and the execution of the expression
evaluation. Typically, inputs are produced by a distinct operator that has a
separate pool from the FilterProject operator. Therefore, to mimic this
scenario, this modification ensures that we use separate memory pools.

Reviewed By: kagamiori

Differential Revision: D54277992

Author: Bikramjeet Vig

velox/docs/develop/testing/fuzzer.rst

@@ -284,6 +284,10 @@ ExpressionRunner supports the following flags:
 
 * ``--store_result_path`` optional directory path for storing the results of evaluating SQL expression or query in 'common', 'simplified' or 'query' modes.
 
+* ``--findMinimalSubExpression`` optional Whether to find minimum failing subexpression on result mismatch. Set to false by default.
+
+* ``--useSeperatePoolForInput`` optional If true (default), expression evaluator and input vectors use different memory pools. This helps trigger code-paths that can depend on vectors having different pools. For eg, when copying a flat string vector copies of the strings stored in the string buffers need to be created. If however, the pools were the same between the vectors then the buffers can simply be shared between them instead.
+
 Example command:
 
 ::

velox/expression/tests/ExpressionRunner.cpp

@@ -115,21 +115,26 @@ void ExpressionRunner::run(
     vector_size_t numRows,
     const std::string& storeResultPath,
     const std::string& lazyColumnListPath,
-    bool findMinimalSubExpression) {
+    bool findMinimalSubExpression,
+    bool useSeperatePoolForInput) {
   VELOX_CHECK(!sql.empty());
-
+  auto memoryManager = memory::memoryManager();
   std::shared_ptr<core::QueryCtx> queryCtx{std::make_shared<core::QueryCtx>()};
-  std::shared_ptr<memory::MemoryPool> pool{
-      memory::deprecatedAddDefaultLeafMemoryPool()};
+  std::shared_ptr<memory::MemoryPool> deserializerPool{
+      memoryManager->addLeafPool()};
+  std::shared_ptr<memory::MemoryPool> pool = useSeperatePoolForInput
+      ? memoryManager->addLeafPool("exprEval")
+      : deserializerPool;
   core::ExecCtx execCtx{pool.get(), queryCtx.get()};
 
   RowVectorPtr inputVector;
+
   if (inputPath.empty()) {
     inputVector = std::make_shared<RowVector>(
-        pool.get(), ROW({}), nullptr, 1, std::vector<VectorPtr>{});
+        deserializerPool.get(), ROW({}), nullptr, 1, std::vector<VectorPtr>{});
   } else {
     inputVector = std::dynamic_pointer_cast<RowVector>(
-        restoreVectorFromFile(inputPath.c_str(), pool.get()));
+        restoreVectorFromFile(inputPath.c_str(), deserializerPool.get()));
     VELOX_CHECK_NOT_NULL(
         inputVector,
         "Input vector is not a RowVector: {}",

velox/expression/tests/ExpressionRunner.h

@@ -49,6 +49,15 @@ class ExpressionRunner {
   /// @param lazyColumnListPath The path to on-disk vector of column indices
   /// that specify which columns of the input row vector should be wrapped in
   /// lazy.
+  /// @param findMinimalSubExpression Whether to find minimum failing
+  ///        subexpression on result mismatch.
+  /// @param useSeperatePoolForInput Whether to use separate memory pools for
+  ///        input vector and expression evaluation. This helps trigger
+  ///        code-paths that can depend on vectors having different pools. For
+  ///        eg, when copying a flat string vector copies of the strings stored
+  ///        in the string buffers need to be created. If however, the pools
+  ///        were the same between the vectors then the buffers can simply be
+  ///        shared between them instead.
   ///
   /// User can refer to 'VectorSaver' class to see how to serialize/preserve
   /// vectors to disk.
@@ -61,7 +70,8 @@ class ExpressionRunner {
       vector_size_t numRows,
       const std::string& storeResultPath,
       const std::string& lazyColumnListPath,
-      bool findMinimalSubExpression = false);
+      bool findMinimalSubExpression = false,
+      bool useSeperatePoolForInput = true);
 
   /// Parse comma-separated SQL expressions. This should be treated as private
   /// except for tests.

velox/expression/tests/ExpressionRunnerTest.cpp

@@ -86,6 +86,16 @@ DEFINE_string(
     "indices that specify which columns of the input row vector should "
     "be wrapped in lazy.");
 
+DEFINE_bool(
+    use_seperate_memory_pool_for_input_vector,
+    true,
+    "If true, expression evaluator and input vectors use different memory pools."
+    " This helps trigger code-paths that can depend on vectors having different"
+    " pools. For eg, when copying a flat string vector copies of the strings"
+    " stored in the string buffers need to be created. If however, the pools"
+    " were the same between the vectors then the buffers can simply be shared"
+    " between them instead.");
+
 static bool validateMode(const char* flagName, const std::string& value) {
   static const std::unordered_set<std::string> kModes = {
       "common", "simplified", "verify", "query"};
@@ -207,7 +217,7 @@ int main(int argc, char** argv) {
     sql = restoreStringFromFile(FLAGS_sql_path.c_str());
     VELOX_CHECK(!sql.empty());
   }
-
+  memory::initializeMemoryManager({});
   test::ExpressionRunner::run(
       FLAGS_input_path,
       sql,
@@ -217,5 +227,6 @@ int main(int argc, char** argv) {
       FLAGS_num_rows,
       FLAGS_store_result_path,
       FLAGS_lazy_column_list_path,
-      FLAGS_find_minimal_subexpression);
+      FLAGS_find_minimal_subexpression,
+      FLAGS_use_seperate_memory_pool_for_input_vector);
 }

velox/expression/tests/ExpressionRunnerUnitTest.cpp

@@ -63,8 +63,20 @@ TEST_F(ExpressionRunnerUnitTest, run) {
   saveVectorToFile(inputVector.get(), inputPath);
   saveVectorToFile(resultVector.get(), resultPath);
 
-  EXPECT_NO_THROW(ExpressionRunner::run(
-      inputPath, "length(c0)", "", resultPath, "verify", 0, "", ""));
+  for (bool useSeperatePoolForInput : {true, false}) {
+    LOG(INFO) << "Using useSeperatePoolForInput: " << useSeperatePoolForInput;
+    EXPECT_NO_THROW(ExpressionRunner::run(
+        inputPath,
+        "length(c0)",
+        "",
+        resultPath,
+        "verify",
+        0,
+        "",
+        "",
+        false,
+        useSeperatePoolForInput));
+  }
 }
 
 TEST_F(ExpressionRunnerUnitTest, persistAndReproComplexSql) {