[#2227] improvement(jdbc-backend): Improve the judgment of exception information in JDBC backend (#2862)

xloya · xiaojiebao · web-flow · commit 9128db0d2f4b · 2024-04-14T12:30:27.000+08:00
### What changes were proposed in this pull request? Determine exceptions more accurately based on SQL Exception error codes. ### Why are the changes needed? Fix: #2227 ### How was this patch tested? Add the unit tests. --------- Co-authored-by: xiaojiebao <xiaojiebao@xiaomi.com>
diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/JDBCBackend.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/JDBCBackend.java
@@ -21,6 +21,7 @@
 import com.datastrato.gravitino.meta.SchemaEntity;
 import com.datastrato.gravitino.meta.TableEntity;
 import com.datastrato.gravitino.meta.TopicEntity;
+import com.datastrato.gravitino.storage.relational.converters.SQLExceptionConverterFactory;
 import com.datastrato.gravitino.storage.relational.service.CatalogMetaService;
 import com.datastrato.gravitino.storage.relational.service.FilesetMetaService;
 import com.datastrato.gravitino.storage.relational.service.MetalakeMetaService;
@@ -44,6 +45,7 @@ public class JDBCBackend implements RelationalBackend {
   @Override
   public void initialize(Config config) {
     SqlSessionFactoryHelper.getInstance().init(config);
+    SQLExceptionConverterFactory.initConverter(config);
   }
 
   @Override
diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/converters/H2ExceptionConverter.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/converters/H2ExceptionConverter.java
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2024 Datastrato Pvt Ltd.
+ * This software is licensed under the Apache License version 2.
+ */
+package com.datastrato.gravitino.storage.relational.converters;
+
+import com.datastrato.gravitino.Entity;
+import com.datastrato.gravitino.exceptions.AlreadyExistsException;
+import com.datastrato.gravitino.exceptions.GravitinoRuntimeException;
+import java.sql.SQLException;
+
+/**
+ * Exception converter to Gravitino exception for H2. The definition of error codes can be found in
+ * the document: <a href="https://h2database.com/javadoc/org/h2/api/ErrorCode.html"></a>
+ */
+public class H2ExceptionConverter implements SQLExceptionConverter {
+  /** It means found a duplicated primary key or unique key entry in H2. */
+  private static final int DUPLICATED_ENTRY_ERROR_CODE = 23505;
+
+  @SuppressWarnings("FormatStringAnnotation")
+  @Override
+  public GravitinoRuntimeException toGravitinoException(
+      SQLException se, Entity.EntityType type, String name) {
+    switch (se.getErrorCode()) {
+      case DUPLICATED_ENTRY_ERROR_CODE:
+        return new AlreadyExistsException(se, se.getMessage());
+      default:
+        return new GravitinoRuntimeException(se, se.getMessage());
+    }
+  }
+}
diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/converters/MySQLExceptionConverter.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/converters/MySQLExceptionConverter.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2024 Datastrato Pvt Ltd.
+ * This software is licensed under the Apache License version 2.
+ */
+package com.datastrato.gravitino.storage.relational.converters;
+
+import com.datastrato.gravitino.Entity;
+import com.datastrato.gravitino.exceptions.AlreadyExistsException;
+import com.datastrato.gravitino.exceptions.GravitinoRuntimeException;
+import java.sql.SQLException;
+
+/**
+ * Exception converter to Gravitino exception for MySQL. The definition of error codes can be found
+ * in the document: <a
+ * href="https://dev.mysql.com/doc/connector-j/en/connector-j-reference-error-sqlstates.html"></a>
+ */
+public class MySQLExceptionConverter implements SQLExceptionConverter {
+  /** It means found a duplicated primary key or unique key entry in MySQL. */
+  private static final int DUPLICATED_ENTRY_ERROR_CODE = 1062;
+
+  @SuppressWarnings("FormatStringAnnotation")
+  @Override
+  public GravitinoRuntimeException toGravitinoException(
+      SQLException se, Entity.EntityType type, String name) {
+    switch (se.getErrorCode()) {
+      case DUPLICATED_ENTRY_ERROR_CODE:
+        return new AlreadyExistsException(se, se.getMessage());
+      default:
+        return new GravitinoRuntimeException(se, se.getMessage());
+    }
+  }
+}
diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/converters/SQLExceptionConverter.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/converters/SQLExceptionConverter.java
@@ -0,0 +1,24 @@
+/*
+ * Copyright 2024 Datastrato Pvt Ltd.
+ * This software is licensed under the Apache License version 2.
+ */
+package com.datastrato.gravitino.storage.relational.converters;
+
+import com.datastrato.gravitino.Entity;
+import com.datastrato.gravitino.exceptions.GravitinoRuntimeException;
+import java.sql.SQLException;
+
+/** Interface for converter JDBC SQL exceptions to Gravitino exceptions. */
+public interface SQLExceptionConverter {
+  /**
+   * Convert JDBC exception to GravitinoException.
+   *
+   * @param sqlException The sql exception to map
+   * @param type The type of the entity
+   * @param name The name of the entity
+   * @return A best attempt at a corresponding jdbc connector exception or generic with the
+   *     SQLException as the cause
+   */
+  GravitinoRuntimeException toGravitinoException(
+      SQLException sqlException, Entity.EntityType type, String name);
+}
diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/converters/SQLExceptionConverterFactory.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/converters/SQLExceptionConverterFactory.java
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2024 Datastrato Pvt Ltd.
+ * This software is licensed under the Apache License version 2.
+ */
+package com.datastrato.gravitino.storage.relational.converters;
+
+import com.datastrato.gravitino.Config;
+import com.datastrato.gravitino.Configs;
+import com.google.common.base.Preconditions;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+public class SQLExceptionConverterFactory {
+  private static final Pattern TYPE_PATTERN = Pattern.compile("jdbc:(\\w+):");
+  private static SQLExceptionConverter converter;
+
+  private SQLExceptionConverterFactory() {}
+
+  public static synchronized void initConverter(Config config) {
+    if (converter == null) {
+      String jdbcUrl = config.get(Configs.ENTITY_RELATIONAL_JDBC_BACKEND_URL);
+      Matcher typeMatcher = TYPE_PATTERN.matcher(jdbcUrl);
+      if (typeMatcher.find()) {
+        String jdbcType = typeMatcher.group(1);
+        if (jdbcType.equalsIgnoreCase("mysql")) {
+          converter = new MySQLExceptionConverter();
+        } else if (jdbcType.equalsIgnoreCase("h2")) {
+          converter = new H2ExceptionConverter();
+        } else {
+          throw new IllegalArgumentException(String.format("Unsupported jdbc type: %s", jdbcType));
+        }
+      } else {
+        throw new IllegalArgumentException(
+            String.format("Cannot find jdbc type in jdbc url: %s", jdbcUrl));
+      }
+    }
+  }
+
+  public static SQLExceptionConverter getConverter() {
+    Preconditions.checkState(converter != null, "Exception converter is not initialized.");
+    return converter;
+  }
+}
diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/service/CatalogMetaService.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/service/CatalogMetaService.java
@@ -112,7 +112,7 @@ public void insertCatalog(CatalogEntity catalogEntity, boolean overwrite) {
             }
           });
     } catch (RuntimeException re) {
-      ExceptionUtils.checkSQLConstraintException(
+      ExceptionUtils.checkSQLException(
           re, Entity.EntityType.CATALOG, catalogEntity.nameIdentifier().toString());
       throw re;
     }
@@ -147,7 +147,7 @@ public <E extends Entity & HasIdentifier> CatalogEntity updateCatalog(
                       POConverters.updateCatalogPOWithVersion(oldCatalogPO, newEntity, metalakeId),
                       oldCatalogPO));
     } catch (RuntimeException re) {
-      ExceptionUtils.checkSQLConstraintException(
+      ExceptionUtils.checkSQLException(
           re, Entity.EntityType.CATALOG, newEntity.nameIdentifier().toString());
       throw re;
     }
diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/service/FilesetMetaService.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/service/FilesetMetaService.java
@@ -122,7 +122,7 @@ public void insertFileset(FilesetEntity filesetEntity, boolean overwrite) {
                     }
                   }));
     } catch (RuntimeException re) {
-      ExceptionUtils.checkSQLConstraintException(
+      ExceptionUtils.checkSQLException(
           re, Entity.EntityType.FILESET, filesetEntity.nameIdentifier().toString());
       throw re;
     }
@@ -177,7 +177,7 @@ public <E extends Entity & HasIdentifier> FilesetEntity updateFileset(
                 mapper -> mapper.updateFilesetMeta(newFilesetPO, oldFilesetPO));
       }
     } catch (RuntimeException re) {
-      ExceptionUtils.checkSQLConstraintException(
+      ExceptionUtils.checkSQLException(
           re, Entity.EntityType.FILESET, newEntity.nameIdentifier().toString());
       throw re;
     }
diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/service/MetalakeMetaService.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/service/MetalakeMetaService.java
@@ -90,7 +90,7 @@ public void insertMetalake(BaseMetalake baseMetalake, boolean overwrite) {
             }
           });
     } catch (RuntimeException re) {
-      ExceptionUtils.checkSQLConstraintException(
+      ExceptionUtils.checkSQLException(
           re, Entity.EntityType.METALAKE, baseMetalake.nameIdentifier().toString());
       throw re;
     }
@@ -125,7 +125,7 @@ public <E extends Entity & HasIdentifier> BaseMetalake updateMetalake(
               MetalakeMetaMapper.class,
               mapper -> mapper.updateMetalakeMeta(newMetalakePO, oldMetalakePO));
     } catch (RuntimeException re) {
-      ExceptionUtils.checkSQLConstraintException(
+      ExceptionUtils.checkSQLException(
           re, Entity.EntityType.METALAKE, newMetalakeEntity.nameIdentifier().toString());
       throw re;
     }
diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/service/SchemaMetaService.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/service/SchemaMetaService.java
@@ -109,7 +109,7 @@ public void insertSchema(SchemaEntity schemaEntity, boolean overwrite) {
             }
           });
     } catch (RuntimeException re) {
-      ExceptionUtils.checkSQLConstraintException(
+      ExceptionUtils.checkSQLException(
           re, Entity.EntityType.SCHEMA, schemaEntity.nameIdentifier().toString());
       throw re;
     }
@@ -142,7 +142,7 @@ public <E extends Entity & HasIdentifier> SchemaEntity updateSchema(
                   mapper.updateSchemaMeta(
                       POConverters.updateSchemaPOWithVersion(oldSchemaPO, newEntity), oldSchemaPO));
     } catch (RuntimeException re) {
-      ExceptionUtils.checkSQLConstraintException(
+      ExceptionUtils.checkSQLException(
           re, Entity.EntityType.SCHEMA, newEntity.nameIdentifier().toString());
       throw re;
     }
diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/service/TableMetaService.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/service/TableMetaService.java
@@ -102,7 +102,7 @@ public void insertTable(TableEntity tableEntity, boolean overwrite) {
             }
           });
     } catch (RuntimeException re) {
-      ExceptionUtils.checkSQLConstraintException(
+      ExceptionUtils.checkSQLException(
           re, Entity.EntityType.TABLE, tableEntity.nameIdentifier().toString());
       throw re;
     }
@@ -135,7 +135,7 @@ public <E extends Entity & HasIdentifier> TableEntity updateTable(
                   mapper.updateTableMeta(
                       POConverters.updateTablePOWithVersion(oldTablePO, newEntity), oldTablePO));
     } catch (RuntimeException re) {
-      ExceptionUtils.checkSQLConstraintException(
+      ExceptionUtils.checkSQLException(
           re, Entity.EntityType.TABLE, newEntity.nameIdentifier().toString());
       throw re;
     }
diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/service/TopicMetaService.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/service/TopicMetaService.java
@@ -53,7 +53,7 @@ public void insertTopic(TopicEntity topicEntity, boolean overwrite) {
           });
       // TODO: insert topic dataLayout version after supporting it
     } catch (RuntimeException re) {
-      ExceptionUtils.checkSQLConstraintException(
+      ExceptionUtils.checkSQLException(
           re, Entity.EntityType.TOPIC, topicEntity.nameIdentifier().toString());
       throw re;
     }
@@ -97,7 +97,7 @@ public <E extends Entity & HasIdentifier> TopicEntity updateTopic(
                   mapper.updateTopicMeta(
                       POConverters.updateTopicPOWithVersion(oldTopicPO, newEntity), oldTopicPO));
     } catch (RuntimeException re) {
-      ExceptionUtils.checkSQLConstraintException(
+      ExceptionUtils.checkSQLException(
           re, Entity.EntityType.TOPIC, newEntity.nameIdentifier().toString());
       throw re;
     }
diff --git a/core/src/main/java/com/datastrato/gravitino/storage/relational/utils/ExceptionUtils.java b/core/src/main/java/com/datastrato/gravitino/storage/relational/utils/ExceptionUtils.java
@@ -5,21 +5,17 @@
 package com.datastrato.gravitino.storage.relational.utils;
 
 import com.datastrato.gravitino.Entity;
-import com.datastrato.gravitino.exceptions.AlreadyExistsException;
-import java.sql.SQLIntegrityConstraintViolationException;
+import com.datastrato.gravitino.storage.relational.converters.SQLExceptionConverterFactory;
+import java.sql.SQLException;
 
 public class ExceptionUtils {
   private ExceptionUtils() {}
 
-  public static void checkSQLConstraintException(
+  public static void checkSQLException(
       RuntimeException re, Entity.EntityType type, String entityName) {
-    if (re.getCause() != null
-        && re.getCause() instanceof SQLIntegrityConstraintViolationException) {
-      // TODO We should make more fine-grained exception judgments
-      //  Usually throwing `SQLIntegrityConstraintViolationException` means that
-      //  SQL violates the constraints of `primary key` and `unique key`.
-      //  We simply think that the entity already exists at this time.
-      throw new AlreadyExistsException("%s entity: %s already exists", type.name(), entityName);
+    if (re.getCause() instanceof SQLException) {
+      throw SQLExceptionConverterFactory.getConverter()
+          .toGravitinoException((SQLException) re.getCause(), type, entityName);
     }
   }
 }
diff --git a/core/src/test/java/com/datastrato/gravitino/storage/relational/TestJDBCBackend.java b/core/src/test/java/com/datastrato/gravitino/storage/relational/TestJDBCBackend.java

Original file line number	Diff line number	Diff line change
`@@ -112,7 +112,7 @@ public void insertCatalog(CatalogEntity catalogEntity, boolean overwrite) {`
`112`	`112`	`}`
`113`	`113`	`});`
`114`	`114`	`} catch (RuntimeException re) {`
`115`		`- ExceptionUtils.checkSQLConstraintException(`
	`115`	`+ ExceptionUtils.checkSQLException(`
`116`	`116`	`re, Entity.EntityType.CATALOG, catalogEntity.nameIdentifier().toString());`
`117`	`117`	`throw re;`
`118`	`118`	`}`
`@@ -147,7 +147,7 @@ public <E extends Entity & HasIdentifier> CatalogEntity updateCatalog(`
`147`	`147`	`POConverters.updateCatalogPOWithVersion(oldCatalogPO, newEntity, metalakeId),`
`148`	`148`	`oldCatalogPO));`
`149`	`149`	`} catch (RuntimeException re) {`
`150`		`- ExceptionUtils.checkSQLConstraintException(`
	`150`	`+ ExceptionUtils.checkSQLException(`
`151`	`151`	`re, Entity.EntityType.CATALOG, newEntity.nameIdentifier().toString());`
`152`	`152`	`throw re;`
`153`	`153`	`}`
Original file line number	Diff line number	Diff line change
`@@ -122,7 +122,7 @@ public void insertFileset(FilesetEntity filesetEntity, boolean overwrite) {`
`122`	`122`	`}`
`123`	`123`	`}));`
`124`	`124`	`} catch (RuntimeException re) {`
`125`		`- ExceptionUtils.checkSQLConstraintException(`
	`125`	`+ ExceptionUtils.checkSQLException(`
`126`	`126`	`re, Entity.EntityType.FILESET, filesetEntity.nameIdentifier().toString());`
`127`	`127`	`throw re;`
`128`	`128`	`}`
`@@ -177,7 +177,7 @@ public <E extends Entity & HasIdentifier> FilesetEntity updateFileset(`
`177`	`177`	`mapper -> mapper.updateFilesetMeta(newFilesetPO, oldFilesetPO));`
`178`	`178`	`}`
`179`	`179`	`} catch (RuntimeException re) {`
`180`		`- ExceptionUtils.checkSQLConstraintException(`
	`180`	`+ ExceptionUtils.checkSQLException(`
`181`	`181`	`re, Entity.EntityType.FILESET, newEntity.nameIdentifier().toString());`
`182`	`182`	`throw re;`
`183`	`183`	`}`
Original file line number	Diff line number	Diff line change
`@@ -90,7 +90,7 @@ public void insertMetalake(BaseMetalake baseMetalake, boolean overwrite) {`
`90`	`90`	`}`
`91`	`91`	`});`
`92`	`92`	`} catch (RuntimeException re) {`
`93`		`- ExceptionUtils.checkSQLConstraintException(`
	`93`	`+ ExceptionUtils.checkSQLException(`
`94`	`94`	`re, Entity.EntityType.METALAKE, baseMetalake.nameIdentifier().toString());`
`95`	`95`	`throw re;`
`96`	`96`	`}`
`@@ -125,7 +125,7 @@ public <E extends Entity & HasIdentifier> BaseMetalake updateMetalake(`
`125`	`125`	`MetalakeMetaMapper.class,`
`126`	`126`	`mapper -> mapper.updateMetalakeMeta(newMetalakePO, oldMetalakePO));`
`127`	`127`	`} catch (RuntimeException re) {`
`128`		`- ExceptionUtils.checkSQLConstraintException(`
	`128`	`+ ExceptionUtils.checkSQLException(`
`129`	`129`	`re, Entity.EntityType.METALAKE, newMetalakeEntity.nameIdentifier().toString());`
`130`	`130`	`throw re;`
`131`	`131`	`}`
Original file line number	Diff line number	Diff line change
`@@ -109,7 +109,7 @@ public void insertSchema(SchemaEntity schemaEntity, boolean overwrite) {`
`109`	`109`	`}`
`110`	`110`	`});`
`111`	`111`	`} catch (RuntimeException re) {`
`112`		`- ExceptionUtils.checkSQLConstraintException(`
	`112`	`+ ExceptionUtils.checkSQLException(`
`113`	`113`	`re, Entity.EntityType.SCHEMA, schemaEntity.nameIdentifier().toString());`
`114`	`114`	`throw re;`
`115`	`115`	`}`
`@@ -142,7 +142,7 @@ public <E extends Entity & HasIdentifier> SchemaEntity updateSchema(`
`142`	`142`	`mapper.updateSchemaMeta(`
`143`	`143`	`POConverters.updateSchemaPOWithVersion(oldSchemaPO, newEntity), oldSchemaPO));`
`144`	`144`	`} catch (RuntimeException re) {`
`145`		`- ExceptionUtils.checkSQLConstraintException(`
	`145`	`+ ExceptionUtils.checkSQLException(`
`146`	`146`	`re, Entity.EntityType.SCHEMA, newEntity.nameIdentifier().toString());`
`147`	`147`	`throw re;`
`148`	`148`	`}`
Original file line number	Diff line number	Diff line change
`@@ -102,7 +102,7 @@ public void insertTable(TableEntity tableEntity, boolean overwrite) {`
`102`	`102`	`}`
`103`	`103`	`});`
`104`	`104`	`} catch (RuntimeException re) {`
`105`		`- ExceptionUtils.checkSQLConstraintException(`
	`105`	`+ ExceptionUtils.checkSQLException(`
`106`	`106`	`re, Entity.EntityType.TABLE, tableEntity.nameIdentifier().toString());`
`107`	`107`	`throw re;`
`108`	`108`	`}`
`@@ -135,7 +135,7 @@ public <E extends Entity & HasIdentifier> TableEntity updateTable(`
`135`	`135`	`mapper.updateTableMeta(`
`136`	`136`	`POConverters.updateTablePOWithVersion(oldTablePO, newEntity), oldTablePO));`
`137`	`137`	`} catch (RuntimeException re) {`
`138`		`- ExceptionUtils.checkSQLConstraintException(`
	`138`	`+ ExceptionUtils.checkSQLException(`
`139`	`139`	`re, Entity.EntityType.TABLE, newEntity.nameIdentifier().toString());`
`140`	`140`	`throw re;`
`141`	`141`	`}`