Move ExprExceptionContext to cpp file to hide exposure

facebookincubator · Oct 10, 2022 · 4d1d1b9 · 4d1d1b9
1 parent 29b8bca
commit 4d1d1b9
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 91 deletions.
diff --git a/velox/expression/Expr.cpp b/velox/expression/Expr.cpp
@@ -314,6 +314,96 @@ void Expr::evalSimplifiedImpl(
 }
 
 namespace {
+
+/// Data needed to generate exception context for the top-level expression. It
+/// also provides functionality to persist both data and sql to disk for
+/// debugging purpose
+class ExprExceptionContext {
+ public:
+  ExprExceptionContext(
+      const Expr* FOLLY_NONNULL expr,
+      const RowVector* FOLLY_NONNULL vector)
+      : expr_(expr), vector_(vector) {}
+
+  /// Persist data and sql on disk. Data will be persisted in $basePath/vector
+  /// and sql will be persisted in $basePath/sql
+  void persistDataAndSql(const char* FOLLY_NONNULL basePath) {
+    // Exception already persisted or failed to persist. We don't persist again
+    // in this situation.
+    if (!dataPath_.empty()) {
+      return;
+    }
+
+    auto dataPath = generateFilePath(basePath, "vector");
+    auto sqlPath = generateFilePath(basePath, "sql");
+    if (!dataPath.has_value()) {
+      dataPath_ = "Failed to create file for saving input vector.";
+      return;
+    }
+    if (!sqlPath.has_value()) {
+      sqlPath_ = "Failed to create file for saving expression SQL.";
+      return;
+    }
+
+    // Persist vector to disk
+    try {
+      std::ofstream outputFile(dataPath.value(), std::ofstream::binary);
+      saveVector(*vector_, outputFile);
+      outputFile.close();
+      dataPath_ = dataPath.value();
+    } catch (...) {
+      dataPath_ =
+          fmt::format("Failed to save input vector to {}.", dataPath.value());
+    }
+
+    // Persist sql to disk
+    try {
+      auto sql = expr_->toSql();
+      std::ofstream outputFile(sqlPath.value(), std::ofstream::binary);
+      outputFile.write(sql.data(), sql.size());
+      outputFile.close();
+      sqlPath_ = sqlPath.value();
+    } catch (...) {
+      sqlPath_ =
+          fmt::format("Failed to save expression SQL to {}.", sqlPath.value());
+    }
+  }
+
+  const Expr* FOLLY_NONNULL expr() const {
+    return expr_;
+  }
+
+  const RowVector* FOLLY_NONNULL vector() const {
+    return vector_;
+  }
+
+  const std::string& dataPath() const {
+    return dataPath_;
+  }
+
+  const std::string& sqlPath() const {
+    return sqlPath_;
+  }
+
+ private:
+  /// The expression.
+  const Expr* FOLLY_NONNULL expr_;
+
+  /// The input vector, i.e. EvalCtx::row(). In some cases, input columns are
+  /// re-used for results. Hence, 'vector' may no longer contain input data at
+  /// the time of exception.
+  const RowVector* FOLLY_NONNULL vector_;
+
+  /// Path of the file storing the serialized 'vector'. Used to avoid
+  /// serializing vector repeatedly in cases when multiple rows generate
+  /// exceptions. This happens when exceptions are suppressed by TRY/AND/OR.
+  std::string dataPath_{""};
+
+  /// Path of the file storing the expression SQL. Used to avoid writing SQL
+  /// repeatedly in cases when multiple rows generate exceptions.
+  std::string sqlPath_{""};
+};
+
 /// Used to generate context for an error occurred while evaluating
 /// top-level expression or top-level context for an error occurred while
 /// evaluating top-level expression. If
@@ -362,48 +452,6 @@ std::string onException(VeloxException::Type /*exceptionType*/, void* arg) {
 }
 } // namespace
 
-void ExprExceptionContext::persistDataAndSql(const char* basePath) {
-  // Exception already persisted or failed to persist. We don't persist again in
-  // this situation.
-  if (!dataPath_.empty()) {
-    return;
-  }
-
-  auto dataPath = generateFilePath(basePath, "vector");
-  auto sqlPath = generateFilePath(basePath, "sql");
-  if (!dataPath.has_value()) {
-    dataPath_ = "Failed to create file for saving input vector.";
-    return;
-  }
-  if (!sqlPath.has_value()) {
-    sqlPath_ = "Failed to create file for saving expression SQL.";
-    return;
-  }
-
-  // Persist vector to disk
-  try {
-    std::ofstream outputFile(dataPath.value(), std::ofstream::binary);
-    saveVector(*vector_, outputFile);
-    outputFile.close();
-    dataPath_ = dataPath.value();
-  } catch (...) {
-    dataPath_ =
-        fmt::format("Failed to save input vector to {}.", dataPath.value());
-  }
-
-  // Persist sql to disk
-  try {
-    auto sql = expr_->toSql();
-    std::ofstream outputFile(sqlPath.value(), std::ofstream::binary);
-    outputFile.write(sql.data(), sql.size());
-    outputFile.close();
-    sqlPath_ = sqlPath.value();
-  } catch (...) {
-    sqlPath_ =
-        fmt::format("Failed to save expression SQL to {}.", sqlPath.value());
-  }
-}
-
 void Expr::evalFlatNoNulls(
     const SelectivityVector& rows,
     EvalCtx& context,

diff --git a/velox/expression/Expr.h b/velox/expression/Expr.h
@@ -71,55 +71,6 @@ struct ExprStats {
   }
 };
 
-/// Data needed to generate exception context for the top-level expression. It
-/// also provides functionality to persist both data and sql to disk for
-/// debugging purpose
-class ExprExceptionContext {
- public:
-  ExprExceptionContext(
-      const Expr* FOLLY_NONNULL expr,
-      const RowVector* FOLLY_NONNULL vector)
-      : expr_(expr), vector_(vector) {}
-
-  /// Persist data and sql on disk. Data will be persisted in $basePath/vector
-  /// and sql will be persisted in $basePath/sql
-  void persistDataAndSql(const char* FOLLY_NONNULL basePath);
-
-  const Expr* FOLLY_NONNULL expr() const {
-    return expr_;
-  }
-
-  const RowVector* FOLLY_NONNULL vector() const {
-    return vector_;
-  }
-
-  const std::string& dataPath() const {
-    return dataPath_;
-  }
-
-  const std::string& sqlPath() const {
-    return sqlPath_;
-  }
-
- private:
-  /// The expression.
-  const Expr* FOLLY_NONNULL expr_;
-
-  /// The input vector, i.e. EvalCtx::row(). In some cases, input columns are
-  /// re-used for results. Hence, 'vector' may no longer contain input data at
-  /// the time of exception.
-  const RowVector* FOLLY_NONNULL vector_;
-
-  /// Path of the file storing the serialized 'vector'. Used to avoid
-  /// serializing vector repeatedly in cases when multiple rows generate
-  /// exceptions. This happens when exceptions are suppressed by TRY/AND/OR.
-  std::string dataPath_{""};
-
-  /// Path of the file storing the expression SQL. Used to avoid writing SQL
-  /// repeatedly in cases when multiple rows generate exceptions.
-  std::string sqlPath_{""};
-};
-
 // An executable expression.
 class Expr {
  public: