search

petoju / terraform-provider-mysql

Terraform MySQL provider – unofficial fork
https://registry.terraform.io/providers/petoju/mysql
Mozilla Public License 2.0
63 stars 40 forks source link

Change collation lookup to be durable across additional engine/versions #158

Closed zph closed 3 days ago

zph commented 1 week ago

This patch looks up collation based on INFORMATION_SCHEMA.COLLATIONS and scopes the response to just the two required columns.

The intention is that it will be a more steady reference and interface because we're able to select only the required columns rather than breaking when new columns are added to the interface for SHOW COLLATION....

Context: using this provider broke for v7.5 of TiDB but I think there's a more durable approach by looking up directly the columns needed (more below).

This patch uses that INFORMATION_SCHEMA.COLLATIONS lookup to be compatible with the following:

  1. Mysql 5.7 and 8.0 without special casing
  2. MariaDB without special casing (assumed base on prior compatibility)
  3. compatible with TiDB for 6.x and 7.x (v7.5.1 which advertises as 8.0.11-TiDB-v7.5.1 but lacks the 7th column in SHOW statement previously used)

To Merge:

petoju commented 6 days ago

Hi, I'm fine with the change itself - but could you please add tests, that will show what you are fixing and to prevent regressions?

Currently, mysql_database tests skip TiDB altogether - maybe there should be some test testing it?

zph commented 6 days ago

@petoju Sounds good 👍. I'll figure out where to integrate the testing and push it up soon for review.

Thank you!

petoju commented 3 days ago

For the tests I now marked as not running in TiDB after enabling TiDB 7.5.3, let me know if you think I should create a second skip function focused on v7 branch to avoid removing runs of those tests for v6.1.

This is also fine for now. In the longer term, they should probably be fixed. That's just to avoid someone breaking it.

petoju commented 3 days ago

Released in v3.0.62

zph commented 3 days ago

Great thank you for your time reviewing and maintaining this library 🙏

On Sat, Jun 29, 2024, at 04:30, petoju wrote:

Released in v3.0.62

— Reply to this email directly, view it on GitHub https://github.com/petoju/terraform-provider-mysql/pull/158#issuecomment-2198112746, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH2UGFL2BOXE56KG3Z3CRLZJ2LDRAVCNFSM6AAAAABJ4MFBUOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJYGEYTENZUGY. You are receiving this because you authored the thread.Message ID: @.***>

--

Zander GH: @ZPH || https://blog.xargs.io