Fix CAST(interval day to second as varchar) (#9386)

Summary: Pull Request resolved: #9386 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 #9384 Reviewed By: xiaoxmeng Differential Revision: D55796600 fbshipit-source-id: 3d51a8eb18b434315bf9285f58b8f2cdbedca63d
facebookincubator · Apr 5, 2024 · 41bed84 · 41bed84
1 parent 00f9fc5
commit 41bed84
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 4 deletions.
diff --git a/velox/expression/CastExpr.cpp b/velox/expression/CastExpr.cpp
@@ -159,10 +159,11 @@ VectorPtr CastExpr::castFromDate(
       auto* resultFlatVector = castResult->as<FlatVector<StringView>>();
       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());
-          std::memcpy(writer.data(), output.data(), output.size());
+          ::memcpy(writer.data(), output.data(), output.size());
           writer.finalize();
         } catch (const VeloxException& ue) {
           if (!ue.isUserError()) {
@@ -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<SimpleVector<int64_t>>();
+  switch (toType->kind()) {
+    case TypeKind::VARCHAR: {
+      auto* resultFlatVector = castResult->as<FlatVector<StringView>>();
+      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());
+          ::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<int64_t>(rows, input, context, fromType, toType);
   } else if (toType->isLongDecimal()) {

diff --git 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 <typename TInput, typename TOutput>
   void applyDecimalCastKernel(
       const SelectivityVector& rows,

diff --git a/velox/expression/ConstantExpr.cpp b/velox/expression/ConstantExpr.cpp
@@ -160,11 +160,25 @@ void appendSqlLiteral(
       break;
     }
     case TypeKind::TINYINT:
+      [[fallthrough]];
     case TypeKind::SMALLINT:
-    case TypeKind::BIGINT:
+      [[fallthrough]];
+    case TypeKind::BIGINT: {
+      if (vector.type()->isIntervalDayTime()) {
+        auto* intervalVector =
+            vector.wrappedVector()->as<SimpleVector<int64_t>>();
+        out << "INTERVAL " << intervalVector->valueAt(vector.wrappedIndex(row))
+            << " MILLISECOND";
+        break;
+      }
+      [[fallthrough]];
+    }
     case TypeKind::HUGEINT:
+      [[fallthrough]];
     case TypeKind::TIMESTAMP:
+      [[fallthrough]];
     case TypeKind::REAL:
+      [[fallthrough]];
     case TypeKind::DOUBLE:
       out << "'" << vector.wrappedVector()->toString(vector.wrappedIndex(row))
           << "'::" << vector.type()->toString();

diff --git 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<int64_t>(
+          {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<std::string>({
+      "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
diff --git a/velox/expression/tests/ExprTest.cpp b/velox/expression/tests/ExprTest.cpp
@@ -2635,8 +2635,7 @@ TEST_P(ParameterizedExprTest, constantToSql) {
   ASSERT_EQ(toSql(variant::null(TypeKind::TIMESTAMP)), "NULL::TIMESTAMP");
 
   ASSERT_EQ(
-      toSql(123'456LL, INTERVAL_DAY_TIME()),
-      "'123456'::INTERVAL DAY TO SECOND");
+      toSql(123'456LL, INTERVAL_DAY_TIME()), "INTERVAL 123456 MILLISECOND");
   ASSERT_EQ(
       toSql(variant::null(TypeKind::BIGINT), INTERVAL_DAY_TIME()),
       "NULL::INTERVAL DAY TO SECOND");