fix: Enable explore button on SQL Lab view when connected to Apache Pinot as a database

[soumitra-st]

Older versions of Pinot did not support subquery, hence the explore view on SQL Lab was disabled. Apache Pinot 1.0 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

Create chart button is enabled after the fix

Query in explore view

Query with alias in SQL Lab

Query with alias in explore

ADDITIONAL INFORMATION

Fixes #13623 issue

• Changes UI

[github-actions]

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

[john-bodley]

@soumitra-st thanks for the change. Per here it seems like the current version of the DB-API only supports Pinto version 0.9.3 (I'm not sure if this is a single version or a limit bound) and thus I don't think we can merge this.

Additionally this change would be deemed breaking for anyone using an older version of Pinot and thus cannot be merged (as is) until the breaking window for 5.0 opens.

In the interim you can locally override your version of the PinotEngineSpec to include the relevant settings.

[soumitra-st]

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 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?

[soumitra-st]

@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.

[rusackas]

Running CI 🤞 Please feel free to open a commit adding your org to our In the Wild page if you want to get a trivial PR under your belt and remove the need to manually trigger CI on every change.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.69%. Comparing base (76d897e) to head (7bea705).
Report is 966 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #28364       +/-   ##
===========================================
+ Coverage   60.48%   83.69%   +23.21%     
===========================================
  Files        1931      518     -1413     
  Lines       76236    37642    -38594     
  Branches     8568        0     -8568     
===========================================
- Hits        46114    31506    -14608     
+ Misses      28017     6136    -21881     
+ Partials     2105        0     -2105     

Flag	Coverage Δ
hive	48.87% <ø> (-0.30%)	⬇️
javascript	?
mysql	77.27% <ø> (?)
postgres	77.35% <ø> (?)
presto	53.47% <ø> (-0.34%)	⬇️
python	83.69% <ø> (+20.21%)	⬆️
sqlite	76.80% <ø> (?)
unit	59.16% <ø> (+1.53%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[soumitra-st]

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?

[xiangfu0]

Latest pinotdb version: 5.3.0 supports newer version of Pinot.
Can you please also update the pinotdb version to pinot = ["pinotdb>=5.0.0, <6.0.0"] here: https://github.com/apache/superset/blob/master/pyproject.toml#L154

[sfirke]

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.

[xiangfu0]

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 (<1.0.0) and new (>=1.0.0) query endpoints/protocols.