feat(sqllab): save query parameters in database

[mayurnewase]

SUMMARY

• Saves template parameters when query is saved.
• Updates frontend to use v1 saved query rest api,
• Removes unnecessary parameters from the payload which api doesn't need.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

1. Enable ENABLE_TEMPLATE_PROCESSING flag
2. write any query in sqllab with params eg.: SELECT '{{ p1 }}' and set parameters {"p1": "value from params"}
3. save the query and close the tab then go to saved queries
4. when saved query is opened, paramter values should show up exactly as it was saved, and when query is run it should show 1 column with value value from params
5. updating the query with sql or parameters should correctly update the query.
6. also test changing the query name, and it should update tab name.

ADDITIONAL INFORMATION

• Has associated issue: [SQL Lab] Saved Queries do not include parameters #20576
• Required feature flags: ENABLE_TEMPLATE_PROCESSING
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided -> not needed, cuz it just adds 1 column without default value
• Introduces new feature or API
• Removes existing feature or API

[villebro]

@mayurnewase I sometimes wonder what purpose query parameters have in SQL Lab, other than while developing virtual table queries that are meant to accept query parameters. Do you have a clear use case for persisting query params? It would be great to understand how this feature is usually leveraged in the wild.

[mayurnewase]

hi @villebro I have seen similar issue raised in the associated ticket.
And 1 more case when people write and save query with parameters and share with other people who are not familier with sql and just want to modify the parameters, so sql writers need to send parameters json separately.

Do you think its worth saving it in the db?

[villebro]

hi @villebro I have seen similar issue raised in the associated ticket. And 1 more case when people write and save query with parameters and share with other people who are not familier with sql and just want to modify the parameters, so sql writers need to send parameters json separately.

Do you think its worth saving it in the db?

Hi,

yes, I think the params should be persisted if they weren't previously if there's a clear use case for it. Personally I've just never really seen a very clear use case for query params in SQL Lab, but that may just be me and how I use SQL Lab. But IMO this PR looks good, so I'm happy to review/test when it's ready.

[codecov]

Codecov Report

Merging #21682 (764c14e) into master (31895f4) will increase coverage by 0.01%.
The diff coverage is 58.82%.

@@            Coverage Diff             @@
##           master   #21682      +/-   ##
==========================================
+ Coverage   66.82%   66.84%   +0.01%     
==========================================
  Files        1799     1799              
  Lines       68830    68886      +56     
  Branches     7332     7320      -12     
==========================================
+ Hits        45997    46047      +50     
+ Misses      20953    20952       -1     
- Partials     1880     1887       +7     

Flag	Coverage Δ
hive	52.92% <100.00%> (+0.02%)	⬆️
javascript	53.14% <53.33%> (-0.01%)	⬇️
mysql	78.24% <100.00%> (+0.03%)	⬆️
postgres	78.30% <100.00%> (+0.03%)	⬆️
presto	52.82% <100.00%> (+0.02%)	⬆️
python	81.44% <100.00%> (+0.02%)	⬆️
sqlite	76.80% <100.00%> (+0.02%)	⬆️
unit	50.95% <100.00%> (+0.03%)	⬆️

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

Impacted Files	Coverage Δ
...frontend/src/SqlLab/components/SaveQuery/index.tsx	75.00% <0.00%> (-1.75%)	⬇️
...frontend/src/SqlLab/components/SqlEditor/index.jsx	55.67% <ø> (+1.82%)	⬆️
superset-frontend/src/SqlLab/reducers/sqlLab.js	35.22% <0.00%> (ø)
superset-frontend/src/SqlLab/actions/sqlLab.js	62.66% <75.00%> (+0.72%)	⬆️
...src/SqlLab/components/SqlEditorTabHeader/index.tsx	90.47% <100.00%> (ø)
superset-frontend/src/SqlLab/fixtures.ts	100.00% <100.00%> (ø)
superset/models/sql_lab.py	74.80% <100.00%> (-1.72%)	⬇️
superset/queries/saved_queries/api.py	95.28% <100.00%> (ø)
...src/filters/components/Range/RangeFilterPlugin.tsx	70.75% <0.00%> (-2.84%)	⬇️
superset-frontend/src/filters/utils.ts	92.10% <0.00%> (-2.64%)	⬇️
... and 65 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[villebro]

Very nice work @mayurnewase ! LGTM and tested to work as expected

[villebro]

I love the migration to v1 API! 🚀

[kenho811]

@mayurnewase

Thank you so much for adding this feature!!

I was one of those who asked for that, because I had some complex jinja templated SQL query and I had trouble using the Native Dashboard filter to allow users to click and select the desired values for substitution.

#20576 (comment)

With your new feature, users can simply take the templated SQL, update the saved paramters and press RUN to get the output (as raw data without visualisation).

THank you :)))

Now I cannot wait until a new release is ready with your feature :))

[villebro]

Thanks @kenho811 for sharing your use case! And thanks again @mayurnewase for taking this on and doing a thorough cleanup of the related code + adding tests, really stellar work!