Skip to content
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.

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

fix(sqllab): flaky json explore modal due to shallow equality checks for extra data #29978

Conversation

justinpark
Copy link
Member

SUMMARY

In the async query, the extra field of the pulling data is in object form, and when the result is completed, it fails to verify equality due to shallow equality checks.
This caused unnecessary over-rendering of the result panel, so this commit resolved the issue by comparing the extra field separately.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

before--flaky-json-modal.mov

After:

after--flaky-json-modal.mov

TESTING INSTRUCTIONS

Run a large query in a new tab, then switch to another tab.
Run a quick query that includes a JSON value, then click the value to verify that the JSON modal is shown steadily.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • 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
  • Introduces new feature or API
  • Removes existing feature or API

@dosubot dosubot bot added global:async-query Related to Async Queries feature sqllab Namespace | Anything related to the SQL Lab labels Aug 21, 2024
@michael-s-molina michael-s-molina added the v4.0 Label added by the release manager to track PRs to be included in the 4.0 branch label Aug 21, 2024
@@ -696,7 +698,19 @@ export default function sqlLabReducer(state = {}, action) {
? prevState
: currentState,
};
change = true;
const newExtra = JSON.stringify(newQueries[id].extra);
Copy link
Member

@michael-s-molina michael-s-molina Aug 21, 2024

Choose a reason for hiding this comment

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

You can use Lodash's isEqual here.

@michael-s-molina michael-s-molina added the v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch label Aug 21, 2024
@justinpark justinpark merged commit 1ca5947 into apache:master Aug 21, 2024
34 checks passed
sadpandajoe pushed a commit that referenced this pull request Aug 22, 2024
michael-s-molina pushed a commit that referenced this pull request Sep 5, 2024
@github-actions github-actions bot added 🍒 4.1.0 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels labels Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels global:async-query Related to Async Queries feature size/M sqllab Namespace | Anything related to the SQL Lab v4.0 Label added by the release manager to track PRs to be included in the 4.0 branch v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch 🍒 4.1.0 🍒 4.1.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants