chore: Upgrade Velox to 59b492 pointed by upstream Presto release-0.297-edge10.#52
Closed
20001020ycx wants to merge 3 commits intoy-scope:presto-0.293-clp-connectorfrom
Closed
Conversation
|
Important Review skippedToo many files! This PR contains 300 files, which is 150 over the limit of 150. 📒 Files selected for processing (300)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
20001020ycx
changed the title
20001020ycx
changed the title
20001020ycx
changed the title
release-0.297-edge10.
20001020ycx
changed the title
release-0.297-edge10.release-0.297-edge10.
20001020ycx
force-pushed
the
2026-02-23-0.297-integration
branch
4 times, most recently
from
56a2c3a to
30580ef
Compare
|
I think this PR mostly looks good to me. Good to approve once the CI passes. It might also be worth updating the clp repo commit to the current latest (i.e. past the initial timestamp support commit), but not really necessary -- if not I'll just do it in my timestamp support PR. |
20001020ycx
force-pushed
the
2026-02-23-0.297-integration
branch
3 times, most recently
from
5536d06 to
c624802
Compare
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>
20001020ycx
force-pushed
the
2026-02-23-0.297-integration
branch
2 times, most recently
from
88a3943 to
59d886b
Compare
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
force-pushed
the
2026-02-23-0.297-integration
branch
3 times, most recently
from
9a089b7 to
e46f441
Compare
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>
20001020ycx
force-pushed
the
2026-02-23-0.297-integration
branch
from
e46f441 to
bd6e5d3
Compare
Author
|
close this as #53 is merged, my personal fork does not have enough permission for publishing velox build image |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR updates the Velox submodule to a newer upstream snapshot and adapts the CLP connector to remain compatible.
Commit 1 (a4383b5) 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 (5b10428) fix: CLP connector API adaptation and dependency upgrades to build against the new Velox version:
Commit 3 (00157a8) fix: Update builder dockerfile — setup-ubuntu.sh switched from pip to uv for cmake installation, so remove the Python venv setup and the manual cmake 3.28.3 pre-installation
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
breaking change.
Validation performed
SELECT CLP_GET_JSON_STRING() from clp.default.default limit 100