cube icon indicating copy to clipboard operation
cube copied to clipboard

feat(vertica-driver): VerticaDriver

Open timbrownls20 opened this issue 2 years ago • 15 comments

Check List

  • [ ] Tests has been run in packages where changes made if available
  • [ ] Linter has been run for changed code
  • [ ] Tests for the changes have been added if not covered yet
  • [ ] Docs have been added / updated if required

Issue Reference this PR resolves

[#690]

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

New @cubejs-backend/vertica-driver package based on vertica-nodejs -- it includes driver itself and basic tests for overridden BaseDriver methods.

published to npm npm i @knowitall/vertica-driver

continuation of this stale PR https://github.com/cube-js/cube/pull/5100

timbrownls20 avatar Oct 24 '23 04:10 timbrownls20

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) Visit Preview Sep 17, 2024 4:24am
examples-react-d3 ⬜️ Ignored (Inspect) Visit Preview Sep 17, 2024 4:24am
examples-react-dashboard ⬜️ Ignored (Inspect) Visit Preview Sep 17, 2024 4:24am
examples-react-data-table ⬜️ Ignored (Inspect) Visit Preview Sep 17, 2024 4:24am
examples-react-highcharts ⬜️ Ignored (Inspect) Visit Preview Sep 17, 2024 4:24am
examples-react-material-ui ⬜️ Ignored (Inspect) Visit Preview Sep 17, 2024 4:24am
examples-react-pivot-table ⬜️ Ignored (Inspect) Visit Preview Sep 17, 2024 4:24am
examples-vue-query-builder ⬜️ Ignored (Inspect) Visit Preview Sep 17, 2024 4:24am

vercel[bot] avatar Oct 24 '23 04:10 vercel[bot]

@timbrownls20 Thanks for contributing! Could you please write a readme on your npm package on how to install and use it? We'll add a backlink to it in the third-party driver list.

paveltiunov avatar Oct 25 '23 03:10 paveltiunov

@timbrownls20 Thanks for contributing! Could you please write a readme on your npm package on how to install and use it? We'll add a backlink to it in the third-party driver list.

No problem. Added instructions to README for the npm package. Wasn't sure what you wanted, so hope this is OK. If you need amends could you post a link to an example. Many Thanks

timbrownls20 avatar Oct 26 '23 05:10 timbrownls20

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 47.04%. Comparing base (5a27200) to head (ea68485). Report is 32 commits behind head on master.

:exclamation: Current head ea68485 differs from pull request most recent head 04334c0. Consider uploading reports for the commit 04334c0 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7289      +/-   ##
==========================================
- Coverage   48.02%   47.04%   -0.99%     
==========================================
  Files         154      155       +1     
  Lines       21019    20768     -251     
  Branches     5264     5224      -40     
==========================================
- Hits        10095     9770     -325     
+ Misses      10723    10678      -45     
- Partials      201      320     +119     
Flag Coverage Δ
cube-backend 47.04% <ø> (-0.99%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 27 '23 19:10 codecov[bot]

@timbrownls20 I actually meant README of @knowitall/vertica-driver. Please see the process here https://github.com/cube-js/cube/blob/master/CONTRIBUTING.md#contributing-database-drivers. You can see https://www.npmjs.com/package/arangodb-cubejs-driver as an example.

paveltiunov avatar Oct 28 '23 05:10 paveltiunov

@timbrownls20 I actually meant README of @knowitall/vertica-driver. Please see the process here https://github.com/cube-js/cube/blob/master/CONTRIBUTING.md#contributing-database-drivers. You can see https://www.npmjs.com/package/arangodb-cubejs-driver as an example.

No problem. README update in knowitall npm

I've also updated the package refs in the PR to be @knowitall/vertica-driver otherwise you get errors with the refresh as it loads a package which doesn't yet exist. I assume we'll change the package to @cubejs-backend/vertoca-driver at the point the PR is merged.

Let me know if you need anything else. Thanks

timbrownls20 avatar Oct 30 '23 22:10 timbrownls20

@timbrownls20 Great! Added it here https://cube.dev/docs/product/configuration/data-sources#third-party-drivers. What's the best way to communicate with you regarding issues? We'll start forwarding all conversations about this Vertica driver to you.

paveltiunov avatar Nov 30 '23 04:11 paveltiunov

@timbrownls20 Great! Added it here https://cube.dev/docs/product/configuration/data-sources#third-party-drivers. What's the best way to communicate with you regarding issues? We'll start forwarding all conversations about this Vertica driver to you.

I've had a shared mailbox set up for our development team. I'll assign a dev to answer queries as they come up - [email protected]

timbrownls20 avatar Dec 06 '23 00:12 timbrownls20

@paveltiunov there any ETA for merging this PR ? something is missing here?

guyzilb avatar Mar 13 '24 14:03 guyzilb

@guyzilb Did you have success using it as @knowitall/vertica-driver? Could you please describe your experience?

igorlukanin avatar Mar 19 '24 19:03 igorlukanin

@igorlukanin Hey, we also wrote Vertica driver and we already work with it in production and it works pretty well, also I tested @knowitall/vertica-driver,(and t looks the same as we did, we also used korowa)
the main problem I see here is with cubestore, you can create preagg with it but you cant change the granularity from day to month for example the reason for it is that the type of the timestamp column written inside the parquet file is not correct, we try to find where to change it but still not found where we need to implement solution for it. also, roll-up join and Lamda Join not work with the Vertica driver because is generate the join condition not correct ( and also I think is because of the problem with the parquet file ) we using external preagg and just with roll-up preagg the problem here is the table that was created inside Vertica without partitions and projection which is very important in Vertica. this is my experience, I hope it helps :)

guyzilb avatar Mar 19 '24 19:03 guyzilb

Hi @guyzilb, in the latest commits on this branch we started to implement the SQL dialect. Do you know if there is an override on the BaseQuery that needs to be added for the timestamp and join conditions there perhaps?

@paveltiunov / @igorlukanin

  • Is it ok if the driver is changed to TS? I can see a number of other implementations are, though I also saw a note which mentioned the driver deps should ideally be browser and node compatible
  • Should commits on the branch be squashed? Typically we squash when we merge a PR, so we left all individual commits in for now

enhanse avatar Mar 22 '24 03:03 enhanse

@enhanse Hey, we worked on this a few months ago, but if I remember well we did not override something in BaseQuery and it was in PreAggregations we saw the join for roll-up join preagg take not the correct name of join column, is take the name of the dimension before the pre-agg example :

the join between preagg should be on a.user_id=b.user_id first pre agg called: test_1 second pre agg called: test_2

When Preagg runs some prefix is added to the column name so the join should be test_1_user_id = test_2_user_id

but what happened is : a.user_id=b.user_id
And because it is not true, we got an error.

I hope I remember it well

guyzilb avatar Mar 25 '24 08:03 guyzilb

Hi @igorlukanin. We've just brought the driver back uptodate with version 0.36.0. If possible we'd like to get this merged into the main branch - anything else we would need to do? Many Thanks

timbrownls20 avatar Sep 17 '24 04:09 timbrownls20