chore: Integrate upstream Presto release-0.297-edge10 and adapt CLP connector. by 20001020ycx · Pull Request #147 · y-scope/presto

20001020ycx · 2026-02-23T21:22:01Z

Description

This PR updates Presto from 0.293 to upstream release-0.297-edge10 and adapts the CLP connector to remain compatible. release-0.297-edge10 is chosen because it provides 1) compatible fmt version with latest log surgeon and clp library which are the dependencies of the velox, and 2) it contains feature commits where we need for plugin our connectors, namely prestodb#24330 and prestodb#26257.

Commit 1 — pulls all changes from upstream prestodb/presto branch release-0.297-edge10 at commit d1aa0c30da8d53d4305315aeed0b8b0e6c99e5d5, with conflict resolution to retain our CLP-related files.
Commit 2 — adapts the CLP connector to the Presto 0.297 interface:
- Extract ClpPrestoToVeloxConnector into its own dedicated file following the 0.297 pattern where the monolithic PrestoToVeloxConnector.cpp was split per-connector; the implementation is unchanged from its previous location in that file.
- Update presto-clp/pom.xml version and fix airlift dependency to match 0.297.
- Update MetadataManager constructor call to include the new MaterializedViewPropertyManager parameter added in 0.297.
Commit 3 — updates CI workflows and build configuration for 0.297 compatibility:
- Drop Java 8 from all workflow matrices because we now target JDK 17.
- presto-common/pom.xml: Remove --add-exports=java.base/sun.misc=ALL-UNNAMED because on JDK 17 it causes a fatal split-package error (sun.misc is exported by both java.base and jdk.unsupported). Later upstream patch with the same fix.
- Add make velox-submodule step to e2e test jobs because these jobs run on a fresh checkout and need the velox submodule initialised to locate native library paths.
- Restore format-check/header-check/format-fix/header-fix Makefile targets because upstream 0.297 removed them.
- Update 0.297 refs across workflow files to release-0.297-edge-10-clp-connector, as it will be our new main branch.
- Limit e2e tests to TestPrestoNativeClpGeneralQueries because the new upstream CTE tests introduced in 0.297-edge10 require plugins not present in our test environment.
- clp-connector-unit-tests job: The existing maven-checks job uses -DskipTests so CLP unit tests were never run in CI. This ensures CI runs CLP unit test.
- Remove non-CLP test jobs

Note for reviewers: Please only review 2nd and 3rd commit. The 2nd commit is relevant to the CLP Presto 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 Presto 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, Presto with the changes in this PR, Velox with integration of upstream Presto's 0.297, and minio as the storage.
- The result is returned as expected.

coderabbitai · 2026-02-23T21:22:14Z

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

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

Pull all changes from upstream prestodb/presto branch release-0.297-edge10 at commit d1aa0c3. Velox submodule pointer is retained from our fork (already upgraded separately on branch 2026-02-23-0.297-integration).

gibber9809

Mostly looks good, just have some questions about parts I didn't understand.

Besides those questions, I'm also curious about why the PrestoToVeloxConnector class is needed now.

.github/workflows/maven-checks.yml

...to-clp/src/main/java/com/facebook/presto/plugin/clp/split/filter/ClpSplitFilterProvider.java

presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.java

20001020ycx · 2026-02-27T16:13:20Z

Besides those questions, I'm also curious about why the PrestoToVeloxConnector class is needed now.

In 0.293, all connectors (Hive, Iceberg, Tpch, and CLP) had their subclass implementations in the same monolithic PrestoToVeloxConnector.cpp. PrestoToVeloxConnector was a single concrete class with one big function containing if/else logic for each connector.

In 0.297, upstream refactored it into an abstract base class with pure virtual methods (= 0). There's no longer a single function to add a branch to — each connector must implement its own subclass. That's why ClpPrestoToVeloxConnector is required now.

gibber9809

Approving the changes in the second commit for adapting our code to 0.297.

- Extract ClpPrestoToVeloxConnector into its own .h/.cpp files to match the per-connector split introduced in 0.297 - Update CMakeLists.txt and Registration.cpp to include the new file - Update presto-clp pom.xml: bump version to 0.297, update groupId from com.facebook.presto to com.facebook.presto.clp - Fix ClpSplitFilterProvider to use MetadataManager's new signature (rowExpression param added in 0.297) - Fix ClpMetadataDbSetUp to use Paths.get() instead of new File() (required by modernizer-maven-plugin built-in rules) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Drop Java 8 from all workflow matrices (0.297 requires Java 11+); update maven-checks, tests, docs, prestocpp-linux-build-and-unit-test - Update artifact paths and version strings from 0.293 to 0.297-edge10.1-SNAPSHOT in maven-checks - Update all branch references from release-0.293-clp-connector to presto-0.293-clp-connector across all workflows - Restore docker/Dockerfile to 0.293 structure (upstream added presto-function-server which is not built by our CI); update Java 11 -> 17 to match build requirement - Update .gitmodules for velox submodule - Update velox submodule pointer to 0.297-compatible commit - Remove --add-exports=java.base/sun.misc=ALL-UNNAMED from presto-common (causes split-package conflict on JDK 17 Corretto; sun.misc is accessible via jdk.unsupported without explicit export flags) - Disable testMultipleIndependentPersistentCtes and testSimplePersistentCteMultipleUses: both are newly introduced in 0.297-edge10 and fail due to missing native support (key_sampling_percent not implemented in Velox; $operator$hash_code not registered for BIGINT) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…onnector. (#147)

