Add coordinator implementation of CLP connector #24868

wraymo · 2025-04-04T15:59:00Z

Description

This PR introduces a CLP connector. The background and proposed implementation details are outlined in the associated RFC.

This PR implements one part of phase 1 of the proposed implementation, namely the Java implementation for the coordinator. The worker implementation will leverage Velox as the default query engine, so once the Velox PR is merged, we will submit another PR to this repo to add the necessary changes to presto-native-execution.

Like other connectors, we have created a presto-clp module and implemented all required connector interfaces. The plan optimizer will be a future PR.

The important classes in the connector are described below.

Core Classes in Java

`ClpConfig`

The configuration class for CLP. Currently, we support the following properties:

clp.metadata-expire-interval: Defines the time interval after which metadata entries are considered expired and removed from the cache.
clp.metadata-refresh-interval: Specifies how frequently metadata should be refreshed from the source to ensure up-to-date information.
clp.polymorphic-type-enabled: Enables or disables support for polymorphic types within CLP. This determines whether dynamic type resolution is allowed.
clp.metadata-provider-type: Defines the type of the metadata provider. It could be a database, a file-based store, or another external system. By default, we use MySQL.
clp.metadata-db-url: The connection URL for the metadata database, used when clp.metadata-provider-type is configured to use a database.
clp.metadata-db-name: The name of the metadata database.
clp.metadata-db-user: The database user with access to the metadata database.
clp.metadata-db-password: The password for the metadata database user.
clp.metadata-table-prefix: A prefix applied to table names in the metadata database.
clp.split-provider-type: Defines the type of split provider for query execution. By default, we use MySQL, and the connection parameters are the same as those for the metadata database.

`ClpSchemaTree`

A helper class for constructing a nested schema representation from CLP’s column definitions. It supports hierarchical column names (e.g., a.b.c), handles name/type conflicts when the clp.polymorphic-type-enabled option is enabled, and maps serialized CLP types to Presto types. The schema tree produces a flat list of ClpColumnHandle instances, including RowType for nested structures, making it suitable for dynamic or semi-structured data formats.

When polymorphic types are enabled, conflicting fields are given unique names by appending a type-specific suffix to the column name. For instance, if an integer field named "a" and a Varstring (CLP type) field named "a" coexist in CLP’s schema tree, they are represented as a_bigint and a_varchar in Presto. This approach ensures that such fields remain queryable while adhering to Presto’s constraints.

`ClpMetadataProvider`

An interface responsible for retrieving metadata from a specified source.

public interface ClpMetadataProvider {
    List<ClpColumnHandle> listColumnHandles(SchemaTableName schemaTableName);  
    List<ClpTableHandle> listTableNames(String schema);  
}

We provide a default implementation called ClpMySqlMetadataProvider, which uses two MySQL tables. One of these is the datasets table, defined with the schema shown below. Currently, we support only a single Presto schema named default, and this metadata table stores all table names, paths, and storage types associated with that Presto schema.

Column Name	Data Type	Constraints
`name`	`VARCHAR(255)`	`PRIMARY KEY`
`archive_storage_type`	`VARCHAR(4096)`	`NOT NULL`
`archive_storage_directory`	`VARCHAR(4096)`	`NOT NULL`

The second MySQL table contains column metadata, defined by the schema shown below. Each Presto table is associated with a corresponding MySQL table that stores metadata about its columns.

Column Name	Data Type	Constraints
`name`	`VARCHAR(512)`	`NOT NULL`
`type`	`TINYINT`	`NOT NULL`
Primary Key	(`name`, `type`)

`ClpSplitProvider`

In CLP, an archive is the fundamental unit for searching, and we treat each archive as a Presto Split. This allows independent parallel searches across archives. The ClpSplitProvider interface, shown below, defines how to retrieve split information from a specified source:

public interface ClpSplitProvider {
    List<ClpSplit> listSplits(ClpTableLayoutHandle clpTableLayoutHandle);  
}

We provide a default implementation called ClpMySqlSplitProvider. It uses an archive table to store archive IDs associated with each table. The table below shows part of the schema (some irrelevant fields are omitted).

Column Name	Data Type	Constraints
`pagination_id`	`BIGINT`	`AUTO_INCREMENT PRIMARY KRY`
`id`	`VARCHAR(128)`	`NOT NULL`
...	...	...

By concatenating the table path (archive_storage_directory) and the archive ID (id), we can retrieve all split paths for a table.

`ClpMetadata`

This interface enables Presto to access various metadata. All requests are delegated to ClpMetadataProvider

For metadata management, it also maintains two caches and periodically refreshes the metadata.

columnHandleCache: A LoadingCache<SchemaTableName, List<ClpColumnHandle>> that maps a SchemaTableName to its corresponding list of ClpColumnHandle objects.
tableHandleCache: A LoadingCache<String, List<ClpTableHandle>> that maps a schema name (String) to a list of ClpTableHandle objects

Motivation and Context

See the associated RFC.

Impact

This module is independent from other modules and will not affect any existing functionality.

