From a775a6ce97f54ce9a4369be5d8ac75301d238c94 Mon Sep 17 00:00:00 2001 From: Wei He Date: Thu, 12 Dec 2024 04:20:32 -0800 Subject: [PATCH] fix(fuzzer): Mark IPADDRESS and IPPREFIX types as unsuppported input types in PrestoQueryRunner (#11820) Summary: Pull Request resolved: https://github.com/facebookincubator/velox/pull/11820 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. Reviewed By: kevinwilfong Differential Revision: D67060600 fbshipit-source-id: 4f5aa7770b9ef7693b534c2e3b3b568fdc34f3aa --- 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 036957964cc8..b5fc55d92cb0 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 51314d60a0ee..ca289a04458e 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 3ab8689030da..37f31869541d 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(