From 0c9f625f73c178a63f4b2c03b9856c93dd08537a Mon Sep 17 00:00:00 2001 From: tanjialiang Date: Tue, 11 Oct 2022 15:06:38 -0700 Subject: [PATCH] Move ExprExceptionContext to cpp file to hide exposure (#2803) Summary: Pull Request resolved: https://github.com/facebookincubator/velox/pull/2803 Reviewed By: kagamiori, mbasmanova Differential Revision: D40233624 Pulled By: tanjialiang fbshipit-source-id: b5a408d0f568c5b1b70f7ce8a8b5729554c09429 --- velox/expression/Expr.cpp | 132 ++++++++++++++++++++++++++------------ velox/expression/Expr.h | 49 -------------- 2 files changed, 90 insertions(+), 91 deletions(-) diff --git a/velox/expression/Expr.cpp b/velox/expression/Expr.cpp index 931365269c03..cca49778062f 100644 --- 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 index 5f90c92c4f33..e5e04626f8c9 100644 --- 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: