chore(ci): Advance velox by amitkdutta · Pull Request #27390 · prestodb/presto

amitkdutta · 2026-03-21T02:51:36Z

== NO RELEASE NOTE ==

Summary by Sourcery

Build:

Update Velox dependency reference in the native execution project to the latest desired commit.

sourcery-ai · 2026-03-21T02:51:44Z

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Updates the Velox submodule reference in presto-native-execution to a newer commit, without any direct source changes in this repository.

File-Level Changes

Change	Details	Files
Advance Velox git submodule pointer to a newer upstream commit.	Update the Velox submodule SHA referenced by presto-native-execution/velox to the latest desired upstream commit No local code, configuration, or build files in this repository are modified aside from the submodule pointer	`presto-native-execution/velox`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey - I've reviewed your changes and they look great!

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

amitkdutta · 2026-03-21T23:08:21Z

@czentgr Submitted a fix in Velox side facebookincubator/velox#16867
Will advance submodule once that is merge.

CC: @han-yan01

aditi-pandit · 2026-03-22T23:05:04Z

@amitkdutta : I saw some other errors in my updates as well

Job 67952471389 fails in the Linux GPU engine build when compiling:

presto-native-execution/presto_cpp/main/functions/remote/RestRemoteFunction.cpp

with:

error: invalid covariant return type for 'virtual std::unique_ptr<...RemoteFunctionResponse> ... invokeRemoteFunction(...) const'

Root cause

In RestRemoteFunction.cpp, the derived class overrides invokeRemoteFunction(...) but returns:

std::unique_ptr<velox::functions::remote::RemoteFunctionResponse>

However, the base class method (in Velox RemoteVectorFunction) is returning a different RemoteFunctionResponse type/namespace (or a different alias), so the override’s return type doesn’t match exactly. For std::unique_ptr<T>, covariance does not apply: the return type must be identical.

Problematic code (from RestRemoteFunction.cpp lines 36–39 at ref 8260627bcbcf9b89fdc5d736b63fd2569639c858):

std::unique_ptr<velox::functions::remote::RemoteFunctionResponse>
invokeRemoteFunction(
    const velox::functions::remote::RemoteFunctionRequest& request) const override {

Fix (code change)

Make the override match the base class signature exactly by using the same RemoteFunctionResponse type that the base declares.

A robust way is to use a type alias from the base (or decltype) so it can’t drift:

// inside class RestRemoteFunction
using ResponsePtr = decltype(std::declval<velox::functions::RemoteVectorFunction>()
                                 .invokeRemoteFunction(
                                     std::declval<const velox::functions::remote::RemoteFunctionRequest&>()));

ResponsePtr invokeRemoteFunction(
    const velox::functions::remote::RemoteFunctionRequest& request) const override {
  ...
}

If you prefer a simpler/cleaner change (and once you confirm the base signature), adjust the return type to exactly what the base uses. For example, if the base returns:

std::unique_ptr<velox::functions::RemoteFunctionResponse> (note: no ::remote), then change to:

std::unique_ptr<velox::functions::RemoteFunctionResponse>
invokeRemoteFunction(
    const velox::functions::remote::RemoteFunctionRequest& request) const override {
  ...
}

Why this is the right fix

This error is purely a C++ override-type mismatch. Once the overridden method’s return type exactly matches the base class declaration, the file will compile and the workflow can proceed past this step.

amitkdutta · 2026-03-23T00:53:17Z

facebookincubator/velox#16867

@aditi-pandit Yes the Remote function issue fix is already in this PR. We need CMAKE work done in facebookincubator/velox#16867 as well.

aditi-pandit · 2026-03-23T18:06:43Z

@amitkdutta : Yes, lets wait for Christian's Velox PR to be merged and update the Velox again. This is already few days ago now.

gggrace14 · 2026-03-24T16:33:07Z

Hi @amitkdutta , I see Christian's Velox PR merged. Would you like to update this PR?

amitkdutta · 2026-03-24T19:55:59Z

Another failure
https://github.com/prestodb/presto/actions/runs/23501753012/job/68412077946?pr=27390

Error:    TestNativeSidecarPlugin.testNonOverriddenInlinedSqlInvokedFunctionsWhenConfigDisabled:639->AbstractTestQueryFramework.assertQueryFails:341 Expected exception message 'line 1:8: Function 'native.default.array_split_into_chunks' has two matching signatures. Please specify parameter types. 
First match : 'native.default.array_split_into_chunks(array(varchar),integer):array(array(varchar))', Second match: 'native.default.array_split_into_chunks(array(varchar),integer):array(array(varchar))'' to match '.*Scalar function name not registered: native.default.array_split_into_chunks.*' for query: SELECT array_split_into_chunks(split(comment, ''), 2) from nation
Error:    TestNativeSidecarPlugin.testNonOverriddenInlinedSqlInvokedFunctionsWhenConfigEnabled:610->AbstractTestQueryFramework.assertQuery:174->AbstractTestQueryFramework.assertQuery:179 Execution of 'actual' query failed: SELECT array_split_into_chunks(split(comment, ''), 2) from nation

CC: @aditi-pandit

aditi-pandit · 2026-03-24T20:01:25Z

@czentgr @pramodsatya @pdabre12 Can you'll check the side-car tests ?

aditi-pandit

Thanks @czentgr

amitkdutta · 2026-03-25T05:58:12Z

There is still failures in

Error:  Failures: 
Error:    TestSqlInvokedFunctions>AbstractTestSqlInvokedFunctions.testArraySplitIntoChunks:43->AbstractTestQueryFramework.assertQuery:184 Execution of 'actual' query failed: select array_split_into_chunks(null, 2)

https://github.com/prestodb/presto/actions/runs/23515091536/job/68458477279?pr=27390
https://github.com/prestodb/presto/actions/runs/23515091536/job/68458477286?pr=27390

CC: @pdabre12 @czentgr @pramodsatya

amitkdutta · 2026-03-25T23:44:44Z

Current failure is

Error:  Failures: 
Error:    TestPrestoNativeAsyncDataCacheCleanupAPI.testAsyncDataCacheCleanup:128 SSD Cache should have non-zero read entries. did not expect [0] but found [0]
[INFO] 
Error:  Tests run: 1030, Failures: 1, Errors: 0, Skipped: 0

Not sure if there was any work done in cache side though.

amitkdutta · 2026-03-26T06:26:28Z

...src/test/java/com/facebook/presto/nativeworker/TestPrestoNativeAsyncDataCacheCleanupAPI.java

        createCustomer(queryRunner);
    }

