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

sql: removed redundant parameter in method to schedule sql stats compaction #82560

Conversation

surahman
Copy link
Contributor

@surahman surahman commented Jun 7, 2022

The crdb_internal.schedule_sql_stats_compaction SQL function does not require the byte string parameter and has thus been removed. Closes #78332.

Jira issue: CRDB-14071

@Azhng

@surahman surahman requested a review from a team June 7, 2022 22:48
@blathers-crl
Copy link

blathers-crl bot commented Jun 7, 2022

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

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

@blathers-crl blathers-crl bot added the O-community Originated from the community label Jun 7, 2022
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rafiss rafiss requested a review from a team June 7, 2022 22:54
Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @rafiss and @surahman)


-- commits line 4 at r1:
please update the commit message:

title should begin with sql: removed parameter from ...

the release note should begin with Release note (sql change):

the release note message is confusing right now, since it makes it seem like the function was removed.

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.

Reviewed 1 of 2 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @cockroachdb, @maryliag, @rafiss, and @surahman)


pkg/sql/sem/builtins/builtins.go line 6538 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

cc @cockroachdb/sql-observability is there any frontend page that calls this function? if so, we'd have to make sure that this change is OK to make.

No, frontend pages don't make any calls to this function

@surahman surahman force-pushed the CRDB-14071-crdb_internal.schedule_sql_stats_compaction_remove_params branch from e7ed582 to 8e87552 Compare June 7, 2022 22:59
@blathers-crl
Copy link

blathers-crl bot commented Jun 7, 2022

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

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

@surahman
Copy link
Contributor Author

@rafiss all requested changes have been affected.

@surahman surahman force-pushed the CRDB-14071-crdb_internal.schedule_sql_stats_compaction_remove_params branch from 8e87552 to 25c3dfd Compare June 13, 2022 20:57
@surahman surahman changed the title [CRDB-14071] Remove parameter from crdb_internal.schedule_sql_stats_compaction sql: removed redundant parameter routine to schedule sql stats compaction Jun 13, 2022
@surahman surahman force-pushed the CRDB-14071-crdb_internal.schedule_sql_stats_compaction_remove_params branch from 25c3dfd to 0bc7213 Compare June 13, 2022 21:00
@surahman surahman changed the title sql: removed redundant parameter routine to schedule sql stats compaction sql: removed redundant parameter in method to schedule sql stats compaction Jun 13, 2022
@maryliag maryliag requested a review from a team June 14, 2022 19:45
Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @cockroachdb, @maryliag, @rafiss, and @surahman)


-- commits line 5 at r3:
nit: this note would be more clear to readers if youo mentioned the function name specifically.

"The byte string parameter in the crdb_internal.schedule_sql_stats_compaction function has been removed."

@surahman surahman force-pushed the CRDB-14071-crdb_internal.schedule_sql_stats_compaction_remove_params branch from 0bc7213 to b9e1fc3 Compare June 15, 2022 15:58
…action

Release note (sql change):
The byte string parameter in the crdb_internal.schedule_sql_stats_compaction function has been removed.
@surahman surahman force-pushed the CRDB-14071-crdb_internal.schedule_sql_stats_compaction_remove_params branch from b9e1fc3 to 5d57e0f Compare June 15, 2022 16:32
@surahman surahman requested a review from rafiss June 15, 2022 18:11
Copy link
Collaborator

@rafiss rafiss 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 your contribution!

bors r+

@craig
Copy link
Contributor

craig bot commented Jun 22, 2022

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-community Originated from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sql: crdb_internal.schedule_sql_stats_compaction takes in unncessary param
4 participants