Skip to content

Commit

Permalink
Update variableArity function signature
Browse files Browse the repository at this point in the history 
Summary: This simplifies the call and makes it more readable. The number of methods should now be equal to the number of argument types.

Differential Revision: D61144588
  • Loading branch information
@facebook-github-bot
Daniel Hunte authored and facebook-github-bot committed Aug 12, 2024
1 parent 42f0a6b commit 8755b0a
Show file tree
Hide file tree
Showing 12 changed files with 64 additions and 54 deletions.
6 changes: 2 additions & 4 deletions velox/exec/tests/FunctionSignatureBuilderTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,7 @@ TEST_F(FunctionSignatureBuilderTest, scalarConstantFlags) {
.argumentType("double")
.constantArgumentType("T")
.argumentType("bigint")
.constantArgumentType("boolean")
.variableArity()
.constantVariableArity("boolean")
.build();
EXPECT_FALSE(signature->constantArguments().at(0));
EXPECT_TRUE(signature->constantArguments().at(1));
Expand Down Expand Up @@ -214,8 +213,7 @@ TEST_F(FunctionSignatureBuilderTest, aggregateConstantFlags) {
.argumentType("bigint")
.constantArgumentType("T")
.argumentType("T")
.constantArgumentType("double")
.variableArity()
.constantVariableArity("double")
.build();
EXPECT_FALSE(aggSignature->constantArguments().at(0));
EXPECT_TRUE(aggSignature->constantArguments().at(1));
Expand Down
37 changes: 37 additions & 0 deletions velox/expression/FunctionSignature.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,24 @@ class FunctionSignatureBuilder {
return *this;
}

/// Variable arity arguments can appear only at the end of the argument list
/// and can appear zero or more times.
FunctionSignatureBuilder& variableArity(const std::string& type) {
argumentTypes_.emplace_back(parseTypeSignature(type));
constantArguments_.push_back(false);
variableArity_ = true;
return *this;
}

/// Variable arity arguments can appear only at the end of the argument list
/// and can appear zero or more times.
FunctionSignatureBuilder& constantVariableArity(const std::string& type) {
argumentTypes_.emplace_back(parseTypeSignature(type));
constantArguments_.push_back(true);
variableArity_ = true;
return *this;
}

FunctionSignaturePtr build();

private:
Expand Down Expand Up @@ -395,6 +413,25 @@ class AggregateFunctionSignatureBuilder {
return *this;
}

/// Variable arity arguments can appear only at the end of the argument list
/// and can appear zero or more times.
AggregateFunctionSignatureBuilder& variableArity(const std::string& type) {
argumentTypes_.emplace_back(parseTypeSignature(type));
constantArguments_.push_back(false);
variableArity_ = true;
return *this;
}

/// Variable arity arguments can appear only at the end of the argument list
/// and can appear zero or more times.
AggregateFunctionSignatureBuilder& constantVariableArity(
const std::string& type) {
argumentTypes_.emplace_back(parseTypeSignature(type));
constantArguments_.push_back(true);
variableArity_ = true;
return *this;
}

std::shared_ptr<AggregateFunctionSignature> build();

private:
Expand Down
9 changes: 3 additions & 6 deletions velox/expression/fuzzer/ExpressionFuzzer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,7 @@ static const std::unordered_map<
// boolean, boolean,.. -> boolean
facebook::velox::exec::FunctionSignatureBuilder()
.argumentType("boolean")
.argumentType("boolean")
.variableArity()
.variableArity("boolean")
.returnType("boolean")
.build()}},
{"or",
Expand All @@ -195,8 +194,7 @@ static const std::unordered_map<
// boolean, boolean,.. -> boolean
facebook::velox::exec::FunctionSignatureBuilder()
.argumentType("boolean")
.argumentType("boolean")
.variableArity()
.variableArity("boolean")
.returnType("boolean")
.build()}},
{"coalesce",
Expand All @@ -206,8 +204,7 @@ static const std::unordered_map<
facebook::velox::exec::FunctionSignatureBuilder()
.typeVariable("T")
.argumentType("T")
.argumentType("T")
.variableArity()
.variableArity("T")
.returnType("T")
.build()}},
{
Expand Down
12 changes: 4 additions & 8 deletions velox/expression/fuzzer/tests/ArgumentTypeFuzzerTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@ TEST_F(ArgumentTypeFuzzerTest, variableArity) {
.typeVariable("X")
.returnType("X")
.argumentType("X")
.argumentType("bigint")
.variableArity()
.variableArity("bigint")
.build();

testFuzzingSuccess(signature, VARCHAR(), {VARCHAR(), BIGINT()});
Expand All @@ -193,8 +192,7 @@ TEST_F(ArgumentTypeFuzzerTest, variableArity) {
auto signature = exec::FunctionSignatureBuilder()
.knownTypeVariable("K")
.returnType("bigint")
.argumentType("K")
.variableArity()
.variableArity("K")
.build();
std::mt19937 seed{0};
ArgumentTypeFuzzer fuzzer{*signature, BIGINT(), seed};
Expand Down Expand Up @@ -434,8 +432,7 @@ TEST_F(ArgumentTypeFuzzerTest, fuzzDecimalArgumentTypes) {
.integerVariable("s")
.returnType("decimal(p,s)")
.argumentType("decimal(p,s)")
.argumentType("decimal(p,s)")
.variableArity()
.variableArity("decimal(p,s)")
.build();
argTypes = fuzzArgumentTypes(*signature, DECIMAL(10, 7));
ASSERT_LE(1, argTypes.size());
Expand Down Expand Up @@ -582,8 +579,7 @@ TEST_F(ArgumentTypeFuzzerTest, fuzzDecimalReturnType) {
.integerVariable("s")
.returnType("decimal(p,s)")
.argumentType("decimal(p,s)")
.argumentType("decimal(p,s)")
.variableArity()
.variableArity("decimal(p,s)")
.build();

returnType = fuzzReturnType(*signature);
Expand Down
6 changes: 2 additions & 4 deletions velox/expression/tests/ExprTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1424,8 +1424,7 @@ class StatefulVectorFunction : public exec::VectorFunction {
return {exec::FunctionSignatureBuilder()
.typeVariable("T")
.returnType("integer")
.argumentType("T")
.variableArity()
.variableArity("T")
.build()};
}

Expand Down Expand Up @@ -2008,8 +2007,7 @@ class NullArrayFunction : public exec::VectorFunction {
return {exec::FunctionSignatureBuilder()
.typeVariable("T")
.returnType("array(varchar)")
.argumentType("T")
.variableArity()
.variableArity("T")
.build()};
}
};
Expand Down
15 changes: 5 additions & 10 deletions velox/expression/tests/SignatureBinderTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -586,8 +586,7 @@ TEST(SignatureBinderTest, variableArity) {
{
auto signature = exec::FunctionSignatureBuilder()
.returnType("varchar")
.argumentType("varchar")
.variableArity()
.variableArity("varchar")
.build();

testSignatureBinder(signature, {}, VARCHAR());
Expand All @@ -602,8 +601,7 @@ TEST(SignatureBinderTest, variableArity) {
auto signature = exec::FunctionSignatureBuilder()
.returnType("varchar")
.argumentType("integer")
.argumentType("double")
.variableArity()
.variableArity("double")
.build();

testSignatureBinder(signature, {INTEGER()}, VARCHAR());
Expand All @@ -617,8 +615,7 @@ TEST(SignatureBinderTest, variableArity) {
{
auto signature = exec::FunctionSignatureBuilder()
.returnType("varchar")
.argumentType("any")
.variableArity()
.variableArity("any")
.build();

testSignatureBinder(signature, {}, VARCHAR());
Expand All @@ -633,8 +630,7 @@ TEST(SignatureBinderTest, variableArity) {
auto signature = exec::FunctionSignatureBuilder()
.returnType("timestamp")
.argumentType("integer")
.argumentType("any")
.variableArity()
.variableArity("any")
.build();

testSignatureBinder(signature, {INTEGER()}, TIMESTAMP());
Expand Down Expand Up @@ -683,8 +679,7 @@ TEST(SignatureBinderTest, unresolvable) {
{
auto signature = exec::FunctionSignatureBuilder()
.returnType("varchar")
.argumentType("integer")
.variableArity()
.variableArity("integer")
.build();

// wrong type
Expand Down
3 changes: 1 addition & 2 deletions velox/functions/lib/MapConcat.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,7 @@ class MapConcatFunction : public exec::VectorFunction {
.typeVariable("V")
.returnType("map(K,V)")
.argumentType("map(K,V)")
.argumentType("map(K,V)")
.variableArity()
.variableArity("map(K,V)")
.build()};
}
};
Expand Down
3 changes: 1 addition & 2 deletions velox/functions/prestosql/ArrayConstructor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ class ArrayConstructor : public exec::VectorFunction {
exec::FunctionSignatureBuilder()
.typeVariable("T")
.returnType("array(T)")
.argumentType("T")
.variableArity()
.variableArity("T")
.build(),
};
}
Expand Down
6 changes: 2 additions & 4 deletions velox/functions/prestosql/StringFunctions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,16 +258,14 @@ class ConcatFunction : public exec::VectorFunction {
.returnType("varchar")
.argumentType("varchar")
.argumentType("varchar")
.argumentType("varchar")
.variableArity()
.variableArity("varchar")
.build(),
// varbinary, varbinary,.. -> varbinary
exec::FunctionSignatureBuilder()
.returnType("varbinary")
.argumentType("varbinary")
.argumentType("varbinary")
.argumentType("varbinary")
.variableArity()
.variableArity("varbinary")
.build(),
};
}
Expand Down
12 changes: 4 additions & 8 deletions velox/functions/sparksql/Hash.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -717,8 +717,7 @@ void checkArgTypes(const std::vector<exec::VectorFunctionArg>& args) {
std::vector<std::shared_ptr<exec::FunctionSignature>> hashSignatures() {
return {exec::FunctionSignatureBuilder()
.returnType("integer")
.argumentType("any")
.variableArity()
.variableArity("any")
.build()};
}

Expand Down Expand Up @@ -748,16 +747,14 @@ std::vector<std::shared_ptr<exec::FunctionSignature>> hashWithSeedSignatures() {
return {exec::FunctionSignatureBuilder()
.returnType("integer")
.constantArgumentType("integer")
.argumentType("any")
.variableArity()
.variableArity("any")
.build()};
}

std::vector<std::shared_ptr<exec::FunctionSignature>> xxhash64Signatures() {
return {exec::FunctionSignatureBuilder()
.returnType("bigint")
.argumentType("any")
.variableArity()
.variableArity("any")
.build()};
}

Expand All @@ -766,8 +763,7 @@ xxhash64WithSeedSignatures() {
return {exec::FunctionSignatureBuilder()
.returnType("bigint")
.constantArgumentType("bigint")
.argumentType("any")
.variableArity()
.variableArity("any")
.build()};
}

Expand Down
6 changes: 2 additions & 4 deletions velox/functions/sparksql/LeastGreatest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,17 +152,15 @@ std::vector<std::shared_ptr<exec::FunctionSignature>> leastSignatures() {
signatures.emplace_back(exec::FunctionSignatureBuilder()
.returnType(type)
.argumentType(type)
.argumentType(type)
.variableArity()
.variableArity(type)
.build());
}
signatures.emplace_back(exec::FunctionSignatureBuilder()
.integerVariable("p")
.integerVariable("s")
.returnType("decimal(p,s)")
.argumentType("decimal(p,s)")
.argumentType("decimal(p,s)")
.variableArity()
.variableArity("decimal(p,s)")
.build());
return signatures;
}
Expand Down
3 changes: 1 addition & 2 deletions velox/functions/tests/FunctionRegistryTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,7 @@ TEST_F(FunctionRegistryTest, getFunctionSignatures) {
functionSignatures["variadic_func"].at(0)->toString(),
exec::FunctionSignatureBuilder()
.returnType("varchar")
.argumentType("varchar")
.variableArity()
.variableArity("varchar")
.build()
->toString());

Expand Down

0 comments on commit 8755b0a

Please sign in to comment.