Skip to content

chore: Upgrade Velox to 59b492 pointed by upstream Presto release-0.297-edge10.#53

Merged
20001020ycx merged 4 commits intopresto-0.297-edge-10-clp-connectorfrom
2026-02-23-0.297-integration
Feb 27, 2026
Merged

chore: Upgrade Velox to 59b492 pointed by upstream Presto release-0.297-edge10.#53
20001020ycx merged 4 commits intopresto-0.297-edge-10-clp-connectorfrom
2026-02-23-0.297-integration

Conversation

@20001020ycx
Copy link

@20001020ycx 20001020ycx commented Feb 25, 2026

Description

This PR updates the Velox submodule to a newer upstream snapshot and adapts the CLP connector to remain compatible.

Commit 1 (1f22e5c) chore: Pull from upstream Velox open-source repo at commit facebookincubator@59b492a, this is the commit pointed by Presto release-0.297-edge10.

Commit 2 (7a75748) fix: CLP connector API adaptation and dependency upgrades to build against the new Velox version:

  • API changes: Updated ClpConnector, ClpDataSource, ClpColumnHandle, and ClpTableHandle to conform to breaking Velox connector API changes:
    • ColumnHandle and ConnectorTableHandle now declare name() as pure virtual — implemented in ClpColumnHandle and ClpTableHandle.
    • createDataSource / ClpDataSource signatures updated to use Velox-defined type aliases ConnectorTableHandlePtr, ColumnHandleMap, and ConnectorInsertTableHandlePtr (all const-qualified in 0.297).
    • Global connector factory registry removed — replaced with direct ClpConnectorFactory instantiation.
    • DataSource::runtimeStats() / RuntimeCounter renamed to getRuntimeStats() / RuntimeMetric; supportsSplitPreload() is now const; connectorConfig() is no longer virtual.
  • clp upgraded to commit f82e6114
  • log-surgeon upgraded to commit 193e1f91
  • spdlog upgraded to v1.15.3

Commit 3 (8c8ce56) — fix: update builder dockerfile for Velox 0.297 setup-ubuntu.sh changes

  • Updates the builder image and fixes several missing CMake link dependencies and compile definitions. The many CMake changes are needed because upstream Velox uses VELOX_MONO_LIBRARY=ON by default, which compiles everything into one monolithic velox library — in that mode, missing target_link_libraries or target_compile_definitions entries are harmless since all symbols are in one binary. We use VELOX_MONO_LIBRARY=OFF (required by our CLP libraries), which builds each target as a separate library and exposes all missing dependencies as linker errors or silent #ifdef misses at runtime.

Commit 4 (7e3d007) — chore: apply clang-format and gersemi (CMake formatter) to CLP connector files

  • The 0.297 upstream merge upgraded clang-format from v18 to v21 and added gersemi (CMake formatter) to the pre-commit config

Note for reviewers: Please only review 2nd and 3rd commit. The 2nd commit is relevant to the CLP Velox connector and warrants review, and the 3rd commit is fixing the breaking changes from ci workflow. The 1st commit is a merge commit from the upstream Velox open-source repository.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

  • All unit test passed
  • End to end test
    • Query: SELECT CLP_GET_JSON_STRING() from clp.default.default limit 100
    • Set up: official clp package v0.9.0, OSS version of Presto with the integration of upstream Presto's 0.297, velox with changes in this PR (integration of upstream Presto's 0.297), and minio as the storage.
    • The result is returned as expected.

@coderabbitai
Copy link

coderabbitai bot commented Feb 25, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 2026-02-23-0.297-integration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@20001020ycx 20001020ycx force-pushed the 2026-02-23-0.297-integration branch 5 times, most recently from 42408ea to 89443c5 Compare February 26, 2026 16:37
@20001020ycx 20001020ycx changed the base branch from presto-0.293-clp-connector to presto-0.297-edge-10-clp-connector February 26, 2026 16:48
@20001020ycx 20001020ycx force-pushed the 2026-02-23-0.297-integration branch from 89443c5 to 3af0f0e Compare February 26, 2026 16:57
20001020ycx and others added 2 commits February 26, 2026 12:08
Squash merge of upstream Velox commit 59b492a.

Conflict resolutions:
- CMake/resolve_dependency_modules/log_surgeon.cmake: Keep fork's CLP
  content; git confused it with tpcds/gen/tests/CMakeLists.txt due to
  rename detection during squash merge.
- CMakeLists.txt: Take upstream Clang warning formatting (drop
  -Wno-implicit-const-int-float-conversion); preserve fork's Boost
  libraries (url, iostreams) needed for CLP with upstream formatting;
  take upstream zstd target alias check block.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adapt CLP connector to upstream Velox API changes in 0.297:

- ConnectorFactory global registry removed: Replace
  connector::registerConnectorFactory / getConnectorFactory /
  unregisterConnectorFactory calls with direct factory instantiation
  (ClpConnectorFactory factory; factory.newConnector(...))

- New type aliases for connector handles: Update createDataSource and
  ClpDataSource constructor signatures to use ConnectorTableHandlePtr
  (shared_ptr<const ConnectorTableHandle>) and ColumnHandleMap
  (unordered_map<string, shared_ptr<const ColumnHandle>>) instead of
  the non-const shared_ptr variants

- Update dynamic_pointer_cast calls to cast to const types to match
  the new const-qualified pointer types

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@20001020ycx 20001020ycx force-pushed the 2026-02-23-0.297-integration branch from 3af0f0e to 7e3d007 Compare February 26, 2026 17:10
20001020ycx and others added 2 commits February 26, 2026 14:08
setup-ubuntu.sh no longer creates a Python venv — it switched from pip
to uv for cmake installation. Remove the mv /tmp/.venv step and the
VIRTUAL_ENV/PATH activation that depended on it. Also remove the cmake
pre-install step, which was a workaround for pip-installed cmake PATH
issues and is no longer needed since uv installs cmake to /usr/local/bin.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Apply clang-format v21 and gersemi v0.21 formatting (introduced by the
0.297 upstream merge) to CLP connector C++ and CMake files.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@20001020ycx 20001020ycx force-pushed the 2026-02-23-0.297-integration branch from 7e3d007 to b7eb104 Compare February 26, 2026 19:08
Copy link

@gibber9809 gibber9809 left a comment

Choose a reason for hiding this comment

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

LGTM. For PR title how about:

chore: Upgrade velox to facebookincubator/velox@59b492 to match version used by Presto release-0.297-edge10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants