You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "soumitra-st (via GitHub)" <gi...@apache.org> on 2024/05/07 01:34:04 UTC
[PR] Enable explore button on SQL Lab view when connected to Apache Pinot as a database [superset]
soumitra-st opened a new pull request, #28364:
URL: https://github.com/apache/superset/pull/28364
Older versions of Pinot did not support subquery, hence the explore view on SQL Lab was disabled. [Apache Pinot 1.0](https://docs.pinot.apache.org/reference/multi-stage-engine) added support for subquery, join, etc. in multi-stage query engine, so this PR enables the explore button.
### Create chart button is disabled before the fix
<img width="900" alt="create-chart-disabled" src="https://github.com/apache/superset/assets/127247229/3205c19a-8f4b-41f2-a96c-400f4cb5fda8">
### Create chart button is enabled after the fix
<img width="947" alt="Screenshot 2024-05-06 at 6 32 59 PM" src="https://github.com/apache/superset/assets/127247229/1b0b0393-970e-4d03-a5c9-d9e89be52671">
### Query in explore view
<img width="1454" alt="explore-view" src="https://github.com/apache/superset/assets/127247229/8ca58b69-314f-4082-978c-8a620592604c">
### Query with alias in SQL Lab
<img width="1039" alt="alias-in-query" src="https://github.com/apache/superset/assets/127247229/66314342-c290-4634-be53-3843daef3ee0">
### Query with alias in explore
<img width="806" alt="explore-query-with-alias" src="https://github.com/apache/superset/assets/127247229/5a6c6c40-8419-46d7-90fd-f80eb23056fd">
### ADDITIONAL INFORMATION
Fixes #13623 issue
- [x] Changes UI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Enable explore button on SQL Lab view when connected to Apache Pinot as a database [superset]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #28364:
URL: https://github.com/apache/superset/pull/28364#issuecomment-2208497445
> Does pinotdb 5.3.0 support earlier versions of pinot than 1.1.0? If so then it seems like this would not be a breaking change and could be merged now in my opinion.
>
>
>
> Even if not, I expect this is worthwhile to merge at 5.0.0 and pinot users would need to update their pinot version along with the Superset upgrade.
Correct, this is not a breaking change. New client library supports both old and new query endpoints/protocols.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Enable explore button on SQL Lab view when connected to Apache Pinot as a database [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #28364:
URL: https://github.com/apache/superset/pull/28364#issuecomment-2206783374
Does pinotdb 5.3.0 support earlier versions of pinot than 1.1.0? If so then it seems like this would not be a breaking change and could be merged now in my opinion.
Even if not, I expect this is worthwhile to merge at 5.0.0 and pinot users would need to update their pinot version along with the Superset upgrade.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Enable explore button on SQL Lab view when connected to Apache Pinot as a database [superset]
Posted by "soumitra-st (via GitHub)" <gi...@apache.org>.
soumitra-st commented on PR #28364:
URL: https://github.com/apache/superset/pull/28364#issuecomment-2159481409
> breaking window for 5.0 opens
@john-bodley , shall we merge this PR as it is? If not, what is the timeline for 5.0 release?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Enable explore button on SQL Lab view when connected to Apache Pinot as a database [superset]
Posted by "soumitra-st (via GitHub)" <gi...@apache.org>.
soumitra-st commented on PR #28364:
URL: https://github.com/apache/superset/pull/28364#issuecomment-2099168327
Thanks @john-bodley for your comments. IMO, the PR is fixing a bug not breaking existing functionality. Wondering why is it a breaking change?
I tested the changes using `pinotdb==5.2.0` and Pinot version `1.1.0`. I will work on updating the supported version of [pinotdb module](https://github.com/python-pinot-dbapi/pinot-dbapi?tab=readme-ov-file#python-db-api-and-sqlalchemy-dialect-for-pinot) to 1.1.0. If I can get a release of pinotdb that is compatible with Pinot version 0.9.3 onwards, then would this change still be considered as breaking?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Enable explore button on SQL Lab view when connected to Apache Pinot as a database [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke merged PR #28364:
URL: https://github.com/apache/superset/pull/28364
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Enable explore button on SQL Lab view when connected to Apache Pinot as a database [superset]
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley commented on code in PR #28364:
URL: https://github.com/apache/superset/pull/28364#discussion_r1592832917
##########
superset/db_engine_specs/pinot.py:
##########
@@ -25,10 +25,10 @@
class PinotEngineSpec(BaseEngineSpec):
engine = "pinot"
engine_name = "Apache Pinot"
- allows_subqueries = False
- allows_joins = False
- allows_alias_in_select = False
- allows_alias_in_orderby = False
+ allows_subqueries = True
Review Comment:
These might not be necessary if they're the same as the base version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Enable explore button on SQL Lab view when connected to Apache Pinot as a database [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #28364:
URL: https://github.com/apache/superset/pull/28364#issuecomment-2099396154
Running CI 🤞 Please feel free to open a commit adding your org to our [In the Wild page](https://github.com/apache/superset/blob/master/RESOURCES/INTHEWILD.md) if you want to get a trivial PR under your belt and remove the need to manually trigger CI on every change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Enable explore button on SQL Lab view when connected to Apache Pinot as a database [superset]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #28364:
URL: https://github.com/apache/superset/pull/28364#issuecomment-2099400460
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/28364?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 64.08%. Comparing base [(`2e5f3ed`)](https://app.codecov.io/gh/apache/superset/commit/2e5f3ed85149951200645e00e79a543de9bec02f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`0ef2752`)](https://app.codecov.io/gh/apache/superset/pull/28364?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 46 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #28364 +/- ##
==========================================
+ Coverage 60.49% 64.08% +3.58%
==========================================
Files 1931 521 -1410
Lines 76241 37180 -39061
Branches 8566 0 -8566
==========================================
- Hits 46122 23826 -22296
+ Misses 28015 13354 -14661
+ Partials 2104 0 -2104
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/28364/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/28364/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `49.10% <ø> (-0.07%)` | :arrow_down: |
| [javascript](https://app.codecov.io/gh/apache/superset/pull/28364/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/28364/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.70% <ø> (-0.11%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/28364/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `64.08% <ø> (+0.59%)` | :arrow_up: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/28364/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.28% <ø> (+0.66%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/28364?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] Enable explore button on SQL Lab view when connected to Apache Pinot as a database [superset]
Posted by "soumitra-st (via GitHub)" <gi...@apache.org>.
soumitra-st commented on PR #28364:
URL: https://github.com/apache/superset/pull/28364#issuecomment-2099318608
@john-bodley I checked internally, the latest pinotdb module supports Pinot version 1.1.0, and we have merged the PR to update the version [here](https://github.com/python-pinot-dbapi/pinot-dbapi?tab=readme-ov-file#python-db-api-and-sqlalchemy-dialect-for-pinot).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org