Skip to content

Commit

Permalink
fix(fuzzer): Mark IPADDRESS and IPPREFIX types as unsuppported input …
Browse files Browse the repository at this point in the history
…types in PrestoQueryRunner (facebookincubator#11820)

Summary:
Pull Request resolved: facebookincubator#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.

Differential Revision: D67060600
  • Loading branch information
kagamiori authored and facebook-github-bot committed Dec 11, 2024
1 parent 4a1de44 commit b4faa23
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 14 deletions.
14 changes: 10 additions & 4 deletions velox/exec/fuzzer/FuzzerUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions velox/exec/fuzzer/FuzzerUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
26 changes: 16 additions & 10 deletions velox/exec/fuzzer/PrestoQueryRunner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -424,16 +426,18 @@ bool PrestoQueryRunner::isConstantExprSupported(
const core::TypedExprPtr& expr) {
if (std::dynamic_pointer_cast<const core::ConstantTypedExpr>(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;
}
Expand All @@ -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<std::string> PrestoQueryRunner::toSql(
Expand Down

0 comments on commit b4faa23

Please sign in to comment.