From cfa113317bd2e2c28598e57d6609045141b14e9d Mon Sep 17 00:00:00 2001 From: Kevin Wilfong Date: Thu, 8 Feb 2024 16:24:05 -0800 Subject: [PATCH] Make special form names case insensitive (#8709) Summary: Pull Request resolved: https://github.com/facebookincubator/velox/pull/8709 Since SQL is case insensitive, we convert function names to lower case when registering and resolving functions. We do this for simple functions and vector functions today. This change does the same for special forms to make it consistent. Reviewed By: kagamiori Differential Revision: D53580611 fbshipit-source-id: 01819b2f0e9848da6e583d7e2eee8eb3c74d4a9c --- velox/expression/SpecialFormRegistry.cpp | 9 ++-- .../tests/FunctionCallToSpecialFormTest.cpp | 45 +++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/velox/expression/SpecialFormRegistry.cpp b/velox/expression/SpecialFormRegistry.cpp index 616378ce4152..3957981905b3 100644 --- a/velox/expression/SpecialFormRegistry.cpp +++ b/velox/expression/SpecialFormRegistry.cpp @@ -29,8 +29,10 @@ SpecialFormRegistry& specialFormRegistryInternal() { void SpecialFormRegistry::registerFunctionCallToSpecialForm( const std::string& name, std::unique_ptr functionCallToSpecialForm) { - registry_.withWLock( - [&](auto& map) { map[name] = std::move(functionCallToSpecialForm); }); + const auto sanitizedName = sanitizeName(name); + registry_.withWLock([&](auto& map) { + map[sanitizedName] = std::move(functionCallToSpecialForm); + }); } void SpecialFormRegistry::unregisterAllFunctionCallToSpecialForm() { @@ -39,9 +41,10 @@ void SpecialFormRegistry::unregisterAllFunctionCallToSpecialForm() { FunctionCallToSpecialForm* FOLLY_NULLABLE SpecialFormRegistry::getSpecialForm(const std::string& name) const { + const auto sanitizedName = sanitizeName(name); FunctionCallToSpecialForm* specialForm = nullptr; registry_.withRLock([&](const auto& map) { - auto it = map.find(name); + auto it = map.find(sanitizedName); if (it != map.end()) { specialForm = it->second.get(); } diff --git a/velox/expression/tests/FunctionCallToSpecialFormTest.cpp b/velox/expression/tests/FunctionCallToSpecialFormTest.cpp index e3b72a513ec6..7c9bd3a59022 100644 --- a/velox/expression/tests/FunctionCallToSpecialFormTest.cpp +++ b/velox/expression/tests/FunctionCallToSpecialFormTest.cpp @@ -185,3 +185,48 @@ TEST_F(FunctionCallToSpecialFormTest, notASpecialForm) { config_); ASSERT_EQ(specialForm, nullptr); } + +class FunctionCallToSpecialFormSanitizeNameTest : public testing::Test, + public VectorTestBase { + protected: + static void SetUpTestCase() { + // This class does not pre-register the special forms. + memory::MemoryManager::testingSetInstance({}); + } +}; + +TEST_F(FunctionCallToSpecialFormSanitizeNameTest, sanitizeName) { + // Make sure no special forms are registered. + unregisterAllFunctionCallToSpecialForm(); + + ASSERT_FALSE(isFunctionCallToSpecialFormRegistered("and")); + ASSERT_FALSE(isFunctionCallToSpecialFormRegistered("AND")); + ASSERT_FALSE(isFunctionCallToSpecialFormRegistered("or")); + ASSERT_FALSE(isFunctionCallToSpecialFormRegistered("OR")); + + registerFunctionCallToSpecialForm( + "and", std::make_unique(true /* isAnd */)); + registerFunctionCallToSpecialForm( + "OR", std::make_unique(false /* isAnd */)); + + auto testLookup = [this](const std::string& name) { + auto type = resolveTypeForSpecialForm(name, {BOOLEAN(), BOOLEAN()}); + ASSERT_EQ(type, BOOLEAN()); + + auto specialForm = constructSpecialForm( + name, + BOOLEAN(), + {std::make_shared( + vectorMaker_.constantVector({true})), + std::make_shared( + vectorMaker_.constantVector({false}))}, + false, + core::QueryConfig{{}}); + ASSERT_EQ(typeid(*specialForm), typeid(const ConjunctExpr&)); + }; + + testLookup("and"); + testLookup("AND"); + testLookup("or"); + testLookup("OR"); +}