From 96188a16bf7873ce201b6fc7f1a6ce4f1bf7dbfc Mon Sep 17 00:00:00 2001 From: Philo He Date: Sun, 3 May 2026 13:18:45 -0700 Subject: [PATCH 1/2] Initial --- .../SubstraitToVeloxPlanValidator.cc | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc index b175e2aedb7..1f716062557 100644 --- a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc +++ b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc @@ -1153,7 +1153,7 @@ bool SubstraitToVeloxPlanValidator::validateAggRelFunctionType(const ::substrait auto funcName = planConverter_->toAggregationFunctionName(baseFuncName, funcStep, resultType); auto signaturesOpt = exec::getAggregateFunctionSignatures(funcName); if (!signaturesOpt) { - LOG_VALIDATION_MSG("can not find function signature for " + funcName + " in AggregateRel."); + LOG_VALIDATION_MSG("No function signatures found for function name: " + funcName); return false; } @@ -1179,8 +1179,8 @@ bool SubstraitToVeloxPlanValidator::validateAggRelFunctionType(const ::substrait } if (resolveType == nullptr) { - LOG_VALIDATION_MSG("Validation failed for function " + funcName + " resolve type in AggregateRel."); - return false; + LOG_VALIDATION_MSG("Failed to resolve intermediate/return type for function " + funcName); + break; } resolved = true; @@ -1188,7 +1188,23 @@ bool SubstraitToVeloxPlanValidator::validateAggRelFunctionType(const ::substrait } } if (!resolved) { - LOG_VALIDATION_MSG("Validation failed for function " + funcName + " bind signatures in AggregateRel."); + std::vector inputTypeStrs; + for (const auto& type : types) { + inputTypeStrs.push_back(type->toString()); + } + + std::vector signatureStrs; + for (const auto& signature : signaturesOpt.value()) { + signatureStrs.push_back(signature->toString()); + } + + LOG_VALIDATION_MSG(fmt::format( + "Validation failed for function {} when binding signatures.\n" + " Input types: [{}]\n" + " Registered signatures:\n {}", + funcName, + fmt::join(inputTypeStrs, ", "), + fmt::join(signatureStrs, "\n "))); return false; } } From dc7bfd429e9d9339bcd0e1cb39c7b982cdb096fc Mon Sep 17 00:00:00 2001 From: Philo He Date: Sun, 3 May 2026 13:51:51 -0700 Subject: [PATCH 2/2] Fix compile error --- cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc index 1f716062557..6b10b805ed1 100644 --- a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc +++ b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc @@ -1203,8 +1203,8 @@ bool SubstraitToVeloxPlanValidator::validateAggRelFunctionType(const ::substrait " Input types: [{}]\n" " Registered signatures:\n {}", funcName, - fmt::join(inputTypeStrs, ", "), - fmt::join(signatureStrs, "\n "))); + folly::join(", ", inputTypeStrs), + folly::join("\n ", signatureStrs))); return false; } }