From fc0a7bb093618c0c496ace40233966361656b984 Mon Sep 17 00:00:00 2001 From: Jimmy Lu Date: Tue, 12 Nov 2024 10:13:27 -0800 Subject: [PATCH] Fix data race in ExprToSubfieldFilterParser::getInstance (#11513) Summary: Pull Request resolved: https://github.com/facebookincubator/velox/pull/11513 Differential Revision: D65824456 --- velox/expression/ExprToSubfieldFilter.cpp | 25 +++++------------------ velox/expression/ExprToSubfieldFilter.h | 12 +++++++---- 2 files changed, 13 insertions(+), 24 deletions(-) diff --git a/velox/expression/ExprToSubfieldFilter.cpp b/velox/expression/ExprToSubfieldFilter.cpp index 791d30f7e6a0..54a11e065e43 100644 --- a/velox/expression/ExprToSubfieldFilter.cpp +++ b/velox/expression/ExprToSubfieldFilter.cpp @@ -112,28 +112,13 @@ toInt64List(const VectorPtr& vector, vector_size_t start, vector_size_t size) { return values; } -} // namespace - -std::function()> - ExprToSubfieldFilterParser::parserFactory_ = nullptr; +static std::shared_ptr defaultParser = + std::make_shared(); -// static -std::unique_ptr -ExprToSubfieldFilterParser::getInstance() { - if (!parserFactory_) { - parserFactory_ = []() { - return std::make_unique(); - }; - } - return parserFactory_(); -} +} // namespace -// static -void ExprToSubfieldFilterParser::registerParserFactory( - std::function()> - parserFactory) { - parserFactory_ = parserFactory; -} +std::function()> + ExprToSubfieldFilterParser::parserFactory_ = [] { return defaultParser; }; bool ExprToSubfieldFilterParser::toSubfield( const core::ITypedExpr* field, diff --git a/velox/expression/ExprToSubfieldFilter.h b/velox/expression/ExprToSubfieldFilter.h index 08578e64c607..12ea9b4c2637 100644 --- a/velox/expression/ExprToSubfieldFilter.h +++ b/velox/expression/ExprToSubfieldFilter.h @@ -418,13 +418,17 @@ class ExprToSubfieldFilterParser { public: virtual ~ExprToSubfieldFilterParser() = default; - static std::unique_ptr getInstance(); + static std::shared_ptr getInstance() { + return parserFactory_(); + } /// Registers a custom parser factory. The factory is called to create a /// parser instance. static void registerParserFactory( - std::function()> - parserFactory); + std::function()> + parserFactory) { + parserFactory_ = std::move(parserFactory); + } /// Converts a leaf call expression (no conjunction like AND/OR) to subfield /// and filter. Return nullptr if not supported for pushdown. This is needed @@ -488,7 +492,7 @@ class ExprToSubfieldFilterParser { private: // Factory method to create a parser instance. - static std::function()> + static std::function()> parserFactory_; };