Add input_dir and lazy_column_list_path option to expression runner (facebookincubator#3231)

Summary:
This patch adds two startup flags to ExpressionRunner:

input_dir: allows it to find required input files in a specified
  directory. It searches for files generated by ExpressionVerfier
  like input_vector, result_vector, sql, indices_of_lazy_columns
  and complex_constants.

lazy_column_list_path: allows it to consume this list of column
  indices that represents the columns in the input vector to be wrapped in lazy

Pull Request resolved: facebookincubator#3231

Test Plan:
- Added a unit test for a utility function
- Manually tested

Reviewed By: mbasmanova

Differential Revision: D41293907

Pulled By: bikramSingh91

fbshipit-source-id: 68c287c8d1e261296e3d6169992b1cb8c963542a

Author: bikramSingh91

velox/docs/develop/testing/fuzzer.rst

@@ -164,21 +164,27 @@ input vector and expression to files and replay these later.
 
 2. Run Expression Runner using generated files.
 
-``--repro_persist_path <path/to/directory>`` flag tells the Fuzzer to save
-input vector and expression SQL to files in the specified directory and print
-out the exact paths. Fuzzer uses :doc:`VectorSaver <../debugging/vector-saver>` for storing vectors on disk
-while preserving encodings.
-
-ExpressionRunner takes a path to input vector, path to expression SQL and
-"mode" and evaluates the specified expression on the specified data.
+``--repro_persist_path <path/to/directory>`` flag tells the Fuzzer to save the
+input vector, initial result vector, expression SQL, and other relevant data to files in a new directory saved within
+the specified directory. It also prints out the exact paths for these. Fuzzer uses :doc:`VectorSaver <../debugging/vector-saver>`
+for storing vectors on disk while preserving encodings.
+
+ExpressionRunner needs at the very least a path to input vector and path to expression SQL to run.
+However, you might need more files to reproduce the issue. All of which will be present in the directory
+that the fuzzer test generated. You can directly point the ExpressionRunner to that directory using --fuzzer_repro_path
+where it will pick up all the files automatically or you can specify each explicitly using other startup flags.
 ExpressionRunner supports the following flags:
 
+* ``--fuzzer_repro_path`` directory path where all input files (required to reproduce a failure) that are generated by the Fuzzer are expected to reside. ExpressionRunner will automatically pick up all the files from this folder unless they are explicitly specified via their respective startup flag.
+
 * ``--input_path`` path to input vector that was created by the Fuzzer
 
 * ``--sql_path`` path to expression SQL that was created by the Fuzzer
 
 * ``--complex_constant_path`` optional path to complex constants that aren't accurately expressable in SQL (Array, Map, Structs, ...). This is used with SQL file to reproduce the exact expression, not needed when the expression doesn't contain complex constants.
 
+* ``--lazy_column_list_path`` optional path for the file stored on-disk which contains a vector of column indices that specify which columns of the input row vector should be wrapped in lazy. This is used when the failing test included input columns that were lazy vector.
+
 * ``--result_path`` optional path to result vector that was created by the Fuzzer. Result vector is used to reproduce cases where Fuzzer passes dirty vectors to expression evaluation as a result buffer. This ensures that functions are implemented correctly, taking into consideration dirty result buffer.
 
 * ``--mode`` run mode. One of "verify", "common" (default), "simplified".
@@ -189,8 +195,12 @@ ExpressionRunner supports the following flags:
 
     - ``simplified`` evaluates the expression using simplified path and prints the results to stdout.
 
+    - ``query`` evaluate SQL query specified in --sql or --sql_path and print out results. If --input_path is specified, the query may reference it as table 't'.
+
 * ``--num_rows`` optional number of rows to process in common and simplified modes. Default: 10. 0 means all rows. This flag is ignored in 'verify' mode.
 
+* ``--store_result_path`` optional directory path for storing the results of evaluating SQL expression or query in 'common', 'simplified' or 'query' modes.
+
 Example command:
 
 ::

velox/expression/tests/ExpressionFuzzer.cpp

@@ -256,8 +256,8 @@ std::vector<column_index_t> generateLazyColumnIds(
   std::vector<column_index_t> columnsToWrapInLazy;
   if (FLAGS_lazy_vector_generation_ratio > 0) {
     for (column_index_t idx = 0; idx < rowVector->childrenSize(); idx++) {
-      if (rowVector->childAt(idx) &&
-          vectorFuzzer.coinToss(FLAGS_lazy_vector_generation_ratio)) {
+      VELOX_CHECK_NOT_NULL(rowVector->childAt(idx));
+      if (vectorFuzzer.coinToss(FLAGS_lazy_vector_generation_ratio)) {
         columnsToWrapInLazy.push_back(idx);
       }
     }
@@ -413,7 +413,7 @@ void ExpressionFuzzer::reSeed() {
 }
 
 RowVectorPtr ExpressionFuzzer::generateRowVector() {
-  return vectorFuzzer_.fuzzRow(
+  return vectorFuzzer_.fuzzInputRow(
       ROW(std::move(inputRowNames_), std::move(inputRowTypes_)));
 }
 

velox/expression/tests/ExpressionRunner.cpp

@@ -20,7 +20,6 @@
 #include "velox/common/memory/Memory.h"
 #include "velox/core/QueryCtx.h"
 #include "velox/exec/tests/utils/AssertQueryBuilder.h"
-#include "velox/exec/tests/utils/QueryAssertions.h"
 #include "velox/expression/Expr.h"
 #include "velox/expression/tests/ExpressionRunner.h"
 #include "velox/expression/tests/ExpressionVerifier.h"
@@ -29,6 +28,7 @@
 #include "velox/parse/QueryPlanner.h"
 #include "velox/parse/TypeResolver.h"
 #include "velox/vector/VectorSaver.h"
+#include "velox/vector/fuzzer/VectorFuzzer.h"
 
 namespace facebook::velox::test {
 
@@ -113,7 +113,8 @@ void ExpressionRunner::run(
     const std::string& resultPath,
     const std::string& mode,
     vector_size_t numRows,
-    const std::string& storeResultPath) {
+    const std::string& storeResultPath,
+    const std::string& lazyColumnListPath) {
   VELOX_CHECK(!sql.empty());
 
   std::shared_ptr<core::QueryCtx> queryCtx{std::make_shared<core::QueryCtx>()};
@@ -134,6 +135,12 @@ void ExpressionRunner::run(
     VELOX_CHECK_GT(inputVector->size(), 0, "Input vector must not be empty.");
   }
 
+  std::vector<column_index_t> columnsToWrapInLazy;
+  if (!lazyColumnListPath.empty()) {
+    columnsToWrapInLazy =
+        restoreStdVectorFromFile<column_index_t>(lazyColumnListPath.c_str());
+  }
+
   parse::registerTypeResolver();
 
   if (mode == "query") {
@@ -179,8 +186,17 @@ void ExpressionRunner::run(
     VELOX_CHECK_EQ(
         1, typedExprs.size(), "'verify' mode supports only one SQL expression");
     test::ExpressionVerifier(&execCtx, {false, ""})
-        .verify(typedExprs[0], inputVector, std::move(resultVector), true);
+        .verify(
+            typedExprs[0],
+            inputVector,
+            std::move(resultVector),
+            true,
+            columnsToWrapInLazy);
   } else if (mode == "common") {
+    if (!columnsToWrapInLazy.empty()) {
+      inputVector =
+          VectorFuzzer::fuzzRowChildrenToLazy(inputVector, columnsToWrapInLazy);
+    }
     exec::ExprSet exprSet(typedExprs, &execCtx);
     auto results = evaluateAndPrintResults(exprSet, inputVector, rows, execCtx);
     if (!storeResultPath.empty()) {

velox/expression/tests/ExpressionRunner.h

@@ -43,6 +43,12 @@ class ExpressionRunner {
   ///        "simplified"]
   /// @param numRows Maximum number of rows to process. 0 means 'all' rows.
   ///         Applies to "common" and "simplified" modes only.
+  /// @param storeResultPath The path to a directory on disk where the results
+  /// of expression or query evaluation will be stored. If empty, the results
+  /// will not be stored.
+  /// @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.
   ///
   /// User can refer to 'VectorSaver' class to see how to serialize/preserve
   /// vectors to disk.
@@ -53,7 +59,8 @@ class ExpressionRunner {
       const std::string& resultPath,
       const std::string& mode,
       vector_size_t numRows,
-      const std::string& storeResultPath);
+      const std::string& storeResultPath,
+      const std::string& lazyColumnListPath);
 
   /// Parse comma-separated SQL expressions. This should be treated as private
   /// except for tests.

velox/expression/tests/ExpressionRunnerTest.cpp

@@ -18,10 +18,14 @@
 #include <folly/init/Init.h>
 #include <gflags/gflags.h>
 #include <gtest/gtest.h>
+#include "velox/common/base/Fs.h"
+#include "velox/expression/tests/ExpressionVerifier.h"
 #include "velox/functions/prestosql/aggregates/RegisterAggregateFunctions.h"
 #include "velox/functions/prestosql/registration/RegistrationFunctions.h"
 #include "velox/vector/VectorSaver.h"
 
+using namespace facebook::velox;
+
 DEFINE_string(
     input_path,
     "",
@@ -39,7 +43,7 @@ DEFINE_string(
 DEFINE_string(
     complex_constant_path,
     "",
-    "Path for complex constants that aren't expressable in SQL.");
+    "Path for complex constants that aren't expressible in SQL.");
 
 DEFINE_string(
     sql,
@@ -68,6 +72,13 @@ DEFINE_string(
     "results. If --input_path is specified, the query may reference it as "
     "table 't'.");
 
+DEFINE_string(
+    lazy_column_list_path,
+    "",
+    "Path for the file stored on-disk which contains a vector of column "
+    "indices that specify which columns of the input row vector should "
+    "be wrapped in lazy.");
+
 static bool validateMode(const char* flagName, const std::string& value) {
   static const std::unordered_set<std::string> kModes = {
       "common", "simplified", "verify", "query"};
@@ -95,32 +106,86 @@ DEFINE_string(
     "Directory path for storing the results of evaluating SQL expression or "
     "query in common, simplified or query modes.");
 
+DEFINE_string(
+    fuzzer_repro_path,
+    "",
+    "Directory path where all input files generated by ExpressionVerifier are "
+    "expected to reside. For more details on which files and their names are "
+    "expected, please checkout the ExpressionVerifier class. Any file paths "
+    "already specified via a startup flag will take precedence.");
+
+static std::string checkAndReturnFilePath(
+    const std::string_view& fileName,
+    const std::string& flagName) {
+  auto path = fmt::format("{}/{}", FLAGS_fuzzer_repro_path, fileName);
+  if (fs::exists(path)) {
+    LOG(INFO) << "Using " << flagName << " = " << path;
+    return path;
+  } else {
+    LOG(INFO) << "File for " << flagName << " not found.";
+  }
+  return "";
+}
+
+static void checkDirForExpectedFiles() {
+  LOG(INFO) << "Searching input directory for expected files at "
+            << FLAGS_fuzzer_repro_path;
+
+  FLAGS_input_path = FLAGS_input_path.empty()
+      ? checkAndReturnFilePath(
+            test::ExpressionVerifier::kInputVectorFileName, "input_path")
+      : FLAGS_input_path;
+  FLAGS_result_path = FLAGS_result_path.empty()
+      ? checkAndReturnFilePath(
+            test::ExpressionVerifier::kResultVectorFileName, "result_path")
+      : FLAGS_result_path;
+  FLAGS_sql_path = FLAGS_sql_path.empty()
+      ? checkAndReturnFilePath(
+            test::ExpressionVerifier::kExpressionSqlFileName, "sql_path")
+      : FLAGS_sql_path;
+  FLAGS_lazy_column_list_path = FLAGS_lazy_column_list_path.empty()
+      ? checkAndReturnFilePath(
+            test::ExpressionVerifier::kIndicesOfLazyColumnsFileName,
+            "lazy_column_list_path")
+      : FLAGS_lazy_column_list_path;
+  FLAGS_complex_constant_path = FLAGS_complex_constant_path.empty()
+      ? checkAndReturnFilePath(
+            test::ExpressionVerifier::kComplexConstantsFileName,
+            "complex_constant_path")
+      : FLAGS_complex_constant_path;
+}
+
 int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);
   // Calls common init functions in the necessary order, initializing
   // singletons, installing proper signal handlers for better debugging
   // experience, and initialize glog and gflags.
   folly::init(&argc, &argv);
 
+  if (!FLAGS_fuzzer_repro_path.empty()) {
+    checkDirForExpectedFiles();
+  }
+
   if (FLAGS_sql.empty() && FLAGS_sql_path.empty()) {
     std::cout << "One of --sql or --sql_path flags must be set." << std::endl;
     exit(1);
   }
 
   auto sql = FLAGS_sql;
   if (sql.empty()) {
-    sql = facebook::velox::restoreStringFromFile(FLAGS_sql_path.c_str());
+    sql = restoreStringFromFile(FLAGS_sql_path.c_str());
     VELOX_CHECK(!sql.empty());
   }
 
-  facebook::velox::functions::prestosql::registerAllScalarFunctions();
-  facebook::velox::aggregate::prestosql::registerAllAggregateFunctions();
-  facebook::velox::test::ExpressionRunner::run(
+  functions::prestosql::registerAllScalarFunctions();
+  aggregate::prestosql::registerAllAggregateFunctions();
+  test::ExpressionRunner::run(
       FLAGS_input_path,
       sql,
       FLAGS_complex_constant_path,
       FLAGS_result_path,
       FLAGS_mode,
       FLAGS_num_rows,
-      FLAGS_store_result_path);
+      FLAGS_store_result_path,
+      FLAGS_lazy_column_list_path);
 }

velox/expression/tests/ExpressionRunnerUnitTest.cpp

@@ -45,7 +45,6 @@ TEST_F(ExpressionRunnerUnitTest, run) {
   auto sqlFile = exec::test::TempFilePath::create();
   auto resultFile = exec::test::TempFilePath::create();
   const char* inputPath = inputFile->path.data();
-  const char* sqlPath = sqlFile->path.data();
   const char* resultPath = resultFile->path.data();
   const int vectorSize = 100;
 
@@ -60,7 +59,7 @@ TEST_F(ExpressionRunnerUnitTest, run) {
   saveVectorToFile(resultVector.get(), resultPath);
 
   EXPECT_NO_THROW(ExpressionRunner::run(
-      inputPath, "length(c0)", "", resultPath, "verify", 0, ""));
+      inputPath, "length(c0)", "", resultPath, "verify", 0, "", ""));
 }
 
 TEST_F(ExpressionRunnerUnitTest, persistAndReproComplexSql) {