Test Plan

Unit tests are included in this PR, and we have also done end-to-end tests.

Contributor checklist

Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==
CLP Connector
* Add coordinator support for the CLP connector :pr:`24868`
* Add documentation for the CLP connector :doc:`../connector/clp` :pr:`24868`

steveburnett

Thanks for the doc, great work! Some nits, suggestions, and questions. Let me know what you think!

presto-docs/src/main/sphinx/connector/clp.rst

beinan · 2025-04-04T18:22:48Z

presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpSplitManager.java

+    @Inject
+    public ClpSplitManager(ClpSplitProvider clpSplitProvider)
+    {
+        this.clpSplitProvider = clpSplitProvider;


Suggested change

this.clpSplitProvider = clpSplitProvider;

this.clpSplitProvider = requireNonNull(clpSplitProvider, "clpSplitProvider is null");

Thanks, also fixed it in ClpPlanOptimizerProvider

beinan · 2025-04-04T18:23:30Z

presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpTableHandle.java

+    @JsonCreator
+    public ClpTableHandle(@JsonProperty("schemaTableName") SchemaTableName schemaTableName)
+    {
+        this.schemaTableName = schemaTableName;


requireNonNull?

Maybe we don't need to do check here, because it is called here and SchemaTableName is created here. Seems it 's unlikely to be a null

beinan · 2025-04-04T18:27:27Z

presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpPlanOptimizer.java

+            TableScanNode newTableScanNode = new TableScanNode(
+                    tableScanNode.getSourceLocation(),
+                    idAllocator.getNextId(),
+                    new TableHandle(


why do we need to create a new table handle here? can we just used the tableHandle? is it just for the kqlQuery?

Yeah, that's because we create a new ClpTableLayoutHandle with the KQL query. Similar to https://github.com/prestodb/presto/blob/master/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/optimization/JdbcComputePushdown.java#L128 and

presto/presto-druid/src/main/java/com/facebook/presto/druid/DruidPlanOptimizer.java

Line 162 in c875296

TableHandle newTableHandle = new TableHandle(

beinan · 2025-04-04T18:29:48Z

presto-clp/src/main/java/com/facebook/presto/plugin/clp/ClpSplit.java

+    @JsonCreator
+    public ClpSplit(@JsonProperty("schemaTableName") @Nullable SchemaTableName schemaTableName,
+                    @JsonProperty("archivePath") @Nullable String archivePath,
+                    @JsonProperty("query") Optional<String> query)


is this query just the kqlQuery? if so, I think it might be better to use the name kqlQuery instead for better consistency

Yes, will rename it

Also renamed it in ClpTableLayoutHandle.

steveburnett

LGTM! (docs)

Pull updated branch, review new local doc build, resolved comments from my previous review. Thanks for your responses! The refactor of the doc in this update looks great.

steveburnett · 2025-04-16T18:53:38Z

Thanks for the release note entry! You can link directly to the new documentation with this release note entry.

== RELEASE NOTES ==

CLP Connector Changes
* Add :doc:`../connector/clp`.

wraymo · 2025-04-17T15:08:09Z

Hi @steveburnett, thanks for the review!
When you have a moment, could you take a look at the updated release note to see if it looks good to you? I’ve also updated the docs in this commit: e7b3bc6

steveburnett · 2025-04-17T15:40:32Z

Hi @steveburnett, thanks for the review! When you have a moment, could you take a look at the updated release note to see if it looks good to you? I’ve also updated the docs in this commit: e7b3bc6

Thanks, I'll review the docs in a minute. I'll talk about the release note here:

We've automated the PR link in the release notes so you don't need to manually add the PR link in the release note entry now.
The link to the doc displays the title, so you don't need to write "CLP connector" which repeats it twice. The edit I have here for the second item compiles in a local doc build - I tested this in a local doc build to make sure - to display "Add documentation for the CLP Connector." with "CLP Connector" linking to the new page.
Should specify which connector is being affected in the Connector Changes heading.

== RELEASE NOTES ==

CLP Connector Changes
* Add coordinator support for the CLP connector.
* Add documentation for the :doc:`../connector/clp`.

Let me know if you have any questions!

steveburnett

LGTM! (docs)

Pull updated branch, new local doc build, looks good. Thanks!

steveburnett · 2025-04-24T18:43:18Z

@beinan, when you have time would you PTAL at @wraymo's responses to your review?

wraymo · 2025-05-23T15:40:42Z

For test other modules. There's a failure in presto-delta module and the error message was something like

2025-05-22T18:34:47.597-0600 WARNING Failed to load checkpoint metadata from file file:/home/runner/work/presto/presto/presto-delta/target/test-classes/delta_v3/data-reader-nested-struct/_delta_log/_last_checkpoint`

For test, there's a timeout error

TestDistributedQueuesSchedulingPolicy.testWeightedScheduling » ThreadTimeout Method com.facebook.presto.execution.resourceGroups.db.TestDistributedQueuesSchedulingPolicy.testWeightedScheduling() didn't finish within the time-out 60000

Both are not related to the changes in this PR.

steveburnett · 2025-06-06T14:54:53Z

@beinan, when you have time would you PTAL at @wraymo's responses to your review?

…ataProvider and ClpMySqlSplitProvider

…mizer and unit tests for a future PR

steveburnett · 2025-07-03T16:58:53Z

@beinan, when you have time would you PTAL at @wraymo's responses to your review?

tdcmeehan

A couple of high level questions:

I don't see the connector optimizer mentioned in the RFC: is that to be done in a later phase? What is functional in this phase?
Is there any way to add end to end tests? Most connectors run our abstract test suite, which uses canned queries against TPCH and DS, would that be possible here?

wraymo · 2025-08-29T15:05:16Z

@tdcmeehan Thanks for the question! Here are the details:

The connector optimizer is mentioned here. It will be added in a later phase. In this phase, the coordinator side is already usable with all required classes implemented, but we can’t run end-to-end tests with just this PR since the worker implementation is in Velox.
We do have an open PR in our fork to add end-to-end tests for very basic queries: https://github.com/y-scope/presto/pull/54. I think it's doable to run TPCH and DS, but we need to prepare our dataset (archive) first since we don't support insertion.

wraymo requested review from a team, elharo and steveburnett as code owners April 4, 2025 15:59

wraymo requested a review from presto-oss April 4, 2025 15:59

wraymo changed the title ~~Add CLP connector~~ Add coordinator implementation of CLP connector Apr 4, 2025

wraymo force-pushed the clp_integration branch 2 times, most recently from f240d86 to 0566292 Compare April 4, 2025 16:22

steveburnett requested changes Apr 4, 2025

View reviewed changes

beinan reviewed Apr 4, 2025

View reviewed changes

wraymo requested review from beinan and steveburnett April 5, 2025 01:42

steveburnett previously approved these changes Apr 7, 2025

View reviewed changes

wraymo mentioned this pull request Apr 16, 2025

feat: Add CLP connector y-scope/velox#3

Closed

wraymo dismissed steveburnett’s stale review via e7b3bc6 April 17, 2025 14:59

steveburnett previously approved these changes Apr 17, 2025

View reviewed changes

wraymo mentioned this pull request May 16, 2025

feat: Add CLP connector facebookincubator/velox#13364

Closed

wraymo dismissed steveburnett’s stale review via 86eccb1 May 23, 2025 00:04

wraymo force-pushed the clp_integration branch 2 times, most recently from 86eccb1 to d2fdfe9 Compare May 23, 2025 00:10

wraymo force-pushed the clp_integration branch from d2fdfe9 to 290e5b8 Compare May 26, 2025 01:42

wraymo added 4 commits June 16, 2025 11:16

Add CLP connector

ad3a2ff

apply review suggestions

09f96a4

Add a dummy ClpRecordSetProvider implementation and fix ClpMySqlMetad…

52d1e00

…ataProvider and ClpMySqlSplitProvider

Fixed incorrect configuration options

d18995f

wraymo added 2 commits June 16, 2025 11:16

Fixed several bugs and improved robustness; removed the CLP plan opti…

f6f09cc

…mizer and unit tests for a future PR

Fixed several issues and refactored the test files

ed55acc

wraymo force-pushed the clp_integration branch from 290e5b8 to ed55acc Compare June 16, 2025 17:49

Fixed several issues and formatted the code

daab58e

tdcmeehan reviewed Aug 28, 2025

View reviewed changes

tdcmeehan self-assigned this Aug 29, 2025

	this.clpSplitProvider = clpSplitProvider;
	this.clpSplitProvider = requireNonNull(clpSplitProvider, "clpSplitProvider is null");

Add coordinator implementation of CLP connector #24868

Are you sure you want to change the base?

Add coordinator implementation of CLP connector #24868

Uh oh!

Conversation

wraymo commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Core Classes in Java

ClpConfig

ClpSchemaTree

ClpMetadataProvider

ClpSplitProvider

ClpMetadata

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

Uh oh!

steveburnett left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wraymo Apr 5, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

steveburnett left a comment

Choose a reason for hiding this comment

Uh oh!

steveburnett commented Apr 16, 2025

Uh oh!

wraymo commented Apr 17, 2025

Uh oh!

steveburnett commented Apr 17, 2025

Uh oh!

steveburnett left a comment

Choose a reason for hiding this comment

Uh oh!

steveburnett commented Apr 24, 2025

Uh oh!

wraymo commented May 23, 2025

Uh oh!

steveburnett commented Jun 6, 2025

Uh oh!

steveburnett commented Jul 3, 2025

Uh oh!

tdcmeehan left a comment

Choose a reason for hiding this comment

Uh oh!

wraymo commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

wraymo commented Apr 4, 2025 •

edited

Loading

`ClpConfig`

`ClpSchemaTree`

`ClpMetadataProvider`

`ClpSplitProvider`

`ClpMetadata`

wraymo Apr 5, 2025 •

edited

Loading

wraymo commented Aug 29, 2025 •

edited

Loading