+    @Disabled("See https://github.com/prestodb/presto/issues/27439")


We need @Test(groups = {"abc"}, enabled = false)

@amitkdutta : Can you create an issue for this test failure ? I can get someone take a deeper look.

#27439 For cache tear down issue

testMetadataWithNullableColumns

presto/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/iceberg/TestMetadata.java

Line 167 in f25c990

public void testMetadataWithNullableColumns()

Is also failing. CC: @aditi-pandit @czentgr

@czentgr : That disabled test is important to us. This Velox advance has a PR to update Iceberg stats that could be problematic. Created issue #27450

@aditi-pandit Thanks! Thanks for creating the issue. I was about to do this now. Indeed they are important to us. I wanted the CI to get going.

amitkdutta requested review from a team as code owners March 21, 2026 02:51

prestodb-ci added the from:Meta PR from Meta label Mar 21, 2026

sourcery-ai bot reviewed Mar 21, 2026

View reviewed changes

amitkdutta force-pushed the ewe2 branch 2 times, most recently from 48cf53a to 60c3c25 Compare March 21, 2026 06:51

amitkdutta force-pushed the ewe2 branch from 60c3c25 to 4883c60 Compare March 24, 2026 16:54

czentgr requested a review from pdabre12 as a code owner March 24, 2026 20:32

pdabre12 previously approved these changes Mar 24, 2026

View reviewed changes

czentgr dismissed pdabre12’s stale review via 01d3175 March 24, 2026 22:22

czentgr force-pushed the ewe2 branch from 14fcd2c to 01d3175 Compare March 24, 2026 22:22

gggrace14 previously approved these changes Mar 24, 2026

View reviewed changes

aditi-pandit previously approved these changes Mar 25, 2026

View reviewed changes

czentgr dismissed stale reviews from aditi-pandit and gggrace14 via eb8a228 March 25, 2026 14:22

czentgr force-pushed the ewe2 branch 2 times, most recently from eb8a228 to 6912016 Compare March 25, 2026 16:57

czentgr mentioned this pull request Mar 26, 2026

Fix testAsyncDataCacheCleanup #27439

Open

czentgr force-pushed the ewe2 branch from 6912016 to 1e2789c Compare March 26, 2026 02:48

amitkdutta commented Mar 26, 2026

View reviewed changes

czentgr force-pushed the ewe2 branch from 1e2789c to 776443d Compare March 26, 2026 13:23

amitkdutta and others added 2 commits March 26, 2026 19:01

chore(ci): Advance velox

f3f62f6

fix(test): Handle new built-in native.default.array_split_into_chunks

d4e5344

czentgr force-pushed the ewe2 branch from 776443d to d4e5344 Compare March 26, 2026 23:08

aditi-pandit mentioned this pull request Mar 26, 2026

Test failures in Iceberg: TestCreateTable and TestMetadata suite #27450

Open

gggrace14 self-requested a review March 27, 2026 02:52

gggrace14 approved these changes Mar 27, 2026

View reviewed changes

feilong-liu approved these changes Mar 27, 2026

View reviewed changes

amitkdutta merged commit 06f6613 into prestodb:master Mar 27, 2026
83 of 84 checks passed

This was referenced Mar 31, 2026

docs: Add release notes for 0.297 unix280/presto#51

Closed

docs: Add release notes for 0.297 unix280/presto#52

Open

nmahadevuni mentioned this pull request Apr 1, 2026

refactor: Revert iceberg data file statistics changes facebookincubator/velox#16999

Open

prestodb-ci mentioned this pull request Apr 1, 2026

docs: Add release notes for 0.297 #27484

Open

15 tasks

Conversation

amitkdutta commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

amitkdutta commented Mar 21, 2026

Uh oh!

aditi-pandit commented Mar 22, 2026

Root cause

Fix (code change)

Why this is the right fix

Uh oh!

amitkdutta commented Mar 23, 2026

Uh oh!

aditi-pandit commented Mar 23, 2026

Uh oh!

gggrace14 commented Mar 24, 2026

Uh oh!

amitkdutta commented Mar 24, 2026

Uh oh!

aditi-pandit commented Mar 24, 2026

Uh oh!

aditi-pandit left a comment

Choose a reason for hiding this comment

Uh oh!

amitkdutta commented Mar 25, 2026

Uh oh!

amitkdutta commented Mar 25, 2026

Uh oh!

amitkdutta Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

aditi-pandit Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

amitkdutta Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

aditi-pandit Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

czentgr Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

amitkdutta commented Mar 21, 2026 •

edited

Loading

sourcery-ai bot commented Mar 21, 2026 •

edited

Loading

czentgr Mar 27, 2026 •

edited

Loading