Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(driver): introduce Apache Pinot #8689

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jronsse
Copy link

@jronsse jronsse commented Sep 10, 2024

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

#8579

Description of Changes Made

The driver uses Pinot's Broker Rest API to submit queries. Multi-stage engine is enabled by default for every query since it offers better SQL support.
Published to npm npm i @inthememory/pinot-driver

@jronsse jronsse requested review from a team as code owners September 10, 2024 15:43
Copy link

vercel bot commented Sep 10, 2024

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

@github-actions github-actions bot added the pr:community Contribution from Cube.js community members. label Sep 10, 2024
Copy link
Member

@paveltiunov paveltiunov left a comment

Choose a reason for hiding this comment

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

@jronsse Thanks for contributing this one! This is really huge! Overall looks great! We need to do only minor polishing I mentioned in comments.


### Installation

`npm i @inthememory/pinot-driver`
Copy link
Member

Choose a reason for hiding this comment

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

We don't need those instructions as it'd be packed as a part of distribution.

}

public hllInit(sql: string) {
return this.countDistinctApprox(sql); // todo: ensure the correct way to do so in pinot
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem correct so let's remove it for now to avoid weird error messages

}

public hllMerge(sql: string) {
return this.countDistinctApprox(sql); // todo: ensure the correct way to do so in pinot
Copy link
Member

Choose a reason for hiding this comment

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

This one as well


## SSL

Cube does not require any additional configuration to enable SSL as Elasticsearch connections are made over HTTPS.
Copy link
Member

Choose a reason for hiding this comment

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

I guess it should refer to Pinot instead of Elasticsearch?

`npm i @inthememory/pinot-driver`

### Usage
#### For Docker
Copy link
Member

Choose a reason for hiding this comment

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

This is as well


[Learn more](https://github.com/cube-js/cube.js#getting-started)

### Project Status
Copy link
Member

Choose a reason for hiding this comment

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

I guess we don't need it

/**
* @copyright Cube Dev, Inc.
* @license Apache-2.0
* @fileoverview The `PrestoDriver` and related types declaration.
Copy link
Member

Choose a reason for hiding this comment

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

I guess it's PinotDriver instead of PrestoDriver

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:community Contribution from Cube.js community members.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants