chore(ci): Advance velox

[amitkdutta]

If release note is NOT required, use:

== NO RELEASE NOTE ==

Summary by Sourcery

Chores:

• Advance the Velox submodule reference in presto-native-execution.

[sourcery-ai]

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 the Velox git submodule to a newer upstream commit.	Update the Velox submodule pointer to reference a newer commit hash from the upstream repository. Pull in upstream Velox changes implicitly (behavioral or API differences, if any, come from the new Velox version rather than changes in this repo). No local code, configuration, or build scripts in this repository are modified aside from the submodule reference update.	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 ✨

• X
• Mastodon
• LinkedIn
• Facebook

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

[aditi-pandit]

@amitkdutta : These errors related to Iceberg and Arrow/Parquet seem quite legit.

ref facebookincubator/velox#16062

@PingLiuPing @czentgr

From co-pilot

The failing job 67284023403 encountered linker errors due to missing or undefined references in the code:

The key error is:

undefined reference to `facebook::velox::parquet::arrow::FileMetaData::getNaNCount(int) const'
undefined reference to `facebook::velox::parquet::arrow::ColumnChunkMetaData::~ColumnChunkMetaData()'
undefined reference to `facebook::velox::parquet::arrow::RowGroupMetaData::~RowGroupMetaData()'

This suggests that code in IcebergParquetStatsCollector.cpp (inside velox_hive_iceberg_splitreader.a) depends on the implementation for these functions, but they are either missing or not linked correctly from the Velox library.

Solution Steps

1. Check that the velox parquet arrow module provides these functions:

   • Ensure that getNaNCount(int) const is implemented in FileMetaData.
   • Ensure that destructors for ColumnChunkMetaData and RowGroupMetaData are implemented and properly exported.

2. Check CMakeLists.txt linkage:

   • Make sure that the library containing these implementations (usually velox_dwio_arrow_parquet_writer_lib or equivalent) is properly linked to your target (velox_hive_iceberg_splitreader.a).

3. Add missing implementations if necessary:

   • If the functions/destructors do not exist, implement them. For example, in FileMetaData:

     // In FileMetaData.h
     int getNaNCount(int columnIdx) const;
     
     // In FileMetaData.cpp
     int FileMetaData::getNaNCount(int columnIdx) const {
         // Provide your logic, or a stub if not used
         return 0;
     }

4. Export the symbols:

   • If your project uses static/dynamic libraries, ensure the classes and functions are exported as needed for linkage.

5. Rebuild Velox dependencies:

   • Make sure your build pulls in the latest changes from Velox or matches the version expected by PrestoDB.

Code Fix Example

If getNaNCount(int) const is missing, add a stub implementation:

// velox/velox/parquet/arrow/FileMetaData.h
class FileMetaData {
  public:
    int getNaNCount(int columnIdx) const;
};

// velox/velox/parquet/arrow/FileMetaData.cpp
int FileMetaData::getNaNCount(int columnIdx) const {
    // TODO: Proper implementation
    return 0;
}

Similarly, check destructors for ColumnChunkMetaData and RowGroupMetaData and ensure they are correctly declared and defined.

Summary

• Implement missing methods or destructors in the Velox Parquet Arrow library.
• Ensure libraries are linked correctly in your build scripts.
• Rebuild the project and rerun CI.

Once these changes are made, your job should succeed and the linker errors should disappear.

@czentgr

[czentgr]

@aditi-pandit @amitkdutta I've made a PR to fix the issue: facebookincubator/velox#16867

The problem was that the IcebergParquetStatsCollector requires also the parquet writer that defines the missing symbols. I suspect that in Velox we don't see this problem because of the shared build. The symbols show up eventually. But in PrestoC++ when building presto test executables they don't. So we need the explicit dependency declared to make sure we see all relevant libraries. I did also a minor cleanup of headers where I saw duplication and such. This build now successfully locally.