fix(sqllab): SqlJsonExecutionContext.query null pointer

[serenajiang]

SUMMARY

Users saw this error when their sqllab queries failed due to access errors:
'SqlJsonExecutionContext' object has no attribute 'query'

The issue is that _validate_access is run before set_query, so the execution_context does not have a query attribute during _validate_access, which may throw an exception that uses query details to generate an error message. This was likely a regression caused by #16852.

I am not very satisfied with this fix - SqlJsonExecutionContext should probably always have a query attribute since it's a dataclass? But I'm not sure what the intended behavior is, so input is welcome.

@ofekisr @etr2460

[codecov]

Codecov Report

Merging #16997 (e5fecde) into master (c993c58) will decrease coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #16997      +/-   ##
==========================================
- Coverage   76.93%   76.85%   -0.08%     
==========================================
  Files        1030     1030              
  Lines       55029    55029              
  Branches     7465     7465              
==========================================
- Hits        42338    42294      -44     
- Misses      12437    12481      +44     
  Partials      254      254              

Flag	Coverage Δ
hive	81.45% <100.00%> (ø)
mysql	81.90% <100.00%> (ø)
postgres	81.91% <100.00%> (ø)
presto	?
python	82.26% <100.00%> (-0.16%)	⬇️
sqlite	81.58% <100.00%> (ø)

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

Impacted Files	Coverage Δ
superset/sqllab/sqllab_execution_context.py	90.55% <100.00%> (ø)
superset/db_engine_specs/presto.py	84.30% <0.00%> (-6.07%)	⬇️
superset/connectors/sqla/models.py	85.83% <0.00%> (-1.42%)	⬇️
superset/models/core.py	89.26% <0.00%> (-0.74%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c993c58...e5fecde. Read the comment docs.

[etr2460]

ehh, i think this is fine. It's only for getting query details anyway

[amitmiran137]

maybe we should also add unit test for that cover that usecase in SqlJsonExecutionContext so future would be stronger

[AAfghahi]

🏷 v1.4