From b34a61e3ab3c1887c9d0cc8a9cf4679810bf4130 Mon Sep 17 00:00:00 2001 From: Masha Basmanova Date: Fri, 5 Apr 2024 05:19:25 -0700 Subject: [PATCH] Fix CAST(interval day to second as varchar) Summary: CAST expression used to handle INTERVAL DAY TO SECOND values as BIGINT. This allowed invalid casts (e.g. from varchar, to double, etc.) and produced incorrect results when casting to varchar. Casting 1 second interval used to return '1000' instead of '0 00:00:01.000'. Fixes https://github.com/facebookincubator/velox/issues/9384 Differential Revision: D55796600 --- velox/expression/CastExpr.cpp | 51 +++++++++++++++++++++++++ velox/expression/CastExpr.h | 6 +++ velox/expression/tests/CastExprTest.cpp | 30 +++++++++++++++ 3 files changed, 87 insertions(+) diff --git a/velox/expression/CastExpr.cpp b/velox/expression/CastExpr.cpp index 2a27789e5ce9..9ab1b62f3b0a 100644 --- a/velox/expression/CastExpr.cpp +++ b/velox/expression/CastExpr.cpp @@ -159,6 +159,7 @@ VectorPtr CastExpr::castFromDate( auto* resultFlatVector = castResult->as>(); applyToSelectedNoThrowLocal(context, rows, castResult, [&](int row) { try { + // TODO Optimize to avoid creating an intermediate string. auto output = DATE()->toString(inputFlatVector->valueAt(row)); auto writer = exec::StringWriter<>(resultFlatVector, row); writer.resize(output.size()); @@ -247,6 +248,49 @@ VectorPtr CastExpr::castToDate( } } +VectorPtr CastExpr::castFromIntervalDayTime( + const SelectivityVector& rows, + const BaseVector& input, + exec::EvalCtx& context, + const TypePtr& toType) { + VectorPtr castResult; + context.ensureWritable(rows, toType, castResult); + (*castResult).clearNulls(rows); + + auto* inputFlatVector = input.as>(); + switch (toType->kind()) { + case TypeKind::VARCHAR: { + auto* resultFlatVector = castResult->as>(); + applyToSelectedNoThrowLocal(context, rows, castResult, [&](int row) { + try { + // TODO Optimize to avoid creating an intermediate string. + auto output = + INTERVAL_DAY_TIME()->valueToString(inputFlatVector->valueAt(row)); + auto writer = exec::StringWriter<>(resultFlatVector, row); + writer.resize(output.size()); + std::memcpy(writer.data(), output.data(), output.size()); + writer.finalize(); + } catch (const VeloxException& ue) { + if (!ue.isUserError()) { + throw; + } + VELOX_USER_FAIL( + makeErrorMessage(input, row, toType) + " " + ue.message()); + } catch (const std::exception& e) { + VELOX_USER_FAIL( + makeErrorMessage(input, row, toType) + " " + e.what()); + } + }); + return castResult; + } + default: + VELOX_UNSUPPORTED( + "Cast from {} to {} is not supported", + INTERVAL_DAY_TIME()->toString(), + toType->toString()); + } +} + namespace { void propagateErrorsOrSetNulls( bool setNullInResultAtError, @@ -677,6 +721,13 @@ void CastExpr::applyPeeled( result = castFromDate(rows, input, context, toType); } else if (toType->isDate()) { result = castToDate(rows, input, context, fromType); + } else if (fromType->isIntervalDayTime()) { + result = castFromIntervalDayTime(rows, input, context, toType); + } else if (toType->isIntervalDayTime()) { + VELOX_UNSUPPORTED( + "Cast from {} to {} is not supported", + fromType->toString(), + toType->toString()); } else if (toType->isShortDecimal()) { result = applyDecimal(rows, input, context, fromType, toType); } else if (toType->isLongDecimal()) { diff --git a/velox/expression/CastExpr.h b/velox/expression/CastExpr.h index b4446b158ad9..e6af1f82a529 100644 --- a/velox/expression/CastExpr.h +++ b/velox/expression/CastExpr.h @@ -189,6 +189,12 @@ class CastExpr : public SpecialForm { exec::EvalCtx& context, const TypePtr& fromType); + VectorPtr castFromIntervalDayTime( + const SelectivityVector& rows, + const BaseVector& input, + exec::EvalCtx& context, + const TypePtr& toType); + template void applyDecimalCastKernel( const SelectivityVector& rows, diff --git a/velox/expression/tests/CastExprTest.cpp b/velox/expression/tests/CastExprTest.cpp index e78917e2bb19..25f904a08146 100644 --- a/velox/expression/tests/CastExprTest.cpp +++ b/velox/expression/tests/CastExprTest.cpp @@ -2582,5 +2582,35 @@ TEST_F(CastExprTest, skipCastEvaluation) { assertEqualVectors(result, expected); } } + +TEST_F(CastExprTest, intervalDayTimeToVarchar) { + auto data = makeRowVector({ + makeFlatVector( + {kMillisInDay, + kMillisInHour, + kMillisInMinute, + kMillisInSecond, + 5 * kMillisInDay + 14 * kMillisInHour + 20 * kMillisInMinute + + 52 * kMillisInSecond + 88}, + INTERVAL_DAY_TIME()), + }); + + auto result = evaluate("cast(c0 as varchar)", data); + auto expected = makeFlatVector({ + "1 00:00:00.000", + "0 01:00:00.000", + "0 00:01:00.000", + "0 00:00:01.000", + "5 14:20:52.088", + }); + + assertEqualVectors(result, expected); + + // Reverse cast is not supported. + VELOX_ASSERT_THROW( + evaluate("cast('5 14:20:52.088' as interval day to second)", data), + "Cast from VARCHAR to INTERVAL DAY TO SECOND is not supported"); +} + } // namespace } // namespace facebook::velox::test