kfaraz commented on code in PR #14271:
URL: https://github.com/apache/druid/pull/14271#discussion_r1214419693


##########
server/src/main/java/org/apache/druid/metadata/SQLMetadataStorageActionHandler.java:
##########
@@ -165,34 +166,61 @@ public void insert(
   {
     try {
       getConnector().retryWithHandle(
-          (HandleCallback<Void>) handle -> {
-            final String sql = StringUtils.format(
-                "INSERT INTO %s (id, created_date, datasource, payload, type, 
group_id, active, status_payload) "
-                + "VALUES (:id, :created_date, :datasource, :payload, :type, 
:group_id, :active, :status_payload)",
-                getEntryTable()
-            );
-            handle.createStatement(sql)
-                  .bind("id", id)
-                  .bind("created_date", timestamp.toString())
-                  .bind("datasource", dataSource)
-                  .bind("payload", jsonMapper.writeValueAsBytes(entry))
-                  .bind("type", type)
-                  .bind("group_id", groupId)
-                  .bind("active", active)
-                  .bind("status_payload", jsonMapper.writeValueAsBytes(status))
-                  .execute();
-            return null;
-          },
-          e -> getConnector().isTransientException(e) && 
!(isStatementException(e) && getEntry(id).isPresent())
+          handle -> insertEntryWithHandle(handle, id, timestamp, dataSource, 
entry, active, status, type, groupId),
+          this::isTransientDruidException
       );
     }
+    catch (CallbackFailedException e) {
+      propageAsRuntimeException(e.getCause());
+    }
     catch (Exception e) {
-      if (isStatementException(e) && getEntry(id).isPresent()) {
-        throw new EntryExistsException(id, e);
-      } else {
-        Throwables.propagateIfPossible(e);
-        throw new RuntimeException(e);
-      }
+      propageAsRuntimeException(e);
+    }
+  }
+
+  private void propageAsRuntimeException(Throwable t)
+  {
+    Throwables.propagateIfPossible(t);
+    throw new RuntimeException(t);
+  }
+
+  /**
+   * Inserts the given entry into the metadata store. Any exception thrown by a
+   * HandleCallback is wrapped in a {@link DruidException} wrapped in a
+   * {@link CallbackFailedException}.
+   */
+  private int insertEntryWithHandle(

Review Comment:
   Yeah, either way is fine. I chose `int` because metadata operations 
typically return an integer value representing the number of updated rows. But 
I guess we can use `Void` as we don't need to use the int value here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to