Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[native] Advance velox. #24267

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[native] Advance velox. #24267

wants to merge 1 commit into from

Conversation

amitkdutta
Copy link
Contributor

== NO RELEASE NOTE ==

@amitkdutta amitkdutta requested a review from a team as a code owner December 17, 2024 04:15
@prestodb-ci prestodb-ci added the from:Meta PR from Meta label Dec 17, 2024
gggrace14
gggrace14 previously approved these changes Dec 17, 2024
@amitkdutta
Copy link
Contributor Author

amitkdutta commented Dec 18, 2024

https://app.circleci.com/pipelines/github/prestodb/presto/23654/workflows/d4277539-8e6d-4725-8be3-3a709e0e84a4/jobs/98812
Build is failing with

[806/1098] Linking CXX executable velox/velox/functions/remote/benchmarks/velox_benchmark_local_remote_comparison
FAILED: velox/velox/functions/remote/benchmarks/velox_benchmark_local_remote_comparison 
....
/opt/rh/gcc-toolset-12/root/usr/libexec/gcc/x86_64-redhat-linux/12/ld: cannot find -lvelox_benchmark_builder: No such file or directory
collect2: error: ld returned 1 exit status

CC: @majetideepak @czentgr @kgpai

@czentgr
Copy link
Contributor

czentgr commented Dec 18, 2024

@amitkdutta Yes. Working on a Velox change that fixes it. We identified the problem.

@czentgr
Copy link
Contributor

czentgr commented Dec 18, 2024

@amitkdutta PR to fix issue in Velox: facebookincubator/velox#11905

@amitkdutta
Copy link
Contributor Author

Unit tests failing

presto_cpp/main/operators/tests/BroadcastTest.cpp
/root/project/presto-native-execution/presto_cpp/main/operators/tests/BroadcastTest.cpp: In member function 'facebook::velox::RowVectorPtr facebook::presto::operators::test::BroadcastTest::readFromFile(const std::string&, const facebook::velox::RowTypePtr&)':
/root/project/presto-native-execution/presto_cpp/main/operators/tests/BroadcastTest.cpp:214:28: error: no matching function for call to 'facebook::velox::VectorStreamGroup::read(std::unique_ptr<facebook::velox::BufferInputStream, std::default_delete<facebook::velox::BufferInputStream> >::pointer, facebook::velox::memory::MemoryPool*, const facebook::velox::RowTypePtr&, facebook::velox::VectorSerde*, facebook::velox::RowVectorPtr*)'
  214 |     VectorStreamGroup::read(
      |     ~~~~~~~~~~~~~~~~~~~~~~~^
  215 |         byteStream.get(),
      |         ~~~~~~~~~~~~~~~~~   
  216 |         pool(),
      |         ~~~~~~~             
  217 |         dataType,
      |         ~~~~~~~~~           
  218 |         velox::getNamedVectorSerde(velox::VectorSerde::Kind::kPresto),
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  219 |         &result);
      |         ~~~~~~~~            
In file included from /root/project/presto-native-execution/./presto_cpp/main/operators/BroadcastFactory.h:19,
                 from /root/project/presto-native-execution/./presto_cpp/main/operators/BroadcastExchangeSource.h:16,
                 from /root/project/presto-native-execution/presto_cpp/main/operators/tests/BroadcastTest.cpp:16:
/root/project/presto-native-execution/velox/velox/vector/VectorStream.h:416:15: note: candidate: 'static void facebook::velox::VectorStreamGroup::read(facebook::velox::ByteInputStream*, facebook::velox::memory::MemoryPool*, facebook::velox::RowTypePtr, facebook::velox::VectorSerde*, facebook::velox::RowVectorPtr*, const facebook::velox::VectorSerde::Options*)'
  416 |   static void read(
      |               ^~~~
/root/project/presto-native-execution/velox/velox/vector/VectorStream.h:416:15: note:   candidate expects 6 arguments, 5 provided
At global scope:

CC: @xiaoxmeng

@amitkdutta
Copy link
Contributor Author

Being fixed here
#24291

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
from:Meta PR from Meta
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants