-
Notifications
You must be signed in to change notification settings - Fork 1k
Add visible field to schema #1043
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
base: master
Are you sure you want to change the base?
Add visible field to schema #1043
Conversation
3af6107
to
c7a82aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest lgtm
// - "8.0.25-0ubuntu0.20.04.1" | ||
func isMySQL8OrAbove(version string) bool { | ||
|
||
// Extract the version number before any additional info (like "-log", "-ubuntu", etc.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CompareServerVersions
has been updated, the comment says "ignoring anything past -
in version.". I think you can directly use it.
@@ -338,6 +391,14 @@ func (ta *Table) fetchIndexes(conn mysql.Executer) error { | |||
colName, _ := r.GetString(i, 4) | |||
currentIndex.AddColumn(colName, cardinality) | |||
currentIndex.NoneUnique, _ = r.GetUint(i, 1) | |||
|
|||
// Only set to false if explicitly marked as invisible | |||
if hasInvisibleIndex { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: considering MariaDB use case, we can check the columns returned by show index
to know if there are Visible
column.
} | ||
|
||
func hasInvisibleIndexSupport(conn mysql.Executer) bool { | ||
versionQuery := "SELECT VERSION()" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can use the version from the handshake and skip the extra roundtrip?
This pull request introduces a new feature targeting MySQL version 8 and above, where the concept of invisible indexes has been introduced. Specifically, this PR adds a Visible field to the schema.Index struct, allowing users to easily filter out invisible indexes.
We encountered an issue with ghostferry where an invisible index was mistakenly selected as the shardingKeyIndexName. To address this, we initially applied a patch directly to the ghostferry project, as documented in ghostferry PR #383. This current PR seeks to integrate those changes into the upstream codebase for broader applicability. Part of Shopify/ghostferry#384.