facebookincubator / velox

A C++ vectorized database acceleration library aimed to optimizing query engines and data processing systems.
https://velox-lib.io/
Apache License 2.0
3.2k stars 1.06k forks source link

Fix CAST(tinyint/smallint/integer/bigint as varbinary) for Spark #9819

Closed rui-mo closed 1 week ago

rui-mo commented 2 weeks ago

Fixes https://github.com/facebookincubator/velox/issues/9820.

netlify[bot] commented 2 weeks ago

Deploy Preview for meta-velox canceled.

Name Link
Latest commit 77eda917232a991c8a34d2b89c8c4d6b86f7f24e
Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/6650838991df410008b730b8
rui-mo commented 2 weeks ago

@PHILO-HE Would you like to take a review? Thanks.

jinchengchenghh commented 2 weeks ago

Do you need to update the document?

rui-mo commented 1 week ago

@jinchengchenghh Updated the documentation. Thanks.

rui-mo commented 1 week ago

@mbasmanova Could you help review this change? Thanks!

facebook-github-bot commented 1 week ago

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

mbasmanova commented 1 week ago

@rui-mo

I wonder if I can follow-up below TODO in a separate PR to provide custom cast signatures for Presto and Spark.

That would be great. Thanks.

facebook-github-bot commented 1 week ago

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

mbasmanova commented 1 week ago

@rui-mo I'm seeing "Conbench performance report — Found 2 regressions". Would you take a look?

CC: @assignUser @kgpai

facebook-github-bot commented 1 week ago

@mbasmanova merged this pull request in facebookincubator/velox@9446f67b14e9690ff3e5b0f85e46abb788e6560a.

conbench-facebook[bot] commented 1 week ago

Conbench analyzed the 1 benchmark run on commit 9446f67b.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

rui-mo commented 6 days ago

@mbasmanova Thanks for your review. It appears that no regression is mentioned in the most recent report. Please kindly contact me if I'm missing something.

mbasmanova commented 4 days ago

@rui-mo Perf testing can be flaky at times. This must be one of these cases. CC: @assignUser

assignUser commented 4 days ago

@mbasmanova @rui-mo Yeah looks like at the time this was committed the times where very close together so false positives can happen. But while checking this out I noticed that PR a day later clearly made the results worse: https://velox-conbench.voltrondata.run/compare/benchmark-results/0665116962307f828000737782ce0ccc...06651355ec2377e8800073bd529feb0a/