velox icon indicating copy to clipboard operation
velox copied to clipboard

Published 20 hours ago • verified publisher icon facebookincubator

Add Spark CAST(integral as timestamp)

Open boneanxs opened this issue 1 year ago • 9 comments

Add Spark CAST (integral as timestamp). The input value is treated as the number of seconds since the epoch (1970-01-01 00:00:00 UTC). Supported types are tinyint, smallint, integer and bigint.

Spark's implementation: https://github.com/apache/spark/blob/v3.5.1/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala#L680

boneanxs avatar Sep 25 '24 06:09 boneanxs

Deploy Preview for meta-velox canceled.

Name Link
Latest commit 7166539adca3af0f930240bc9a39c4c5a4711c87
Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/6719ad6e2fdd440008a97fa2

netlify[bot] avatar Sep 25 '24 06:09 netlify[bot]

Hey @rui-mo, could you please help review this?

boneanxs avatar Sep 25 '24 09:09 boneanxs

cc: @pedroerp @mbasmanova If you have any more comment. Thanks!

rui-mo avatar Oct 12 '24 03:10 rui-mo

Thank you guys, I'll get this one merged.

pedroerp avatar Oct 23 '24 02:10 pedroerp

@pedroerp has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Oct 23 '24 02:10 facebook-github-bot

@boneanxs looks like this is triggering a test failure (velox/functions/sparksql/tests:velox_functions_spark_test - SparkCastExprTest.intToTimestamp):

Test was added in target revision (54f19147c915f03eaf17f0531ce7ac00954fc585) hence is not present in base rev (2d4890ad47b2c140867328778d7102733cd1cb32)
Note: Google Test filter = SparkCastExprTest.intToTimestamp
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from SparkCastExprTest
[ RUN      ] SparkCastExprTest.intToTimestamp

buck-out/v2/gen/fbcode/b321bd6172db5152/velox/type/__velox_timestamp__/buck-headers/velox/type/Timestamp.h:242:35: runtime error: signed integer overflow: -9223372036855 * 1000000 cannot be represented in type 'long'
    #0 0x7f94603d8eec in facebook::velox::Timestamp::fromMicros(long) fbcode/velox/type/Timestamp.h:242
    #1 0x7f94603d6868 in facebook::velox::functions::sparksql::SparkCastHooks::castIntToTimestamp(long) const fbcode/velox/functions/sparksql/specialforms/SparkCastHooks.cpp:39
    #2 0x7f947224d666 in void facebook::velox::exec::CastExpr::applyCastKernel<(facebook::velox::TypeKind)9, (facebook::velox::TypeKind)4, facebook::velox::util::SparkCastPolicy>(int, facebook::velox::exec::EvalCtx&, facebook::velox::SimpleVector<facebook::velox::TypeTraits<(facebook::velox::TypeKind)4>::NativeType> const*, facebook::velox::FlatVector<facebook::velox::TypeTraits<(facebook::velox::TypeKind)9>::NativeType>*) fbcode/velox/expression/CastExpr-inl.h:284

pedroerp avatar Oct 23 '24 15:10 pedroerp

@boneanxs looks like this is triggering a test failure (velox/functions/sparksql/tests:velox_functions_spark_test - SparkCastExprTest.intToTimestamp):

Test was added in target revision (54f19147c915f03eaf17f0531ce7ac00954fc585) hence is not present in base rev (2d4890ad47b2c140867328778d7102733cd1cb32)
Note: Google Test filter = SparkCastExprTest.intToTimestamp
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from SparkCastExprTest
[ RUN      ] SparkCastExprTest.intToTimestamp

buck-out/v2/gen/fbcode/b321bd6172db5152/velox/type/__velox_timestamp__/buck-headers/velox/type/Timestamp.h:242:35: runtime error: signed integer overflow: -9223372036855 * 1000000 cannot be represented in type 'long'
    #0 0x7f94603d8eec in facebook::velox::Timestamp::fromMicros(long) fbcode/velox/type/Timestamp.h:242
    #1 0x7f94603d6868 in facebook::velox::functions::sparksql::SparkCastHooks::castIntToTimestamp(long) const fbcode/velox/functions/sparksql/specialforms/SparkCastHooks.cpp:39
    #2 0x7f947224d666 in void facebook::velox::exec::CastExpr::applyCastKernel<(facebook::velox::TypeKind)9, (facebook::velox::TypeKind)4, facebook::velox::util::SparkCastPolicy>(int, facebook::velox::exec::EvalCtx&, facebook::velox::SimpleVector<facebook::velox::TypeTraits<(facebook::velox::TypeKind)4>::NativeType> const*, facebook::velox::FlatVector<facebook::velox::TypeTraits<(facebook::velox::TypeKind)9>::NativeType>*) fbcode/velox/expression/CastExpr-inl.h:284

fixed

boneanxs avatar Oct 24 '24 02:10 boneanxs

@pedroerp has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Oct 26 '24 02:10 facebook-github-bot

Thank you guys. I'll get it merged.

pedroerp avatar Oct 26 '24 02:10 pedroerp

@pedroerp merged this pull request in facebookincubator/velox@5180b68dc75dbc802f7e4f5538f4acd220fb6d1e.

facebook-github-bot avatar Oct 28 '24 16:10 facebook-github-bot

Conbench analyzed the 1 benchmark run on commit 5180b68d.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

conbench-facebook[bot] avatar Oct 28 '24 16:10 conbench-facebook[bot]