[#5533] fix (trino-connector): Fix the exception of ArrayIndexOutOfBo…

…undsException when execute COMMENT COLUMN command (#6201)

### What changes were proposed in this pull request?

Fix the exception of ArrayIndexOutOfBoundsException when handle error
message of IllegalArgumentException

### Why are the changes needed?

Fix: #5533 

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

IT

Co-authored-by: Yuhui <[email protected]>

trino-connector/integration-test/src/test/java/org/apache/gravitino/trino/connector/integration/test/TrinoQueryIT.java

@@ -55,15 +55,15 @@
 public class TrinoQueryIT extends TrinoQueryITBase {
   private static final Logger LOG = LoggerFactory.getLogger(TrinoQueryIT.class);
 
-  static String testsetsDir = "";
-  AtomicInteger passCount = new AtomicInteger(0);
-  AtomicInteger totalCount = new AtomicInteger(0);
-  static boolean exitOnFailed = true;
+  protected static String testsetsDir;
+  protected AtomicInteger passCount = new AtomicInteger(0);
+  protected AtomicInteger totalCount = new AtomicInteger(0);
+  protected static boolean exitOnFailed = true;
 
   // key: tester name, value: tester result
-  private static Map<String, TestStatus> allTestStatus = new TreeMap<>();
+  private static final Map<String, TestStatus> allTestStatus = new TreeMap<>();
 
-  private static int testParallelism = 2;
+  private static final int testParallelism = 2;
 
   static Map<String, String> queryParams = new HashMap<>();
 
@@ -275,8 +275,8 @@ void executeSqlFileWithCheckResult(
    * actual result matches the query failed result. 3. The expected result is a regular expression,
    * and the actual result matches the regular expression.
    *
-   * @param expectResult
-   * @param result
+   * @param expectResult the expected result
+   * @param result the actual result
    * @return false if the expected result is empty or the actual result does not match the expected.
    *     For {@literal <BLANK_LINE>} case, return true if the actual result is empty. For {@literal
    *     <QUERY_FAILED>} case, replace the placeholder with "^Query \\w+ failed.*: " and do match.
@@ -338,7 +338,7 @@ static boolean match(String expectResult, String result) {
   @Test
   public void testSql() throws Exception {
     ExecutorService executor = Executors.newFixedThreadPool(testParallelism);
-    CompletionService completionService = new ExecutorCompletionService<>(executor);
+    CompletionService<Integer> completionService = new ExecutorCompletionService<>(executor);
 
     String[] testSetNames =
         Arrays.stream(TrinoQueryITBase.listDirectory(testsetsDir))
@@ -357,7 +357,7 @@ public void testSql() throws Exception {
 
   public void testSql(String testSetDirName, String catalog, String testerPrefix) throws Exception {
     ExecutorService executor = Executors.newFixedThreadPool(testParallelism);
-    CompletionService completionService = new ExecutorCompletionService<>(executor);
+    CompletionService<Integer> completionService = new ExecutorCompletionService<>(executor);
 
     totalCount.addAndGet(getTesterCount(testSetDirName, catalog, testerPrefix));
     List<Future<Integer>> futures =
@@ -369,7 +369,7 @@ public void testSql(String testSetDirName, String catalog, String testerPrefix)
 
   private void waitForCompleted(
       ExecutorService executor,
-      CompletionService completionService,
+      CompletionService<Integer> completionService,
       List<Future<Integer>> allFutures) {
     for (int i = 0; i < allFutures.size(); i++) {
       try {
@@ -405,7 +405,7 @@ public String generateTestStatus() {
   }
 
   public List<Future<Integer>> runOneTestset(
-      CompletionService completionService,
+      CompletionService<Integer> completionService,
       String testSetDirName,
       String catalog,
       String testerFilter)

trino-connector/integration-test/src/test/java/org/apache/gravitino/trino/connector/integration/test/TrinoQueryRunner.java

@@ -42,9 +42,9 @@
 class TrinoQueryRunner {
   private static final Logger LOG = LoggerFactory.getLogger(TrinoQueryRunner.class);
 
-  private QueryRunner queryRunner;
-  private Terminal terminal;
-  private URI uri;
+  private final QueryRunner queryRunner;
+  private final Terminal terminal;
+  private final URI uri;
 
   TrinoQueryRunner(String trinoUri) throws Exception {
     this.uri = new URI(trinoUri);
@@ -92,10 +92,11 @@ String runQuery(String query) {
   String runQueryOnce(String query) {
     Query queryResult = queryRunner.startQuery(query);
     StringOutputStream outputStream = new StringOutputStream();
+    StringOutputStream errorStream = new StringOutputStream();
     queryResult.renderOutput(
         this.terminal,
         new PrintStream(outputStream),
-        new PrintStream(outputStream),
+        new PrintStream(errorStream),
         CSV,
         Optional.of(""),
         false);
@@ -109,17 +110,19 @@ String runQueryOnce(String query) {
       session = builder.build();
       queryRunner.setSession(session);
     }
-    return outputStream.toString();
+
+    // Avoid the IDE capturing the error message as failure
+    String err_message = errorStream.toString().replace("\nCaused by:", "\n-Caused by:");
+    String out_message = outputStream.toString();
+    return err_message + out_message;
   }
 
-  boolean stop() {
+  void stop() {
     try {
       queryRunner.close();
       terminal.close();
-      return true;
     } catch (Exception e) {
       LOG.error("Failed to stop query runner", e);
-      return false;
     }
   }
 }

trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00002_alter_table.sql

@@ -37,6 +37,8 @@ show create table gt_mysql.gt_db1.tb01;
 alter table gt_mysql.gt_db1.tb01 add column address varchar(200) not null comment 'address of users';
 show create table gt_mysql.gt_db1.tb01;
 
+COMMENT ON COLUMN gt_mysql.gt_db1.tb01.city IS NULL;
+
 drop table gt_mysql.gt_db1.tb01;
 
 drop schema gt_mysql.gt_db1;

trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00002_alter_table.txt

@@ -104,6 +104,8 @@ WITH (
    engine = 'InnoDB'
 )"
 
+<QUERY_FAILED> "newComment" field is required and cannot be empty
+
 DROP TABLE
 
 DROP SCHEMA

trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoErrorCode.java

@@ -23,6 +23,7 @@
 import io.trino.spi.ErrorCode;
 import io.trino.spi.ErrorCodeSupplier;
 import io.trino.spi.ErrorType;
+import java.util.List;
 
 public enum GravitinoErrorCode implements ErrorCodeSupplier {
   GRAVITINO_UNSUPPORTED_TRINO_VERSION(0, EXTERNAL),
@@ -64,4 +65,9 @@ public enum GravitinoErrorCode implements ErrorCodeSupplier {
   public ErrorCode toErrorCode() {
     return errorCode;
   }
+
+  public static String toSimpleErrorMessage(Exception e) {
+    List<String> lines = e.getMessage().lines().toList();
+    return lines.size() > 1 ? lines.get(0) + lines.get(1) : lines.get(0);
+  }
 }

trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java

@@ -190,8 +190,7 @@ private void applyAlter(SchemaTableName tableName, TableChange... change) {
       // TODO yuhui need improve get the error message. From IllegalArgumentException.
       // At present, the IllegalArgumentException cannot get the error information clearly from the
       // Gravitino server.
-      String message =
-          e.getMessage().lines().toList().get(0) + e.getMessage().lines().toList().get(1);
+      String message = GravitinoErrorCode.toSimpleErrorMessage(e);
       throw new TrinoException(GravitinoErrorCode.GRAVITINO_ILLEGAL_ARGUMENT, message, e);
     }
   }