diff --git a/velox/core/SimpleFunctionMetadata.h b/velox/core/SimpleFunctionMetadata.h index af51c01462cc..842333f33137 100644 --- a/velox/core/SimpleFunctionMetadata.h +++ b/velox/core/SimpleFunctionMetadata.h @@ -394,7 +394,9 @@ class ISimpleFunctionMetadata { virtual bool physicalSignatureEquals( const ISimpleFunctionMetadata& other) const = 0; virtual std::string helpMessage(const std::string& name) const = 0; - virtual std::string toString(const std::string& name) const = 0; + /// Returns string of all the fields such as logical signature, + /// physical signature and priority. + virtual std::string toDebugString() const = 0; }; template @@ -537,40 +539,19 @@ class SimpleFunctionMetadata : public ISimpleFunctionMetadata { return s; } - std::string toString(const std::string& name) const final { - std::stringstream sig; - bool first = true; - for (auto& arg : signature_->argumentTypes()) { - if (!first) { - sig << ", "; - } - first = false; - sig << boost::algorithm::to_upper_copy(arg.toString()); - } + std::string toDebugString() const final { + auto logicalArguments = + argumentToString(signature_->argumentTypes()); - if (isVariadic()) { - sig << "..."; - } + auto physicalArguments = argumentToString(argPhysicalTypes_); - std::stringstream ss; - first = true; - for (const auto& arg : argPhysicalTypes_) { - if (!first) { - ss << ", "; - } - first = false; - ss << arg->toString(); - } - if (isVariadic()) { - ss << "..."; - } return fmt::format( - "FunctionName: {}\nSignature argument types:\n{}\nPhysical argument types:\n{}" - "\nPhysical result types:\n{}\nPriority:{}\nDefaultNullBehavior:{}", - name, - sig.str(), - ss.str(), - resultPhysicalType_, + "Logical signature: ({}) -> {}\nPhysical signature: ({}) -> {}\n" + "Priority: {}\nDefaultNullBehavior: {}", + logicalArguments, + signature_->returnType().toString(), + physicalArguments, + resultPhysicalType_->toString(), priority_, defaultNullBehavior_); } @@ -648,6 +629,27 @@ class SimpleFunctionMetadata : public ISimpleFunctionMetadata { signature_ = builder.build(); } + template + static std::string argumentToString(const std::vector& arguments) { + std::stringstream ss; + bool first = true; + for (const auto& arg : arguments) { + if (!first) { + ss << ", "; + } + first = false; + if constexpr (std::is_same_v) { + ss << arg.toString(); + } else { + ss << arg->toString(); + } + } + if (isVariadic()) { + ss << "..."; + } + return ss.str(); + } + const bool defaultNullBehavior_; exec::FunctionSignaturePtr signature_; uint32_t priority_; @@ -878,9 +880,9 @@ class UDFHolder { // null, without calling the function implementation. static constexpr bool is_default_null_behavior = !udf_has_callNullable; - // If any of the the provided "call" flavors can produce null (in case any of - // them return bool). This is only false if all the call methods provided for - // a function return void. + // If any of the the provided "call" flavors can produce null (in case any + // of them return bool). This is only false if all the call methods provided + // for a function return void. static constexpr bool can_produce_null_output = udf_has_call_return_bool | udf_has_callNullable_return_bool | udf_has_callNullFree_return_bool | udf_has_callAscii_return_bool; diff --git a/velox/expression/SimpleFunctionRegistry.h b/velox/expression/SimpleFunctionRegistry.h index 3e4df252daa5..61c77a23c3f0 100644 --- a/velox/expression/SimpleFunctionRegistry.h +++ b/velox/expression/SimpleFunctionRegistry.h @@ -132,8 +132,8 @@ class SimpleFunctionRegistry { return functionEntry_.getMetadata().helpMessage(name); } - std::string toString(const std::string& name) const { - return functionEntry_.getMetadata().toString(name); + std::string toDebugString() const { + return functionEntry_.getMetadata().toDebugString(); } VectorFunctionMetadata metadata() const { diff --git a/velox/expression/tests/SimpleFunctionTest.cpp b/velox/expression/tests/SimpleFunctionTest.cpp index 6a67c0feeaa3..b800cf1a712e 100644 --- a/velox/expression/tests/SimpleFunctionTest.cpp +++ b/velox/expression/tests/SimpleFunctionTest.cpp @@ -1631,21 +1631,28 @@ struct DecimalPlusValueFunction { int8_t scale_; }; -TEST_F(SimpleFunctionTest, toString) { - const std::string functionName = "decimal_plus_value"; +TEST_F(SimpleFunctionTest, toDebugString) { + const std::string functionName = "decimal_plus_value_debug_string"; registerFunction< DecimalPlusValueFunction, + LongDecimal, LongDecimal, - LongDecimal, - int64_t>({functionName}); - auto function = exec::simpleFunctions().resolveFunction( - functionName, {DECIMAL(20, 2), BIGINT()}); - EXPECT_TRUE(function.has_value()); + int32_t>( + {functionName}, + {exec::SignatureVariable( + P2::name(), + fmt::format("{a_precision} + 1", fmt::arg("a_precision", P1::name())), + exec::ParameterType::kIntegerParameter)}); + + auto resolved = exec::simpleFunctions().resolveFunction( + functionName, {DECIMAL(20, 2), INTEGER()}); + + ASSERT_TRUE(resolved.has_value()); EXPECT_EQ( - function.value().toString(functionName), - "FunctionName: decimal_plus_value\nSignature argument types:\nDECIMAL(I1,I5), BIGINT\n" - "Physical argument types:\nHUGEINT, BIGINT\nPhysical result types:\nHUGEINT\n" - "Priority:999997\nDefaultNullBehavior:true"); + resolved.value().toDebugString(), + "Logical signature: (decimal(i1,i5), integer) -> decimal(i2,i5)\n" + "Physical signature: (HUGEINT, INTEGER) -> HUGEINT\n" + "Priority: 999997\nDefaultNullBehavior: true"); } } // namespace