ui: allow stmts page to search across explain plan page

[gtr]

Closes #71615.

Previously, the search functionality for the stmts page only allowed a search
through the statement query. This change adds the statement's Plan as part of
that search.

Release note (ui change): logical plan text included in searchable text for
stmts page.

Demo video: shows that the search feature works for statements' plan

Screen.Recording.2022-01-21.at.3.03.48.PM.mov

[cockroach-teamcity]

This change is 

[gtr]

Closes PR #74820. I had to switch to this PR since I had the fork on the main cockroach repository, not my fork. Thanks for the feedback on the other PR! 😄

[jocrl]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @gtr and @xinhaoz)

---

pkg/ui/workspaces/cluster-ui/src/statementDetails/planView/planView.tsx, line 64 at r1 (raw file):

}

// planNodeAttributesToString recursively converts a FlatPlanNode into a string.

Yay functions!

nit: s/planNodeAttributesToString/planNodeToString

---

pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx, line 430 at r1 (raw file):

      .filter(statement => (filters.fullScan ? statement.fullScan : true))
      .filter(statement =>
        search ? filterBySearchQuery(statement, search) : true,

Archer brought up in the prior PR that he's concerned about the performance implications of running all this every time the filter is changed (and actually, on every render).

What kind of data did you use when you were building this? I think it would be helpful to test out your filter feature with a larger table to see if you indeed run into performance issues. You can do via the following, and happy to pair if you're confused/stuck

• go to the statements table for a cluster on staging, turn on App: $internal in the filters, and change the start time to like a year back to get more statements. You should get ~700 rows
• Right click -> inspect -> Network tab -> refresh and get the return response for the statements endpoint
• Run storybook
• Feed the statement response into the storybook fixtures for the statements page, and view the story for the statements page
• Test if you have performance issues when filtering 🙂. If you do, now you have an environment where you can test whether you successfully fix them. And if you don't, maybe quadruple the number of rows for good measure and see if you do. @xinhaoz's been dealing with issues from having too much data in the statements page, and I'm not sure what the order of magnitude of rows for that is.

---

pkg/ui/workspaces/cluster-ui/src/statementDetails/planView/planView.spec.tsx, line 284 at r1 (raw file):

        "Into users(id, city, name, address, credit_card) Size 5 columns, 3 rows";

      assert.deepEqual(planNodeAttrsToString(testNodeAttrs), expectedString);

nit: I don't think you need to deep equal a string? assert.equal should do here, and same for the others.
docs for assert: https://www.chaijs.com/api/assert/#method_equal
shallow vs deep equality: https://stackoverflow.com/questions/5703609/what-is-the-difference-between-being-shallowly-and-deeply-equal-how-is-this-app

[jocrl]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @gtr and @xinhaoz)

---

pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx, line 430 at r1 (raw file):

Previously, jocrl (Josephine) wrote…

Archer brought up in the prior PR that he's concerned about the performance implications of running all this every time the filter is changed (and actually, on every render).

What kind of data did you use when you were building this? I think it would be helpful to test out your filter feature with a larger table to see if you indeed run into performance issues. You can do via the following, and happy to pair if you're confused/stuck

• go to the statements table for a cluster on staging, turn on App: $internal in the filters, and change the start time to like a year back to get more statements. You should get ~700 rows
• Right click -> inspect -> Network tab -> refresh and get the return response for the statements endpoint
• Run storybook
• Feed the statement response into the storybook fixtures for the statements page, and view the story for the statements page
• Test if you have performance issues when filtering 🙂. If you do, now you have an environment where you can test whether you successfully fix them. And if you don't, maybe quadruple the number of rows for good measure and see if you do. @xinhaoz's been dealing with issues from having too much data in the statements page, and I'm not sure what the order of magnitude of rows for that is.

Actually, now that we have the ability to point local frontends to staging backends, you can instead run that and see if the filter has performance issues on a staging cluster.

[Azhng]

Nice! Glad to see it's starting to come together!

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @gtr and @xinhaoz)

---

pkg/ui/workspaces/cluster-ui/src/statementDetails/planView/planView.tsx, line 70 at r3 (raw file):

  if (plan.children.length > 0) {
    return plan.children
      .map(child => `${str} ${planNodeToString(child)}`)

Hmm, are we duplicating the ${str} for each child of the plan here?

---

pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx, line 430 at r1 (raw file):

Previously, jocrl (Josephine) wrote…

Actually, now that we have the ability to point local frontends to staging backends, you can instead run that and see if the filter has performance issues on a staging cluster.

Just some thoughts here. Since the full text match search is likely the most expensive part of the filtering, it might be better if we move this filtering to the bottom of the chain of filters, where most of the statements has been filtered out using other relatively cheap filters (e.g. latency threshold, sql type etc.)

---

pkg/ui/workspaces/cluster-ui/src/statementDetails/planView/planView.spec.tsx, line 352 at r3 (raw file):

      const expectedString =
        "render  group (scalar)  filter filter variable = _ virtual table table cluster_settings@primary";
      assert.deepEqual(planNodeToString(testPlanNode), expectedString);

Let's also test the top-level filteryByQueryString too as well

[gtr]

Thanks for the feedback @jocrl @Azhng @xinhaoz! I made the requested changes and tested the performance of my filter function by populating the statements page with 915 statements and the filter function works just fine.

[jocrl]

Yay! A question for everyone else - what're our general guidelines for when the PR description should include a before/after screenshot or video?

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @gtr and @xinhaoz)

[maryliag]

@jocrl For UI changes, the PR must include the before/after images. For behaviour changes, videos are not required, but are welcomed.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @gtr and @xinhaoz)

[jocrl]

🎉 lgtm! Whenever you get the TeamCity build fixed 😉

For that, you want to go to the "build log" tab and find the error

It looks like eslint errors, which you can set up to auto-format on save in your editor. I know I'm saying that it's eslint, but make sure you know how to go to TeamCity and find the error 😛

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @gtr and @xinhaoz)

[xinhaoz]

Nice work! Changes look good granted CI passes. Thanks for testing it!

Reviewed 3 of 4 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Azhng, @gtr, @jocrl, @maryliag, and @xinhaoz)

[maryliag]

Great job on this issue!

Reviewed all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @Azhng, @gtr, @jocrl, @maryliag, and @xinhaoz)

[gtr]

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)