From ff11d28c2c8ee10b2b4b1af5e93ddff61ec60b9d Mon Sep 17 00:00:00 2001 From: Wei He Date: Tue, 10 Dec 2024 15:58:45 -0800 Subject: [PATCH] fix(fuzzer): Mark IPADDRESS and IPPREFIX types as unsuppported input types in PrestoQueryRunner Summary: Mark IPADDRES and IPPREFIX types as unsuppported input types in PrestoQueryRunner because Presto doesn't allow creating HIVE columns of these types and requires constant literals of these types to be valid strings. Differential Revision: D67060600 --- velox/exec/fuzzer/FuzzerUtil.cpp | 14 +++++++++---- velox/exec/fuzzer/FuzzerUtil.h | 6 ++++++ velox/exec/fuzzer/PrestoQueryRunner.cpp | 26 +++++++++++++++---------- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/velox/exec/fuzzer/FuzzerUtil.cpp b/velox/exec/fuzzer/FuzzerUtil.cpp index 036957964cc8c..b5fc55d92cb04 100644 --- a/velox/exec/fuzzer/FuzzerUtil.cpp +++ b/velox/exec/fuzzer/FuzzerUtil.cpp @@ -260,12 +260,9 @@ bool containTypeName( return false; } -bool usesTypeName( +bool usesInputTypeName( const exec::FunctionSignature& signature, const std::string& typeName) { - if (containTypeName(signature.returnType(), typeName)) { - return true; - } for (const auto& argument : signature.argumentTypes()) { if (containTypeName(argument, typeName)) { return true; @@ -274,6 +271,15 @@ bool usesTypeName( return false; } +bool usesTypeName( + const exec::FunctionSignature& signature, + const std::string& typeName) { + if (containTypeName(signature.returnType(), typeName)) { + return true; + } + return usesInputTypeName(signature, typeName); +} + // If 'type' is a RowType or contains RowTypes with empty field names, adds // default names to these fields in the RowTypes. TypePtr sanitize(const TypePtr& type) { diff --git a/velox/exec/fuzzer/FuzzerUtil.h b/velox/exec/fuzzer/FuzzerUtil.h index 51314d60a0ee6..ca289a04458e8 100644 --- a/velox/exec/fuzzer/FuzzerUtil.h +++ b/velox/exec/fuzzer/FuzzerUtil.h @@ -93,6 +93,12 @@ RowTypePtr concat(const RowTypePtr& a, const RowTypePtr& b); /// TODO Investigate mismatches reported when comparing Varbinary. bool containsUnsupportedTypes(const TypePtr& type); +/// Determines whether the signature has an argument that contains typeName. +/// typeName should be in lower case. +bool usesInputTypeName( + const exec::FunctionSignature& signature, + const std::string& typeName); + /// Determines whether the signature has an argument or return type that /// contains typeName. typeName should be in lower case. bool usesTypeName( diff --git a/velox/exec/fuzzer/PrestoQueryRunner.cpp b/velox/exec/fuzzer/PrestoQueryRunner.cpp index 3ab8689030da1..37f31869541d3 100644 --- a/velox/exec/fuzzer/PrestoQueryRunner.cpp +++ b/velox/exec/fuzzer/PrestoQueryRunner.cpp @@ -30,6 +30,8 @@ #include "velox/exec/fuzzer/FuzzerUtil.h" #include "velox/exec/fuzzer/ToSQLUtil.h" #include "velox/exec/tests/utils/QueryAssertions.h" +#include "velox/functions/prestosql/types/IPAddressType.h" +#include "velox/functions/prestosql/types/IPPrefixType.h" #include "velox/functions/prestosql/types/JsonType.h" #include "velox/serializers/PrestoSerializer.h" #include "velox/type/parser/TypeParser.h" @@ -424,16 +426,18 @@ bool PrestoQueryRunner::isConstantExprSupported( const core::TypedExprPtr& expr) { if (std::dynamic_pointer_cast(expr)) { // TODO: support constant literals of these types. Complex-typed constant - // literals require support of converting them to SQL. Json can be enabled - // after we're able to generate valid Json strings, because when Json is - // used as the type of constant literals in SQL, Presto implicitly invoke - // json_parse() on it, which makes the behavior of Presto different from - // Velox. Timestamp constant literals require further investigation to + // literals require support of converting them to SQL. Json, Ipaddress, and + // Ipprefix can be enabled after we're able to generate valid input values, + // because when these types are used as the type of a constant literal in + // SQL, Presto implicitly invoke json_parse(), cast(x as Ipaddress), and + // cast(x as Ipprefix) on it, which makes the behavior of Presto different + // from Velox. Timestamp constant literals require further investigation to // ensure Presto uses the same timezone as Velox. Interval type cannot be // used as the type of constant literals in Presto SQL. auto& type = expr->type(); return type->isPrimitiveType() && !type->isTimestamp() && - !isJsonType(type) && !type->isIntervalDayTime(); + !isJsonType(type) && !type->isIntervalDayTime() && + !isIPAddressType(type) && !isIPPrefixType(type); } return true; } @@ -444,14 +448,16 @@ bool PrestoQueryRunner::isSupported(const exec::FunctionSignature& signature) { // cast-to or constant literals. Hyperloglog can only be casted from varbinary // and cannot be used as the type of constant literals. Interval year to month // can only be casted from NULL and cannot be used as the type of constant - // literals. Json requires special handling, because Presto requires Json - // literals to be valid Json strings, and doesn't allow creation of Json-typed - // HIVE columns. + // literals. Json, Ipaddress, and Ipprefix require special handling, because + // Presto requires literals of these types to be valid, and doesn't allow + // creating HIVE columns of these types. return !( usesTypeName(signature, "interval year to month") || usesTypeName(signature, "hugeint") || usesTypeName(signature, "hyperloglog") || - usesTypeName(signature, "json")); + usesInputTypeName(signature, "json") || + usesInputTypeName(signature, "ipaddress") || + usesInputTypeName(signature, "ipprefix")); } std::optional PrestoQueryRunner::toSql(