Skip to content

Commit

Permalink
Make special form names case insensitive (facebookincubator#8709)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebookincubator#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
  • Loading branch information
Kevin Wilfong authored and facebook-github-bot committed Feb 9, 2024
1 parent 98c805a commit cfa1133
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 3 deletions.
9 changes: 6 additions & 3 deletions velox/expression/SpecialFormRegistry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ SpecialFormRegistry& specialFormRegistryInternal() {
void SpecialFormRegistry::registerFunctionCallToSpecialForm(
const std::string& name,
std::unique_ptr<FunctionCallToSpecialForm> 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() {
Expand All @@ -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();
}
Expand Down
45 changes: 45 additions & 0 deletions velox/expression/tests/FunctionCallToSpecialFormTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<ConjunctCallToSpecialForm>(true /* isAnd */));
registerFunctionCallToSpecialForm(
"OR", std::make_unique<ConjunctCallToSpecialForm>(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<ConstantExpr>(
vectorMaker_.constantVector<bool>({true})),
std::make_shared<ConstantExpr>(
vectorMaker_.constantVector<bool>({false}))},
false,
core::QueryConfig{{}});
ASSERT_EQ(typeid(*specialForm), typeid(const ConjunctExpr&));
};

testLookup("and");
testLookup("AND");
testLookup("or");
testLookup("OR");
}

0 comments on commit cfa1133

Please sign in to comment.