Change collation lookup to be durable across additional engine/versions #158
Conversation
This patch looks up collation based on INFORMATION_SCHEMA.COLLATIONS and scopes the response to just the two required columns. With this patch it's: 1. Mysql 5.7 and 8.0 without special casing 2. MariaDB without special casing 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)
|
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, |
|
@petoju Sounds good 👍. I'll figure out where to integrate the testing and push it up soon for review. Thank you! |
These are skipped because of syntactical differences between mysql and TiDB. These tend to have mariadb/rds/tidb matching more closely in behavior.
There was a problem hiding this comment.
@petoju Ok, I did a draft of this today. Please take a look when you have time and let me know what questions / feedback you have.
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. |
|
Released in v3.0.62 |
|
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 <#158 (comment)>, 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
|
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.COLLATIONSlookup to be compatible with the following:To Merge: