Skip to content

feat(pinot-driver): Add enableNullHandling to query options using env var #9213

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Feb 12, 2025
Merged

feat(pinot-driver): Add enableNullHandling to query options using env var #9213

merged 8 commits into from
Feb 12, 2025

Conversation

@sergisulca
Copy link
Contributor

@sergisulca sergisulca commented Feb 11, 2025 •
edited by KSDaemon
Loading

Check List

  • Tests have 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

@sergisulca sergisulca requested review from a team as code owners February 11, 2025 21:23
@github-actions github-actions bot added the pr:community Contribution from Cube.js community members. label Feb 11, 2025
@sergisulca sergisulca mentioned this pull request Feb 11, 2025
Closed
4 tasks
@sergisulca sergisulca changed the title Pinot driver addl option Pinot driver addl option using env var Feb 11, 2025
KSDaemon
KSDaemon requested changes Feb 11, 2025
View reviewed changes
Copy link
Member

@KSDaemon KSDaemon left a comment

Choose a reason for hiding this comment

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

Hey @sergisulca Thnx for your contribution!
You've made it mosty right. However, the name of the env var should be changed a bit as it is specific to the Pinot Driver.

ron-damon
ron-damon reviewed Feb 12, 2025
View reviewed changes
ron-damon
ron-damon reviewed Feb 12, 2025
View reviewed changes
@KSDaemon KSDaemon changed the title Pinot driver addl option using env var feat(pinot-driver): Add option for setting null handling Feb 12, 2025
@sergisulca sergisulca changed the title feat(pinot-driver): Add option for setting null handling feat(pinot-driver): Add enableNullHandling to query options using env var Feb 12, 2025
KSDaemon
KSDaemon approved these changes Feb 12, 2025
View reviewed changes
Copy link
Member

@KSDaemon KSDaemon left a comment

Choose a reason for hiding this comment

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

@sergisulca @ron-damon I think this looks good now! 👍🏻 Thank you for this contribution!

@KSDaemon KSDaemon merged commit 4d9452a into cube-js:pinot-driver-addl-option Feb 12, 2025
46 checks passed
@marianore-muttdata
Copy link
Contributor

marianore-muttdata commented Mar 6, 2025

Hi @KSDaemon! We noted that this contribution was merged into an internal branch rather than master, so as you can see in the current code, the feature is not available.
Should we create another PR with master source or just reuse this one?
Let me know! :D

@KSDaemon
Copy link
Member

KSDaemon commented Mar 6, 2025

Ahaha) @marianore-muttdata Thnx for pointing it out!)) My bad!
Yeah, That would be great! As I can't find the pinot-driver-addl-option branch...
Assign it and ping me when you'll be ready!

@marianore-muttdata marianore-muttdata mentioned this pull request Mar 6, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@KSDaemon KSDaemon KSDaemon approved these changes

+1 more reviewer

@ron-damon ron-damon ron-damon left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

pr:community Contribution from Cube.js community members.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sergisulca @marianore-muttdata @KSDaemon @ron-damon