Skip to content

Conversation

@wraymo
Copy link

@wraymo wraymo commented Apr 3, 2025

This PR adds an RFC for the CLP connector, which enables SQL queries on the compressed CLP archives.

Copy link
Contributor

@tdcmeehan tdcmeehan left a comment

Choose a reason for hiding this comment

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

This all seems pretty reasonable to me. There doesn't seem to be anything that needs to change in the SPI, runtime or execution engines besides things that are at the connector level.

One suggestion is to keep the CLP C++ connector in Presto for now. We can always move it to Velox later, and it will make the landing of this code more predictable.


## Table & Column Resolution

Implementing `ConnectorMetadata` requires determining what tables and columns exist in a CLP dataset, which in turn requires querying CLP's metadata database. Since different users may have their own implementation of a CLP metadata database, the connector should expose the required methods through a `ClpMetadataProvider` interface. For table and column resolution, the interface should contain the following methods:
Copy link
Contributor

Choose a reason for hiding this comment

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

To be clear, ClpMetadataProvider lives in the CLP connector, not the runtime?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that's right

@wraymo
Copy link
Author

wraymo commented Aug 29, 2025

This all seems pretty reasonable to me. There doesn't seem to be anything that needs to change in the SPI, runtime or execution engines besides things that are at the connector level.

One suggestion is to keep the CLP C++ connector in Presto for now. We can always move it to Velox later, and it will make the landing of this code more predictable.

Thanks for the suggestion! We'll spend some time working on it.

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.

4 participants