From 39df5c862221a4d143e1e0c0d5b937c15cb38437 Mon Sep 17 00:00:00 2001 From: zhaokuo Date: Wed, 3 Jul 2024 09:55:17 +0800 Subject: [PATCH] fix --- cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc | 2 +- cpp/velox/substrait/SubstraitToVeloxPlanValidator.h | 2 +- cpp/velox/utils/Common.cc | 9 +++++++-- cpp/velox/utils/Common.h | 2 +- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc index bbd8931698b90..1930fe4ed3c4c 100644 --- a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc +++ b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc @@ -162,7 +162,7 @@ bool SubstraitToVeloxPlanValidator::validateRegexExpr( const auto& pattern = patternArg.literal().string(); std::string rewrite; - if (scalarFunction.arguments().size() > 2) { + if (name == "regexp_replace " && scalarFunction.arguments().size() > 2) { const auto& rewriteArg = scalarFunction.arguments()[2].value(); if (!rewriteArg.has_literal() || !rewriteArg.literal().has_string()) { LOG_VALIDATION_MSG("Rewrite is not string literal for " + name); diff --git a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.h b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.h index 1fe174928fd96..b94ed9e3a40e9 100644 --- a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.h +++ b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.h @@ -116,7 +116,7 @@ class SubstraitToVeloxPlanValidator { /// Validates regex functions. /// Ensures the second pattern argument is a literal string. - /// Check if the pattern can pass with RE2 compilation. + /// Check if the pattern can pass with RE2 compilation and check rewriteString of regexp_replace is validate bool validateRegexExpr(const std::string& name, const ::substrait::Expression::ScalarFunction& scalarFunction); /// Validate Substrait scarlar function. diff --git a/cpp/velox/utils/Common.cc b/cpp/velox/utils/Common.cc index 491d7894c9fd7..e9ebd772ddaa9 100644 --- a/cpp/velox/utils/Common.cc +++ b/cpp/velox/utils/Common.cc @@ -52,18 +52,23 @@ std::unique_ptr compilePattern(const std::string& pattern) { return std::make_unique(re2::StringPiece(pattern), RE2::Quiet); } -bool validatePattern(const std::string& pattern, const std::string& rewrite, std::string& error) { +bool validateRe2Function(const std::string& pattern, const std::string& rewrite, std::string& error) { auto re2 = compilePattern(pattern); if (!re2->ok()) { error = "Pattern " + pattern + " compilation failed in RE2. Reason: " + re2->error(); return false; } + + if (!ensureRegexIsCompatible(pattern, error)) { + return false; + } + std::string err; if (!rewrite.empty() && !re2->CheckRewriteString(re2::StringPiece(rewrite), &err)) { error = "Rewrite " + rewrite + "check failed in RE2. Reason: " + err; return false; } - return ensureRegexIsCompatible(pattern, error); + return true; } } // namespace gluten diff --git a/cpp/velox/utils/Common.h b/cpp/velox/utils/Common.h index 5558de5cd2209..dd23afb7a7d74 100644 --- a/cpp/velox/utils/Common.h +++ b/cpp/velox/utils/Common.h @@ -29,7 +29,7 @@ namespace gluten { // Compile the given pattern and return the RE2 object. inline std::unique_ptr compilePattern(const std::string& pattern); -bool validatePattern(const std::string& pattern, const std::string& rewrite, std::string& error); +bool validateRe2Function(const std::string& pattern, const std::string& rewrite, std::string& error); static inline void fastCopy(void* dst, const void* src, size_t n) { facebook::velox::simd::memcpy(dst, src, n);