[SPARK-39639][SQL] Fix possible null pointer in MySQLDialect listIndexes

[panbingkun]

What changes were proposed in this pull request?

Fix possible null pointer in MySQLDialect listIndexes

Why are the changes needed?

Code as follow:
https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/jdbc/MySQLDialect.scala#L203~L232

when:

val indexComment = rs.getString("Index_comment")

return null

FYI:

String ResultSet.getString(String columnLabel) throws SQLException;
return the column value; if the value is SQL NULL, the value returned is null

then:

if (indexComment.nonEmpty) properties.put("COMMENT", indexComment)

throw NullPointerException

finally:

MySQLDialect.listIndexes

return incorrect result(ignore exist index).

Does this PR introduce any user-facing change?

No

How was this patch tested?

Update exist UT & Pass GA

[LuciferYang]

seems need this protection, Is it possible to add a UT?

[panbingkun]

seems need this protection, Is it possible to add a UT?

Updated exist UT: org.apache.spark.sql.jdbc.v2.MySQLIntegrationSuite

[info] MySQLIntegrationSuite:
[info] - SPARK-33034: ALTER TABLE ... add new columns (146 milliseconds)
[info] - SPARK-33034: ALTER TABLE ... drop column (129 milliseconds)
[info] - SPARK-33034: ALTER TABLE ... update column type (79 milliseconds)
[info] - SPARK-33034: ALTER TABLE ... rename column (52 milliseconds)
[info] - SPARK-33034: ALTER TABLE ... update column nullability (52 milliseconds)
[info] - CREATE TABLE with table comment (39 milliseconds)
[info] - CREATE TABLE with table property (46 milliseconds)
[info] - SPARK-36895: Test INDEX Using SQL (186 milliseconds)
[info] - SPARK-37038: Test TABLESAMPLE (0 milliseconds)
[info] - scan with aggregate push-down: VAR_POP with distinct: false (156 milliseconds)
[info] - scan with aggregate push-down: VAR_SAMP with distinct: false (116 milliseconds)
[info] - scan with aggregate push-down: STDDEV_POP with distinct: false (190 milliseconds)
[info] - scan with aggregate push-down: STDDEV_SAMP with distinct: false (132 milliseconds)

[AmplabJenkins]

Can one of the admins verify this patch?

[huaxingao]

Seems to me if Index_comment is not set, MySQL defaults it to empty String?

[panbingkun]

Yes, you are right! I investigated it carefully.
INDEX_COMMENT nullability is NO, the default value is empty String.

mysql> desc INFORMATION_SCHEMA.STATISTICS;
+---------------+---------------+------+-----+---------+-------+
| Field | Type | Null | Key | Default | Extra |
+---------------+---------------+------+-----+---------+-------+
| TABLE_CATALOG | varchar(512) | NO | | | |
| TABLE_SCHEMA | varchar(64) | NO | | | |
| TABLE_NAME | varchar(64) | NO | | | |
| NON_UNIQUE | bigint(1) | NO | | 0 | |
| INDEX_SCHEMA | varchar(64) | NO | | | |
| INDEX_NAME | varchar(64) | NO | | | |
| SEQ_IN_INDEX | bigint(2) | NO | | 0 | |
| COLUMN_NAME | varchar(64) | NO | | | |
| COLLATION | varchar(1) | YES | | NULL | |
| CARDINALITY | bigint(21) | YES | | NULL | |
| SUB_PART | bigint(3) | YES | | NULL | |
| PACKED | varchar(10) | YES | | NULL | |
| NULLABLE | varchar(3) | NO | | | |
| INDEX_TYPE | varchar(16) | NO | | | |
| COMMENT | varchar(16) | YES | | NULL | |
| INDEX_COMMENT | varchar(1024) | NO | | | |
+---------------+---------------+------+-----+---------+-------+

[panbingkun]

I will close it, but maybe we need add some UT for it.I will open new pr for it.
FYI: Whether it is necessary to implement createIndex & existIndex & dropIndex & listIndex in H2Dialect ?

I found the above "possible null pointer" when implementing the method: listIndex in H2Dialect(the default value is null instead of empty string). @huaxingao

[panbingkun]

Updated:

new UT for it: #37060

[huaxingao]

maybe we need add some UT for it

Sounds good to me. I will take a look at the new PR.

Whether it is necessary to implement createIndex & existIndex & dropIndex & listIndex in H2Dialect ?

It's OK to implement these.

[panbingkun]

@huaxingao implement for h2 dialect: #37112

[srowen]

@panbingkun can you resolve the conflict?

[panbingkun]

@panbingkun can you resolve the conflict?

I will close the PR，reason:
MySQL defaults it to empty String.

[dongjoon-hyun]

Thank you for closing, @panbingkun .