Schema load with multi-value index works with `NewTable`, fails with `NewTableFromSqlDB`

[shauns]

Hi!

A heads-up that I am by no means a Go developer, but am happy to get my hands dirty on this 🙏

I found that with tables using multi-value indexes, the NewTable function is able to load the table, but the NewTableFromSqlDB function is not. I'll put a concise failing test case below.

It looks like the issue is in loading the column names for the index. MySQL returns a null value for the column name, and in the NewTableFromSqlDB function, we fail with converting NULL to string is unsupported (In NewTable, it uses "").

If someone can point in the direction of what to fix, I'm happy to make a contrib. Or, let us know if there's a fundamental blocker that stops this working.

Here's the short failing test case -- the call to NewTableFromSqlDB fails.

func (s *schemaTestSuite) TestSchemaWithMultiValueIndex() {
	_, err := s.conn.Execute(`DROP TABLE IF EXISTS multi_value_idx_test`)
	require.NoError(s.T(), err)

	str := `
        CREATE TABLE IF NOT EXISTS multi_value_idx_test (
            id INT,
            entries json,
            PRIMARY KEY(id)
        ) ENGINE = INNODB;
    `

	_, err = s.conn.Execute(str)
	require.NoError(s.T(), err)

	str = `CREATE INDEX idx_entries ON multi_value_idx_test((CAST((entries->'$') AS CHAR(64))));`
	_, err = s.conn.Execute(str)
	require.NoError(s.T(), err)

	ta, err := NewTable(s.conn, *schema, "multi_value_idx_test")
	require.NoError(s.T(), err)

	require.Len(s.T(), ta.Indexes, 2)

	require.Equal(s.T(), "PRIMARY", ta.Indexes[0].Name)
	require.Len(s.T(), ta.Indexes[0].Columns, 1)
	require.Equal(s.T(), "id", ta.Indexes[0].Columns[0])

	require.Equal(s.T(), "idx_entries", ta.Indexes[1].Name)
	require.Len(s.T(), ta.Indexes[1].Columns, 1)
	require.Equal(s.T(), "", ta.Indexes[1].Columns[0])

	taSqlDb, err := NewTableFromSqlDB(s.sqlDB, *schema, "multi_value_idx_test")
	require.NoError(s.T(), err)

	require.Equal(s.T(), ta, taSqlDb)
}

[shauns]

The following fixes the test above, but I don't have the context to judge if this is appropriate or not.

diff --git a/schema/schema.go b/schema/schema.go
index 61e6531..804583c 100644
--- a/schema/schema.go
+++ b/schema/schema.go
@@ -354,7 +354,8 @@ func (ta *Table) fetchIndexesViaSqlDB(conn *sql.DB) error {
 	var unusedVal interface{}
 
 	for r.Next() {
-		var indexName, colName string
+		var indexName string
+		var colName sql.NullString
 		var noneUnique uint64
 		var cardinality interface{}
 		cols, err := r.Columns()
@@ -387,7 +388,12 @@ func (ta *Table) fetchIndexesViaSqlDB(conn *sql.DB) error {
 		}
 
 		c := toUint64(cardinality)
-		currentIndex.AddColumn(colName, c)
+		// If colName is a null string, switch to ""
+		if colName.Valid {
+			currentIndex.AddColumn(colName.String, c)
+		} else {
+			currentIndex.AddColumn("", c)
+		}
 		currentIndex.NoneUnique = noneUnique
 	}

[lance6716]

Thank you. I think your fix is correct. When the column name is NULL we must use sql.NullString to handle it.

mysql> show index from multi_value_idx_test\G
*************************** 1. row ***************************
        Table: multi_value_idx_test
   Non_unique: 0
     Key_name: PRIMARY
 Seq_in_index: 1
  Column_name: id
    Collation: A
  Cardinality: 0
     Sub_part: NULL
       Packed: NULL
         Null:
   Index_type: BTREE
      Comment:
Index_comment:
      Visible: YES
   Expression: NULL
*************************** 2. row ***************************
        Table: multi_value_idx_test
   Non_unique: 1
     Key_name: idx_entries
 Seq_in_index: 1
  Column_name: NULL
    Collation: A
  Cardinality: 0
     Sub_part: NULL
       Packed: NULL
         Null: YES
   Index_type: BTREE
      Comment:
Index_comment:
      Visible: YES
   Expression: cast(json_extract(`entries`,_utf8mb3\'$\') as char(64) charset utf8mb3)
2 rows in set (0.00 sec)

[shauns]

Great -- I'll open a PR from a forked repo shortly then.

Edit: FYI @lance6716 - opened at #904