chore(ci): Advance velox #26584

amitkdutta · 2025-11-11T17:40:50Z

== NO RELEASE NOTE ==

sourcery-ai · 2025-11-11T17:40:56Z

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR advances the Velox submodule to the latest upstream commit and updates associated build, test, and CI configurations to align with API changes.

File-Level Changes

Change	Details	Files
Bump Velox submodule to new commit	Updated submodule pointer to new Velox hash Regenerated build scripts and artifacts	`presto-native-execution/velox`
Update CI workflows for Velox upgrade	Adjusted GitHub Actions to checkout updated submodule Revised Docker image tags for new Velox version	`.github/workflows/ci.yml` `ci/docker/*`
Adapt tests to Velox API changes	Modified expected outputs in integration tests Refactored test fixtures to match updated APIs	`presto-native-execution/src/test/java`

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 there - 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.}

czentgr · 2025-11-11T19:51:28Z

@amitkdutta Lets get a newer Velox version. Deepak's fix for CPR just came in and this helps Prestissimo as well.
Looks like some sidecar issues. Are you aware of anything?

Edit: I'm suspecting the worker is crashing on handling the request. Taking a look.

czentgr · 2025-11-11T22:42:11Z

@amitkdutta I found the root cause. A bunch of new iceberg transform functions were added that are internal but get exposed through the API. And they don't conform to the expected function naming convention for (regular) functions.
I can add a commit to ignore them for now. I don't think they are required externally for a user.

czentgr · 2025-11-11T23:00:44Z

presto-native-execution/presto_cpp/main/functions/FunctionMetadata.cpp

        name.find("$internal$") != std::string::npos ||
+        // TODO Review. Iceberg functions, if internal should use the $internal$
+        // prefix?
+        name.find("iceberg_") != std::string::npos ||


@PingLiuPing Newly added Iceberg functions don't conform to the expected name layout. I suspect they are internal for the transformations and are not exposed to the user?
In that case they probably should get the $internal$ prefix? We can clean up Velox later and open an issue if you agree.

@czentgr Thanks for looking. Yea we are experiencing same issue as well. Which PR exposed these functions? Can we revert it? Also we can add something so that the worker does not crash, just skip such functions.

I think its this one: facebookincubator/velox#15440

@czentgr Thanks.
Those functions should be exposed to users. Currently Presto only support bucket function but Spark(Gluten) support all of them. And those functions has been added to Velox document https://facebookincubator.github.io/velox/functions/iceberg/functions.html.

I will update the code to use three part names.

catalog.schema.function_name

Sorry for the trouble.

@amitkdutta @czentgr

Newly added Iceberg functions don't conform to the expected name layout

@czentgr Looks like we are lacking documentation for this requirement and PR time testing. Let's follow-up.

@PingLiuPing Yes, Iceberg functions in Velox but are they PrestoSQL functions as well? They can be used through the Velox APIs (PlanBuilder) but are also expected to work in PrestoSQL?

Looks like some iceberg functions are registered using correct prefix: #26581 in PrestoC++.

I think the issue is that Velox is registering the functions itself instead of letting PrestoC++ call the registration (like it does for all the other functions).

@czentgr Suppose those functions been registered by Prestissimo, do we have any mechanisms to get the function prefix inside velox?

I'm not sure if there has been a case where a Velox function was internal but also externalized to PrestoSQL.
We can look. But I would think we have two separate registrations then, once as internal (with the appropriate prefix which gets filtered in prestoc++) and once for PrestoSQL.

Summary: Original commit changeset: ae45744c1ae1 Original Phabricator Diff: D86686271 details: prestodb/presto#26584 (comment) Reviewed By: abhash09 Differential Revision: D86821554

…ms" (facebookincubator#15475) Summary: Original commit changeset: ae45744c1ae1 Original Phabricator Diff: D86686271 details: prestodb/presto#26584 (comment) Reviewed By: abhash09 Differential Revision: D86821554

…ms" (#15475) Summary: Pull Request resolved: #15475 Original commit changeset: ae45744c1ae1 Original Phabricator Diff: D86686271 details: prestodb/presto#26584 (comment) Reviewed By: kgpai, tanjialiang, abhash09 Differential Revision: D86821554 fbshipit-source-id: 79bf0de3deec19ba5d8cd720a3abef05dc29e94b

amitkdutta requested review from a team as code owners November 11, 2025 17:40

prestodb-ci added the from:Meta PR from Meta label Nov 11, 2025

sourcery-ai bot reviewed Nov 11, 2025

View reviewed changes

czentgr force-pushed the vel232330 branch from 096a068 to b9695d7 Compare November 11, 2025 22:57

czentgr reviewed Nov 11, 2025

View reviewed changes

amitkdutta mentioned this pull request Nov 12, 2025

fix: Revert Add support for evaluating Iceberg partition transforms" facebookincubator/velox#15475

Closed

PingLiuPing mentioned this pull request Nov 12, 2025

feat: Add support for evaluating Iceberg partition transforms facebookincubator/velox#15477

Closed

chore(ci): Advance velox

a34dc1b

amitkdutta force-pushed the vel232330 branch from b9695d7 to a34dc1b Compare November 12, 2025 14:28

majetideepak approved these changes Nov 12, 2025

View reviewed changes

czentgr merged commit 0e12d41 into prestodb:master Nov 12, 2025
112 of 113 checks passed

This was referenced Nov 14, 2025

refactor(native): Move function to extract function name parts to utilities #26605

Merged

docs: Update presto.default-namespace description #26625

Open

unidevel mentioned this pull request Nov 14, 2025

Add release notes for 0.296 unix280/presto#47

Merged

15 tasks

PingLiuPing mentioned this pull request Nov 21, 2025

misc(native): Remove the force restriction on function name prefix #26675

Open

7 tasks

chore(ci): Advance velox #26584

chore(ci): Advance velox #26584

Uh oh!

Conversation

amitkdutta commented Nov 11, 2025

Uh oh!

sourcery-ai bot commented Nov 11, 2025 • 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!

czentgr commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

czentgr commented Nov 11, 2025

Uh oh!

czentgr Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

amitkdutta Nov 12, 2025 • 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!

PingLiuPing Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

mbasmanova Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

czentgr Nov 12, 2025 • 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!

PingLiuPing Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

czentgr Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sourcery-ai bot commented Nov 11, 2025 •

edited

Loading

czentgr commented Nov 11, 2025 •

edited

Loading

amitkdutta Nov 12, 2025 •

edited

Loading

czentgr Nov 12, 2025 •

edited

Loading