Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(fuzzer): Mark IPADDRESS and IPPREFIX types as unsuppported input types in PrestoQueryRunner #11820

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
8 changes: 8 additions & 0 deletions velox/exec/fuzzer/ToSQLUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,14 @@ std::string toCallSql(const core::CallTypedExprPtr& call) {
sql << "ARRAY[";
toCallInputsSql(call->inputs(), sql);
sql << "]";
} else if (call->name() == "between") {
const auto& inputs = call->inputs();
VELOX_CHECK_EQ(inputs.size(), 3);
toCallInputsSql({inputs[0]}, sql);
sql << " between ";
toCallInputsSql({inputs[1]}, sql);
sql << " and ";
toCallInputsSql({inputs[2]}, sql);
} else {
// Regular function call syntax.
sql << call->name() << "(";
Expand Down
Loading