Skip to content

MDEV-21510 optimizer trace should show the index name in the block chosen_access_method #3998

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

Conversation

bsrikanth-mariadb
Copy link

@svoj
Copy link
Contributor

svoj commented Apr 22, 2025

@bsrikanth-mariadb there should be no need to create separate pull requests for the same issue. You could've restored #3995 by force pushing to your branch.

@bsrikanth-mariadb
Copy link
Author

@bsrikanth-mariadb there should be no need to create separate pull requests for the same issue. You could've restored #3995 by force pushing to your branch.

Yeah, sure @svoj. But, my local repo was in a bad state as well. So, felt better to create a new PR.

@sanja-byelkin
Copy link
Member

Please put tests and test changes in the same commit with code changes, it is needed for merge and code investications.

@bsrikanth-mariadb
Copy link
Author

Please put tests and test changes in the same commit with code changes, it is needed for merge and code investications.

Sure Sanja, will rebase before merging.

@svoj
Copy link
Contributor

svoj commented Apr 23, 2025

@bsrikanth-mariadb there should be no need to create separate pull requests for the same issue. You could've restored #3995 by force pushing to your branch.

Yeah, sure @svoj. But, my local repo was in a bad state as well. So, felt better to create a new PR.

You had to fix your local repo in the first place and then push it to github anyway. Not a big deal, but feel free to ask for help if something goes wrong.

@bsrikanth-mariadb bsrikanth-mariadb force-pushed the 11.4-MDEV-21510-optimizer_trace_should_show_the_index_name branch from 1c2f02e to 0c42913 Compare April 23, 2025 08:27
@bsrikanth-mariadb
Copy link
Author

@bsrikanth-mariadb there should be no need to create separate pull requests for the same issue. You could've restored #3995 by force pushing to your branch.

Yeah, sure @svoj. But, my local repo was in a bad state as well. So, felt better to create a new PR.

You had to fix your local repo in the first place and then push it to github anyway. Not a big deal, but feel free to ask for help if something goes wrong.

sure Sergey, Thank you!

Copy link
Member

@spetrunia spetrunia left a comment

Choose a reason for hiding this comment

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

Please address the input.
Other than that, we're close to finish

@spetrunia
Copy link
Member

spetrunia commented Apr 29, 2025

Please make the PR have one commit, address the

Please print "index" member right after "type". It doesn't affect automatic tools but it makes the output more readable.

part.

@bsrikanth-mariadb bsrikanth-mariadb force-pushed the 11.4-MDEV-21510-optimizer_trace_should_show_the_index_name branch 2 times, most recently from cd65ef5 to 19bee15 Compare April 29, 2025 09:29
Copy link
Member

@spetrunia spetrunia left a comment

Choose a reason for hiding this comment

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

And ok to push after the above is addressed.

@bsrikanth-mariadb bsrikanth-mariadb force-pushed the 11.4-MDEV-21510-optimizer_trace_should_show_the_index_name branch 2 times, most recently from 05721a3 to c94133a Compare April 30, 2025 03:36
@bsrikanth-mariadb bsrikanth-mariadb merged commit c94133a into 11.4 Apr 30, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants