-
Notifications
You must be signed in to change notification settings - Fork 104
refine redundant index message to indicate false positives are possible #238
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
Looks perfect, and thank you for also tackling the invisible/ignored part, this is great! Merging momentarily. re: tests, yeah the existing integration test framework for the linter subpackage will already confirm the checker is triggered where expected (see internal/linter/testdata/validcfg/dupeidx.sql), but as you mentioned that aspect of the checker hasn't changed here anyway. The testing framework currently just catches "is this checker flagging the things we expect, and not anything else" but it doesn't handle specific text matching yet. Regarding the invisible/ignored logic, we can be fairly confident your code is correct, since the test coverage for check_dupe_index.go has remained at 100%. That makes sense since GH Actions is currently configured to use MySQL 5.7 (no invisible/ignored), MySQL 8.0 (invisible) and MariaDB 10.11 (ignored) so all 3 cases there are indeed expected to be covered 👍 |
Skeema v1.12.1 has been released, including this change. Thanks again for the excellent pull request! |
| Package | Package file | Manager | Update | Change | |---|---|---|---|---| | [skeema](https://github.com/skeema/skeema) | misk/bin/hermit | hermit | patch | `1.12.0` -> `1.12.1` | --- ### Release Notes <details> <summary>skeema/skeema (skeema)</summary> ### [`v1.12.1`](https://github.com/skeema/skeema/releases/tag/v1.12.1) - **MySQL 9.0 and 9.1** support ([`7c94fcb`](skeema/skeema@7c94fcb), [`47bfc0a`](skeema/skeema@47bfc0a), [`a0a060a`](skeema/skeema@a0a060a)) - The new `VECTOR` column type is supported. In [`skeema diff`](https://www.skeema.io/docs/commands/diff/) and [`skeema push`](https://www.skeema.io/docs/commands/push/), altering a column type between `VECTOR` and any other sufficiently-large binary type is permitted as a [safe operation](https://www.skeema.io/docs/features/safety/#unsafe-change-detection) since the conversion is non-lossy. - MySQL 9 finally processes "inline" foreign key definitions (that is, `REFERENCES` clause in a column definition). These are supported as-is in Skeema. - **MariaDB 11.5 and 11.6** support ([`6165c90`](skeema/skeema@6165c90), [`f24ad30`](skeema/skeema@f24ad30)) - MariaDB 11.5 changes the default collation for Unicode charsets to use uca1400\_ai_ci collations, which are fully supported in Skeema. - MariaDB 11.5 solves the `TIMESTAMP` col type's previous Y2K38 limitation, and Skeema's [lint-has-time](https://www.skeema.io/docs/options/#lint-has-time) annotation message has been adjusted accordingly. - **`CHECK` constraint improvement**: When a diff only affects the *name* of a `CHECK` constraint without modifying its check expression, [`skeema diff`](https://www.skeema.io/docs/commands/diff/) and [`skeema push`](https://www.skeema.io/docs/commands/push/) now ignore this cosmetic change by default. This improves compatibility with external [OSC tools](https://www.skeema.io/docs/features/osc/), which inherently need to rename `CHECK` constraints as part of their operation. This new behavior can be overridden by enabling the [--exact-match](https://www.skeema.io/docs/options/#exact-match) option. ([`f000616`](skeema/skeema@f000616)) - **[Event](https://www.skeema.io/docs/features/events/) handling** improvements and fixes ([Skeema Premium](https://www.skeema.io/download/)) - When an event diff only included a change to the `DEFINER` clause, and no other differences, the `ALTER EVENT` emitted by Skeema was not valid SQL (despite conforming to syntax in the MySQL and MariaDB manuals). To fix this situation, the SQL will now also include an additional no-op clause, such as `ENABLE` for an event that is already enabled. - Several dump normalizations for `CREATE EVENT` statements were inadvertently omitted the first time an event was dumped by [`skeema init`](https://www.skeema.io/docs/commands/init/) or [`skeema pull`](https://www.skeema.io/docs/commands/pull/). - If any workspace query failed (e.g. query timeout), and any events were present in the \*.sql definitions, a panic would result instead of the intended workspace query failure error message. ([#​229](skeema/skeema#229)) - **[SSH tunnel](https://www.skeema.io/docs/features/ssh/) enhancements** ([Skeema Premium](https://www.skeema.io/download/)) - CAs, which are indicated in the known_hosts file using `@cert-authority` lines, are now fully supported. ([skeema/knownhosts#8](skeema/knownhosts#8), [skeema/knownhosts#9](skeema/knownhosts#9)) - known_hosts lines using non-default ports are now matched properly. ([skeema/knownhosts#10](skeema/knownhosts#10)) - If any hand-written \*.sql files use the optional **`CREATE OR REPLACE` SQL syntax**, Skeema now parses and ignores the `OR REPLACE` clause. Previously, use of this syntax would prevent Skeema from parsing the statement. ([`6805737`](skeema/skeema@6805737)) - Enhancements for [Docker workspaces](https://www.skeema.io/docs/features/workspaces/#docker-workspaces) - Significant performance improvements for several common situations ([`d348249`](skeema/skeema@d348249), [`ca85df7`](skeema/skeema@ca85df7), [`7a40155`](skeema/skeema@7a40155)) - When using Percona Server 8.x, the Docker image / point release selection logic has been improved ([`e09350c`](skeema/skeema@e09350c), [`fe55d62`](skeema/skeema@fe55d62), [`af1b3b5`](skeema/skeema@af1b3b5)) - When a redundant non-unique index is flagged by [lint-dupe-index](https://www.skeema.io/docs/options/#lint-dupe-index), the annotation message is now clearer (since false positives may be possible) and suggests making the index be `INVISIBLE` / `IGNORED` before dropping ([#​238](skeema/skeema#238), [#​237](skeema/skeema#237)) - MariaDB's August 2024 point releases have changed the formatting of compressed columns in `SHOW CREATE TABLE`, which affected Skeema's [diff logic safeguards](https://www.skeema.io/docs/features/safety/#table-introspection-validation). This change is now handled and compressed columns are fully supported again. ([`49aed41`](skeema/skeema@49aed41)) - Minor wording changes in log messages and help text, for consistency. ([`4f8fa44`](skeema/skeema@4f8fa44), [`5f7598e`](skeema/skeema@5f7598e)) **Thank you** to all code contributors and issue reporters! An [installation guide](https://www.skeema.io/docs/install/) and [full documentation](https://www.skeema.io/docs/) are available on our website [skeema.io](https://www.skeema.io/). </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 6pm every weekday,before 2am every weekday" in timezone Australia/Melbourne, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). GitOrigin-RevId: 90a37c8a0c3c86b3fa245502cfeefd429129ebed
Fixes #237
There are no tests currently, because I didn't see a straight forward way to test (the rules of if something is redundant is unchanged). I did perform a manual test: