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

ui: removed formatting to statements on the details pages #75443

Merged
merged 2 commits into from
Jan 25, 2022

Conversation

THardy98
Copy link

@THardy98 THardy98 commented Jan 24, 2022

ui: removed formatting to statements on the details pages

Previously, statements displayed on the statement/transaction/index
details pages were formatted (formatting was added to allow for better
readability of statements on these detail pages). However, statements
queries from the frontend were noticeably slower due to this
implementation. This change reverts the changes to statement formatting
(updates the fixtures to show the non-formatted statements), but keeps
the change that uses statement ID as an identifier in the URL instead of
the raw statement.

Reference: Original PR

Release note (ui change): change to replace raw
statement with statement ID in the URL.

@THardy98 THardy98 requested a review from a team January 24, 2022 16:17
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Thomas Hardy added 2 commits January 24, 2022 11:21
Previously, all statements were formatted prior to being sent to the
frontend. However, the formatting was causing statements queries from
the frontend to run noticeably more slowly. This change removes the
logic that formats all queries, but keeps the addition of the new
builtin function prettify_statement.

Release note (sql change): statements are no longer formatted prior to
being sent to the UI, but the new builtin function remains.
Previously, statements displayed on the statement/transaction/index
details pages were formatted (formatting was added to allow for better
readability of statements on these detail pages). However, statements
queries from the frontend were noticeably slower due to this
implementation. This change reverts the changes to statement formatting
(updates the fixtures to show the non-formatted statements), but keeps
the change that uses statement ID as an identifier in the URL instead of
the raw statement.

Release note (ui change): removed formatting to statements on the
statement, transaction and index details pages, change to replace raw
statement with statement ID in the URL remained.
@THardy98 THardy98 force-pushed the revert_formatting_statements branch from ab2b429 to 39604d8 Compare January 24, 2022 16:51
Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

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

Thanks for making those changes! :lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@THardy98
Copy link
Author

bors r+

@THardy98
Copy link
Author

blathers backport 21.2

@blathers-crl
Copy link

blathers-crl bot commented Jan 24, 2022

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from c02b3b1 to blathers/backport-release-21.2-75443: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 21.2 failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@craig
Copy link
Contributor

craig bot commented Jan 25, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Jan 25, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Jan 25, 2022

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Jan 25, 2022

Build succeeded:

@craig craig bot merged commit da5d0fc into cockroachdb:master Jan 25, 2022
@blathers-crl
Copy link

blathers-crl bot commented Jan 25, 2022

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from c02b3b1 to blathers/backport-release-21.2-75443: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 21.2.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants