Skip to content

Commit d108d09

Browse files
chenhuiyehchenhuiyeh
committed
use show index columns to determine visible index and linting
1 parent c7a82aa commit d108d09

File tree

2 files changed

+94
-68
lines changed

2 files changed

+94
-68
lines changed

‎schema/schema.go

Lines changed: 22 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -320,54 +320,28 @@ func (ta *Table) fetchColumnsViaSqlDB(conn *sql.DB) error {
320320
return r.Err()
321321
}
322322

323-
// isMySQL8OrAbove checks if the given version string represents MySQL 8.0 or above.
324-
// It handles various MySQL version formats including:
325-
// - "8.0.25"
326-
// - "8.4.0"
327-
// - "8.0.25-log"
328-
// - "8.0.25-0ubuntu0.20.04.1"
329-
func isMySQL8OrAbove(version string) bool {
330-
331-
// Extract the version number before any additional info (like "-log", "-ubuntu", etc.)
332-
versionParts := strings.Fields(version)
333-
if len(versionParts) == 0 {
334-
return false
335-
}
336-
337-
// Take the first part and remove any suffix after hyphen
338-
versionStr := strings.Split(versionParts[0], "-")[0]
339-
340-
// Use semver comparison for accurate version checking
341-
if eq, err := mysql.CompareServerVersions(versionStr, "8.0.0"); err == nil && eq >= 0 {
342-
return true
323+
// hasInvisibleIndexSupportFromResult checks if the result from SHOW INDEX has Visible column
324+
func hasInvisibleIndexSupportFromResult(r *mysql.Result) bool {
325+
for name := range r.FieldNames {
326+
if strings.EqualFold(name, "Visible") {
327+
return true
328+
}
343329
}
344-
345330
return false
346331
}
347332

348-
func hasInvisibleIndexSupport(conn mysql.Executer) bool {
349-
versionQuery := "SELECT VERSION()"
350-
r, err := conn.Execute(versionQuery)
351-
if err != nil {
352-
return false
353-
}
354-
355-
if r.RowNumber() == 0 {
356-
return false
333+
// hasInvisibleIndexSupportFromColumns checks if the columns from SHOW INDEX include Visible column
334+
func hasInvisibleIndexSupportFromColumns(cols []string) bool {
335+
for _, col := range cols {
336+
if strings.EqualFold(col, "Visible") {
337+
return true
338+
}
357339
}
358-
359-
version, _ := r.GetString(0, 0)
360-
return isMySQL8OrAbove(version)
340+
return false
361341
}
362342

363-
func hasInvisibleIndexSupportSqlDB(conn *sql.DB) bool {
364-
versionQuery := "SELECT VERSION()"
365-
var version string
366-
err := conn.QueryRow(versionQuery).Scan(&version)
367-
if err != nil {
368-
return false
369-
}
370-
return isMySQL8OrAbove(version)
343+
func isIndexInvisible(value string) bool {
344+
return strings.EqualFold(value, "NO")
371345
}
372346

373347
func (ta *Table) fetchIndexes(conn mysql.Executer) error {
@@ -378,7 +352,7 @@ func (ta *Table) fetchIndexes(conn mysql.Executer) error {
378352
var currentIndex *Index
379353
currentName := ""
380354

381-
hasInvisibleIndex := hasInvisibleIndexSupport(conn)
355+
hasInvisibleIndex := hasInvisibleIndexSupportFromResult(r)
382356

383357
for i := 0; i < r.RowNumber(); i++ {
384358
indexName, _ := r.GetString(i, 2)
@@ -390,11 +364,10 @@ func (ta *Table) fetchIndexes(conn mysql.Executer) error {
390364
colName, _ := r.GetString(i, 4)
391365
currentIndex.AddColumn(colName, cardinality)
392366
currentIndex.NoneUnique, _ = r.GetUint(i, 1)
393-
394367
// Only set to false if explicitly marked as invisible
395368
if hasInvisibleIndex {
396369
visible, _ := r.GetString(i, 13)
397-
if visible == "NO" {
370+
if isIndexInvisible(visible) {
398371
currentIndex.Visible = false
399372
}
400373
}
@@ -416,8 +389,6 @@ func (ta *Table) fetchIndexesViaSqlDB(conn *sql.DB) error {
416389

417390
var unusedVal interface{}
418391

419-
hasInvisibleIndex := hasInvisibleIndexSupportSqlDB(conn)
420-
421392
for r.Next() {
422393
var indexName string
423394
var colName sql.NullString
@@ -428,6 +399,7 @@ func (ta *Table) fetchIndexesViaSqlDB(conn *sql.DB) error {
428399
if err != nil {
429400
return errors.Trace(err)
430401
}
402+
hasInvisibleIndex := hasInvisibleIndexSupportFromColumns(cols)
431403
values := make([]interface{}, len(cols))
432404
for i := 0; i < len(cols); i++ {
433405
switch i {
@@ -440,7 +412,9 @@ func (ta *Table) fetchIndexesViaSqlDB(conn *sql.DB) error {
440412
case 6:
441413
values[i] = &cardinality
442414
case 13:
443-
values[i] = &visible
415+
if hasInvisibleIndex {
416+
values[i] = &visible
417+
}
444418
default:
445419
values[i] = &unusedVal
446420
}
@@ -465,7 +439,7 @@ func (ta *Table) fetchIndexesViaSqlDB(conn *sql.DB) error {
465439
currentIndex.NoneUnique = noneUnique
466440

467441
// Only set to false if explicitly marked as invisible
468-
if hasInvisibleIndex && visible.Valid && visible.String == "NO" {
442+
if hasInvisibleIndex && visible.Valid && isIndexInvisible(visible.String) {
469443
currentIndex.Visible = false
470444
}
471445
}

‎schema/schema_test.go

Lines changed: 72 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88

99
"github.com/go-mysql-org/go-mysql/client"
1010
_ "github.com/go-mysql-org/go-mysql/driver"
11-
"github.com/go-mysql-org/go-mysql/mysql"
1211
"github.com/go-mysql-org/go-mysql/test_util"
1312
"github.com/stretchr/testify/require"
1413
"github.com/stretchr/testify/suite"
@@ -180,19 +179,7 @@ func (s *schemaTestSuite) TestSchemaWithInvisibleIndex() {
180179
_, err := s.conn.Execute(`DROP TABLE IF EXISTS invisible_idx_test`)
181180
require.NoError(s.T(), err)
182181

183-
// Check MySQL version
184-
hasInvisibleIndex := false
185-
versionQuery := "SELECT VERSION()"
186-
r, err := s.conn.Execute(versionQuery)
187-
require.NoError(s.T(), err)
188-
189-
if r.RowNumber() > 0 {
190-
version, _ := r.GetString(0, 0)
191-
if eq, err := mysql.CompareServerVersions(version, "8.0.0"); err == nil && eq >= 0 {
192-
hasInvisibleIndex = true
193-
}
194-
}
195-
182+
// Create table first to check invisible index support via column presence
196183
str := `
197184
CREATE TABLE IF NOT EXISTS invisible_idx_test (
198185
id INT,
@@ -207,7 +194,12 @@ func (s *schemaTestSuite) TestSchemaWithInvisibleIndex() {
207194
_, err = s.conn.Execute(str)
208195
require.NoError(s.T(), err)
209196

210-
// Add INVISIBLE keyword only for MySQL 8.0+
197+
// Check if invisible index support exists by checking SHOW INDEX columns
198+
r, err := s.conn.Execute(fmt.Sprintf("SHOW INDEX FROM `%s`.`%s`", *schema, "invisible_idx_test"))
199+
require.NoError(s.T(), err)
200+
hasInvisibleIndex := hasInvisibleIndexSupportFromResult(r)
201+
202+
// Add INVISIBLE keyword only if database supports it
211203
if hasInvisibleIndex {
212204
_, err = s.conn.Execute(`ALTER TABLE invisible_idx_test ALTER INDEX name_idx INVISIBLE`)
213205
require.NoError(s.T(), err)
@@ -225,9 +217,10 @@ func (s *schemaTestSuite) TestSchemaWithInvisibleIndex() {
225217
// Find name_idx and email_idx (order may vary)
226218
var nameIdx, emailIdx *Index
227219
for _, idx := range ta.Indexes {
228-
if idx.Name == "name_idx" {
220+
switch idx.Name {
221+
case "name_idx":
229222
nameIdx = idx
230-
} else if idx.Name == "email_idx" {
223+
case "email_idx":
231224
emailIdx = idx
232225
}
233226
}
@@ -238,11 +231,11 @@ func (s *schemaTestSuite) TestSchemaWithInvisibleIndex() {
238231
// email_idx should always be visible (default)
239232
require.True(s.T(), emailIdx.Visible)
240233

241-
// name_idx visibility depends on MySQL version
234+
// name_idx visibility depends on database support for invisible indexes
242235
if hasInvisibleIndex {
243-
require.False(s.T(), nameIdx.Visible, "name_idx should be invisible in MySQL 8.0+")
236+
require.False(s.T(), nameIdx.Visible, "name_idx should be invisible when database supports invisible indexes")
244237
} else {
245-
require.True(s.T(), nameIdx.Visible, "name_idx should be visible in MySQL <8.0")
238+
require.True(s.T(), nameIdx.Visible, "name_idx should be visible when database doesn't support invisible indexes")
246239
}
247240

248241
taSqlDb, err := NewTableFromSqlDB(s.sqlDB, *schema, "invisible_idx_test")
@@ -251,6 +244,65 @@ func (s *schemaTestSuite) TestSchemaWithInvisibleIndex() {
251244
require.Equal(s.T(), ta, taSqlDb)
252245
}
253246

247+
func (s *schemaTestSuite) TestInvisibleIndexColumnDetection() {
248+
_, err := s.conn.Execute(`DROP TABLE IF EXISTS column_detection_test`)
249+
require.NoError(s.T(), err)
250+
251+
str := `
252+
CREATE TABLE IF NOT EXISTS column_detection_test (
253+
id INT PRIMARY KEY,
254+
name VARCHAR(256),
255+
INDEX name_idx (name)
256+
) ENGINE = INNODB;
257+
`
258+
259+
_, err = s.conn.Execute(str)
260+
require.NoError(s.T(), err)
261+
262+
// Test both detection functions work consistently
263+
r, err := s.conn.Execute(fmt.Sprintf("SHOW INDEX FROM `%s`.`%s`", *schema, "column_detection_test"))
264+
require.NoError(s.T(), err)
265+
266+
hasInvisibleFromResult := hasInvisibleIndexSupportFromResult(r)
267+
268+
// Get columns and test the other detection function
269+
cols, err := s.sqlDB.Query(fmt.Sprintf("SHOW INDEX FROM `%s`.`%s`", *schema, "column_detection_test"))
270+
require.NoError(s.T(), err)
271+
defer cols.Close()
272+
273+
columnNames, err := cols.Columns()
274+
require.NoError(s.T(), err)
275+
hasInvisibleFromColumns := hasInvisibleIndexSupportFromColumns(columnNames)
276+
277+
// Both detection methods should agree
278+
require.Equal(s.T(), hasInvisibleFromResult, hasInvisibleFromColumns, "Detection methods should be consistent")
279+
280+
// Test that both connection types work identically
281+
ta1, err := NewTable(s.conn, *schema, "column_detection_test")
282+
require.NoError(s.T(), err)
283+
284+
ta2, err := NewTableFromSqlDB(s.sqlDB, *schema, "column_detection_test")
285+
require.NoError(s.T(), err)
286+
287+
require.Equal(s.T(), ta1, ta2, "Both connection types should produce identical results")
288+
}
289+
290+
func TestInvisibleIndexLogic(t *testing.T) {
291+
// Test MySQL-style visibility logic
292+
require.True(t, isIndexInvisible("NO"), "Visible=NO should be invisible")
293+
require.False(t, isIndexInvisible("YES"), "Visible=YES should be visible")
294+
295+
// Test case insensitivity
296+
require.True(t, isIndexInvisible("no"), "Should be case insensitive")
297+
require.True(t, isIndexInvisible("No"), "Should be case insensitive")
298+
require.False(t, isIndexInvisible("yes"), "Should be case insensitive")
299+
require.False(t, isIndexInvisible("YES"), "Should be case insensitive")
300+
301+
// Test other values default to visible
302+
require.False(t, isIndexInvisible(""), "Empty string should default to visible")
303+
require.False(t, isIndexInvisible("UNKNOWN"), "Unknown value should default to visible")
304+
}
305+
254306
func TestIndexVisibilityDefault(t *testing.T) {
255307
// Test that NewIndex creates visible indexes by default
256308
idx := NewIndex("test_index")

0 commit comments

Comments
 (0)