Skip to content

Commit

Permalink
Make varchar and varbinary compatible (oap-project#115)
Browse files Browse the repository at this point in the history
  • Loading branch information
rui-mo authored and zhejiangxiaomai committed Apr 17, 2023
1 parent 3638f2a commit 44e8e55
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 14 deletions.
3 changes: 2 additions & 1 deletion velox/connectors/hive/HiveConnector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,8 @@ bool testFilters(
template <TypeKind ToKind>
velox::variant convertFromString(const std::optional<std::string>& value) {
if (value.has_value()) {
if constexpr (ToKind == TypeKind::VARCHAR) {
// No need for casting if ToKind is VARCHAR or VARBINARY.
if constexpr (ToKind == TypeKind::VARCHAR || ToKind == TypeKind::VARBINARY) {
return velox::variant(value.value());
}
bool nullOutput = false;
Expand Down
12 changes: 0 additions & 12 deletions velox/substrait/SubstraitToVeloxExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -445,18 +445,6 @@ SubstraitVeloxExprConverter::toVeloxExpr(

std::vector<core::TypedExprPtr> inputs{
toVeloxExpr(castExpr.input(), inputType)};

// TODO: refactor this part for input type validation.
for (const auto& input : inputs) {
switch (input->type()->kind()) {
case TypeKind::ARRAY: {
// Cast from array type is not supported. See CastExpr::applyCast.
VELOX_UNSUPPORTED("Invalid from type in casting: {}", input->type());
}
default: {
}
}
}
return std::make_shared<core::CastTypedExpr>(type, inputs, nullOnFailure);
}

Expand Down
24 changes: 24 additions & 0 deletions velox/substrait/SubstraitToVeloxPlanValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,28 @@ bool SubstraitToVeloxPlanValidator::validateLiteral(
return true;
}

bool SubstraitToVeloxPlanValidator::validateCast(
const ::substrait::Expression::Cast& castExpr,
const RowTypePtr& inputType) {
std::vector<core::TypedExprPtr> inputs{
exprConverter_->toVeloxExpr(castExpr.input(), inputType)};

// Casting from some types is not supported. See CastExpr::applyCast.
for (const auto& input : inputs) {
switch (input->type()->kind()) {
case TypeKind::ARRAY:
case TypeKind::MAP:
case TypeKind::ROW:
case TypeKind::VARBINARY:
VLOG(1) << "Invalid input type in casting: " << input->type() << ".";
return false;
default: {
}
}
}
return true;
}

bool SubstraitToVeloxPlanValidator::validateExpression(
const ::substrait::Expression& expression,
const RowTypePtr& inputType) {
Expand All @@ -104,6 +126,8 @@ bool SubstraitToVeloxPlanValidator::validateExpression(
return validateScalarFunction(expression.scalar_function(), inputType);
case ::substrait::Expression::RexTypeCase::kLiteral:
return validateLiteral(expression.literal(), inputType);
case ::substrait::Expression::RexTypeCase::kCast:
return validateCast(expression.cast(), inputType);
default:
return true;
}
Expand Down
5 changes: 5 additions & 0 deletions velox/substrait/SubstraitToVeloxPlanValidator.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ class SubstraitToVeloxPlanValidator {
const ::substrait::Expression::ScalarFunction& scalarFunction,
const RowTypePtr& inputType);

/// Validate Substrait Cast expression.
bool validateCast(
const ::substrait::Expression::Cast& castExpr,
const RowTypePtr& inputType);

/// Validate Substrait expression.
bool validateExpression(
const ::substrait::Expression& expression,
Expand Down
5 changes: 4 additions & 1 deletion velox/vector/BaseVector.h
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,10 @@ class BaseVector {
// two unknowns but values cannot be assigned into an unknown 'left'
// from a not-unknown 'right'.
static bool compatibleKind(TypeKind left, TypeKind right) {
return left == right || right == TypeKind::UNKNOWN;
// Vectors of VARCHAR and VARBINARY are compatible with each other.
bool varcharAndBinary = (left == TypeKind::VARCHAR && right == TypeKind::VARBINARY) ||
(left == TypeKind::VARBINARY && right == TypeKind::VARCHAR);
return left == right || right == TypeKind::UNKNOWN || varcharAndBinary;
}

/// Returns a brief summary of the vector. If 'recursive' is true, includes a
Expand Down

0 comments on commit 44e8e55

Please sign in to comment.