…onnector. (#147) (#15)

20001020ycx force-pushed the 2026-02-23-0.297-presto-integration branch 3 times, most recently from f80bd10 to 201ee5c Compare February 24, 2026 21:24

20001020ycx requested a review from gibber9809 February 24, 2026 21:31

20001020ycx force-pushed the 2026-02-23-0.297-presto-integration branch from 201ee5c to f4ba649 Compare February 25, 2026 19:13

chore: integrate upstream Presto release-0.297-edge10

f3b5eea

Pull all changes from upstream prestodb/presto branch release-0.297-edge10 at commit d1aa0c3. Velox submodule pointer is retained from our fork (already upgraded separately on branch 2026-02-23-0.297-integration).

20001020ycx force-pushed the 2026-02-23-0.297-presto-integration branch 3 times, most recently from 43f962b to 9a9bbb6 Compare February 25, 2026 20:37

gibber9809 reviewed Feb 25, 2026

View reviewed changes

20001020ycx force-pushed the 2026-02-23-0.297-presto-integration branch from 9a9bbb6 to 70d4b33 Compare February 25, 2026 21:02

20001020ycx requested a review from gibber9809 February 27, 2026 16:22

20001020ycx force-pushed the 2026-02-23-0.297-presto-integration branch from 70d4b33 to d53942c Compare February 27, 2026 16:28

gibber9809 previously approved these changes Feb 27, 2026

View reviewed changes

20001020ycx dismissed gibber9809’s stale review via b1d114e February 27, 2026 17:12

20001020ycx force-pushed the 2026-02-23-0.297-presto-integration branch 13 times, most recently from 53cf846 to b7832df Compare March 1, 2026 05:22

20001020ycx force-pushed the 2026-02-23-0.297-presto-integration branch 5 times, most recently from 33096ca to 9d068e4 Compare March 1, 2026 17:27

20001020ycx changed the base branch from release-0.293-clp-connector to release-0.297-edge-10-clp-connector March 2, 2026 14:27

20001020ycx force-pushed the 2026-02-23-0.297-presto-integration branch 6 times, most recently from cae7d4b to 37c536e Compare March 2, 2026 15:57

20001020ycx requested a review from jackluo923 March 2, 2026 18:27

20001020ycx force-pushed the 2026-02-23-0.297-presto-integration branch 2 times, most recently from 2835eda to 2490fc9 Compare March 2, 2026 19:35

20001020ycx and others added 2 commits March 2, 2026 14:48

20001020ycx force-pushed the 2026-02-23-0.297-presto-integration branch from 2490fc9 to a8c2341 Compare March 2, 2026 19:48

20001020ycx mentioned this pull request Mar 2, 2026

feat: Add support for searching and marshalling the new Timestamp column type. y-scope/velox#54

Merged

3 tasks

jackluo923 approved these changes Mar 2, 2026

View reviewed changes

20001020ycx merged commit 8ab0c60 into y-scope:release-0.297-edge-10-clp-connector Mar 2, 2026
8 checks passed

This was referenced Mar 2, 2026

Presto Connector cannot handle new Timestamp column type from PR #1788 y-scope/clp#2008

Closed

feat: Add timestamp() KQL filter pushdown; sync DeprecatedDateString rename with clp-s upstream. #148

Merged

20001020ycx deleted the 2026-02-23-0.297-presto-integration branch March 3, 2026 15:01

20001020ycx added a commit that referenced this pull request Mar 4, 2026

chore: Integrate upstream Presto release-0.297-edge10 and adapt CLP c…

7c2973a

…onnector. (#147)

20001020ycx added a commit that referenced this pull request Mar 4, 2026

chore: Integrate upstream Presto release-0.297-edge10 and adapt CLP c…

c772cb8

…onnector. (#147) (#15)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Integrate upstream Presto release-0.297-edge10 and adapt CLP connector.#147

chore: Integrate upstream Presto release-0.297-edge10 and adapt CLP connector.#147
20001020ycx merged 3 commits intoy-scope:release-0.297-edge-10-clp-connectorfrom
20001020ycx:2026-02-23-0.297-presto-integration

20001020ycx commented Feb 23, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Feb 23, 2026 •

edited

Loading

Review skipped

Uh oh!

gibber9809 left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

20001020ycx commented Feb 27, 2026

Uh oh!

gibber9809 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

20001020ycx commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Validation performed

Uh oh!

coderabbitai bot commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

gibber9809 left a comment • 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!

Uh oh!

Uh oh!

Uh oh!

20001020ycx commented Feb 27, 2026

Uh oh!

gibber9809 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

20001020ycx commented Feb 23, 2026 •

edited

Loading

coderabbitai bot commented Feb 23, 2026 •

edited

Loading

gibber9809 left a comment •

edited

Loading