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

Modified the query statement to check information schema #166

Merged
merged 3 commits into from
Sep 19, 2024

Conversation

Khadreal
Copy link
Contributor

@Khadreal Khadreal commented Sep 19, 2024

This PR attempts to fix this issue #165

The current usage of SHOW TABLES LIKE work fine but for large db it have a negative effect on performance, changing that to SELECT.. statements is more efficient and improve performance.

@JJJ
Copy link
Collaborator

JJJ commented Sep 19, 2024

Hi and thanks for this PR! I have some thoughts!

I went with SHOW TABLES LIKE for a few reasons:

  • maintain SHOW SQL pattern used in Table::status/columns/column|index_exists()
  • in case the user that is authenticating with the SQL server (via WPDB) does not have read access to what is being queried from information_schema.
    (This was always why WordPress did not query it directly either; at least until 6.1 when Site Health introduced a query for it in should_suggest_persistent_object_cache()!)

But, I can see the appeal of wanting this to be as fast as possible, so let's merge it.

Any thoughts on how this new SQL will work related to #104?

@JJJ JJJ merged commit 8c4fcd9 into berlindb:master Sep 19, 2024
@MathieuLamiot
Copy link

Thanks @JJJ for the reasoning behind SHOW TABLES LIKE. Do you think there are setups in the WP ecosystem that would have read access restrictions on information_schema? I am thinking maybe some shared hosting plans, even if that sounds edgy 🤔
I'm wondering if we should provide an option to use SHOW TABLES LIKE as a workaround, or if this is not something needed in the ecosystem anymore. Thanks 🙏

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.

4 participants