Skip to content

Conversation

@amitkdutta
Copy link
Contributor

@amitkdutta amitkdutta commented Jan 30, 2026

== NO RELEASE NOTE ==

Summary by Sourcery

Chores:

  • Update the Velox submodule reference used by presto-native-execution to the latest desired commit.

@amitkdutta amitkdutta requested review from a team as code owners January 30, 2026 23:20
@prestodb-ci prestodb-ci added the from:Meta PR from Meta label Jan 30, 2026
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Jan 30, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Advance the Velox submodule pointer used by presto-native-execution to a newer upstream commit, with no local code or configuration changes in this repository.

File-Level Changes

Change Details Files
Update the Velox git submodule reference to a newer upstream commit.
  • Move the presto-native-execution/velox submodule to a newer commit SHA in the Velox repository
  • No modifications to tracked files under the submodule are made in this repository; all functional changes come from upstream Velox
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

@amitkdutta amitkdutta marked this pull request as draft January 30, 2026 23:21
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

aditi-pandit
aditi-pandit previously approved these changes Jan 30, 2026
Copy link
Contributor

@aditi-pandit aditi-pandit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @amitkdutta. Its been a while since we updated Velox. Lets try to advance it soon.

@amitkdutta amitkdutta marked this pull request as ready for review January 30, 2026 23:43
spershin
spershin previously approved these changes Jan 31, 2026
@aditi-pandit
Copy link
Contributor

@czentgr : Seems like there are problems still. The AI summry for the errors

The job failed due to missing dependencies and a missing generated source file:

1. Required command-line tools missing: The logs show errors that gh and jq are required for the action, but at least one is missing.
   - Solution: Ensure that both gh and jq are installed on the workflow runner. Add the following steps before your scripts:
     ```yaml
     - name: Install dependencies
       run: |
         sudo apt-get update
         sudo apt-get install -y gh jq
     ```

2. Missing generated file during build: The compiler fails because '/__w/presto/presto/presto-native-execution/_build/debug/velox/velox/functions/remote/if/gen-cpp2/RemoteFunction_types_binary.cpp' does not exist.
   - Solution: This file should be generated by the build system, usually via a code generation process (likely using Thrift or similar). Ensure the generation step is included and executed before compiling dependent code. Check your CMakeLists.txt in velox/velox/functions/remote/if to verify that all necessary generation steps are present. For example:
     ```cmake
     # Example addition to CMakeLists.txt if missing
     add_custom_command(
       OUTPUT gen-cpp2/RemoteFunction_types_binary.cpp
       COMMAND thrift --gen cpp2:templates --out gen-cpp2 ${CMAKE_CURRENT_SOURCE_DIR}/RemoteFunction.thrift
       DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/RemoteFunction.thrift
     )
     ```

In summary:
- Add steps to install gh and jq.
- Ensure Thrift or codegen steps produce all required files before compiling.

After applying these fixes, re-run the workflow to verify the issues are resolved. If more details about the codegen process or CMake configuration are needed, please provide the relevant configuration files.

PingLiuPing
PingLiuPing previously approved these changes Jan 31, 2026
Copy link
Contributor

@PingLiuPing PingLiuPing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @amitkdutta and @czentgr.
The Iceberg changes look good. Though they aren’t my latest change and shouldn’t cause any issues. I can submit a small follow-up PR to clean up the unused code.

@czentgr
Copy link
Contributor

czentgr commented Feb 1, 2026

@PingLiuPing Thanks. I had to make one change to deal with Clang-15 though. It doesn't generate the constructors for the struct automatically. And there are no explicit constructors that handle the members. So I'm using a different initialization.

@czentgr czentgr force-pushed the vew23 branch 2 times, most recently from d1a657c to b2acb25 Compare February 2, 2026 04:24
@PingLiuPing
Copy link
Contributor

@PingLiuPing Thanks. I had to make one change to deal with Clang-15 though. It doesn't generate the constructors for the struct automatically. And there are no explicit constructors that handle the members. So I'm using a different initialization.

Thanks.

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.

6 participants