cube-js / cube

📊 Cube — Universal semantic layer platform for AI, BI, spreadsheets, and embedded analytics
https://cube.dev
Other
17.96k stars 1.78k forks source link

Use the Trino client instead of inheriting from Presto #8948

Open ndrluis opened 1 week ago

ndrluis commented 1 week ago

Hello, I'm opening this PR, but I don't have expertise in TypeScript and JavaScript. I've tried my best to solve the problem, but I need some help to make the test suite work.

This PR aims to decouple the Trino driver from Presto, which uses an outdated client that is quite slow. I ran some tests comparing only the clients, and the Presto driver currently used by Cube (which is not the official one) takes about 3 seconds to execute a SELECT 1 query, while the Trino driver completes it in 300ms.

Additionally, there are features in the Presto driver that are not supported by Trino, such as unloading data through GCS. Therefore, we need to isolate each of these drivers.

Check List

Issue Reference this PR resolves

[For example #12]

Description of Changes Made (if issue reference is not provided)

[Description goes here]

vercel[bot] commented 1 week ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

8 Skipped Deployments | Name | Status | Preview | Comments | Updated (UTC) | | :--- | :----- | :------ | :------- | :------ | | **examples-angular-dashboard** | ⬜️ Ignored ([Inspect](https://vercel.com/cube-dev/examples-angular-dashboard/Edz5LW8xQJCGaWoXCb5Gr25B2PVJ)) | [Visit Preview](https://examples-angular-dashboard-git-fork-ndrluis-tri-740080-cube-dev.vercel.app) | | Nov 13, 2024 5:17pm | | **examples-react-d3** | ⬜️ Ignored ([Inspect](https://vercel.com/cube-dev/examples-react-d3/37zqEBTtFdKrvuEV2TZMB8Xtf5Pi)) | [Visit Preview](https://examples-react-d3-git-fork-ndrluis-trino-driver-cube-dev.vercel.app) | | Nov 13, 2024 5:17pm | | **examples-react-dashboard** | ⬜️ Ignored ([Inspect](https://vercel.com/cube-dev/examples-react-dashboard/BtLVNLYhd9g3otAKJi3Xuqg12wNT)) | [Visit Preview](https://examples-react-dashboard-git-fork-ndrluis-trino-driver-cube-dev.vercel.app) | | Nov 13, 2024 5:17pm | | **examples-react-data-table** | ⬜️ Ignored ([Inspect](https://vercel.com/cube-dev/examples-react-data-table/5sFSjxbbiMio1Q793mp7QChnEbc1)) | [Visit Preview](https://examples-react-data-table-git-fork-ndrluis-trin-62ad7f-cube-dev.vercel.app) | | Nov 13, 2024 5:17pm | | **examples-react-highcharts** | ⬜️ Ignored ([Inspect](https://vercel.com/cube-dev/examples-react-highcharts/F8qZQ6Mtaw4XymTFDL7PMzwXnVrr)) | [Visit Preview](https://examples-react-highcharts-git-fork-ndrluis-trin-f54047-cube-dev.vercel.app) | | Nov 13, 2024 5:17pm | | **examples-react-material-ui** | ⬜️ Ignored ([Inspect](https://vercel.com/cube-dev/examples-react-material-ui/46DTur9V1bGr2bEiqpoqepBPQG4R)) | [Visit Preview](https://examples-react-material-ui-git-fork-ndrluis-tri-809ab1-cube-dev.vercel.app) | | Nov 13, 2024 5:17pm | | **examples-react-pivot-table** | ⬜️ Ignored ([Inspect](https://vercel.com/cube-dev/examples-react-pivot-table/EEFLLiSYquYiheE4RZZMcdnqMZji)) | [Visit Preview](https://examples-react-pivot-table-git-fork-ndrluis-tri-2cebd9-cube-dev.vercel.app) | | Nov 13, 2024 5:17pm | | **examples-vue-query-builder** | ⬜️ Ignored ([Inspect](https://vercel.com/cube-dev/examples-vue-query-builder/zHbmAaQvBtZZCam7QTcBcCPcwdYp)) | [Visit Preview](https://examples-vue-query-builder-git-fork-ndrluis-tri-f24671-cube-dev.vercel.app) | | Nov 13, 2024 5:17pm |
igorlukanin commented 1 week ago

Hi @ndrluis 👋 I think this change was long awaited for, thanks for the contribution!

For the docs, if you could update this page with new env vars and check for other instances of "Trino", that would be fantastic: https://cube.dev/docs/reference/configuration/environment-variables

paveltiunov commented 1 week ago

@ndrluis Hey Andre! Thanks for contributing! I feel it's right direction however in order to make this switch Trino should meet parity in terms of features. At the first sight at least unload support is missing. We need to have in order to land this change.

ndrluis commented 1 week ago

@paveltiunov I understand the Presto implementation, but as a Trino user, I believe it makes more sense to wait for the spooled client protocol, which solves this problem for any catalog. The current solution only works for GCS and, as far as I know, only for the Hive catalog.

I fixed the query method and removed the incorrect stream method, but I'm unsure of the best way to implement and test it, since the driver works with just the query method. Which cube feature uses the stream method?