[FSTORE-813] Unclear error when the feature group contains duplicated…

… columns (#1575)
logicalclocks · Oct 6, 2023 · eafc534 · eafc534
1 parent ada8663
commit eafc534
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 1 deletion.
diff --git a/.../java/io/hops/hopsworks/common/featurestore/featuregroup/FeatureGroupInputValidation.java b/.../java/io/hops/hopsworks/common/featurestore/featuregroup/FeatureGroupInputValidation.java
@@ -34,11 +34,13 @@
 import javax.ejb.TransactionAttributeType;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.logging.Level;
 import java.util.stream.Collectors;
 
 import static io.hops.hopsworks.restutils.RESTCodes.FeaturestoreErrorCode.COULD_NOT_CREATE_ONLINE_FEATUREGROUP;
+import static io.hops.hopsworks.restutils.RESTCodes.FeaturestoreErrorCode.FEATURE_GROUP_DUPLICATE_FEATURE;
 
 @Stateless
 @TransactionAttribute(TransactionAttributeType.NEVER)
@@ -131,6 +133,31 @@ public void verifySchemaProvided(FeaturegroupDTO featuregroupDTO) throws Feature
         "Cannot create an online feature group without a feature schema.");
     }
   }
+
+  /**
+   * Make sure all features are unique.
+   * @param featureGroupDTO
+   * @throws FeaturestoreException
+   */
+  public void verifyNoDuplicatedFeatures(FeaturegroupDTO featureGroupDTO)
+      throws FeaturestoreException {
+    List<String> duplicates = featureGroupDTO.getFeatures()
+        .stream()
+        .collect(Collectors.groupingBy(FeatureGroupFeatureDTO::getName, Collectors.counting()))
+        .entrySet()
+        .stream()
+        .filter(e -> e.getValue() > 1)
+        .map(Map.Entry::getKey)
+        .collect(Collectors.toList());
+
+    if (!duplicates.isEmpty()) {
+      throw new FeaturestoreException(
+          FEATURE_GROUP_DUPLICATE_FEATURE,
+          Level.SEVERE,
+          String.format("Cannot create feature group as there are duplicated feature names: %s",
+              StringUtils.join(duplicates, ", ")));
+    }
+  }
 
   /**
    * Make sure online and offline types match.

diff --git a/.../main/java/io/hops/hopsworks/common/featurestore/featuregroup/FeaturegroupController.java b/.../main/java/io/hops/hopsworks/common/featurestore/featuregroup/FeaturegroupController.java
@@ -202,6 +202,7 @@ public FeaturegroupDTO createFeaturegroup(Featurestore featurestore, Featuregrou
 
     enforceFeaturegroupQuotas(featurestore, featuregroupDTO);
     featureGroupInputValidation.verifySchemaProvided(featuregroupDTO);
+    featureGroupInputValidation.verifyNoDuplicatedFeatures(featuregroupDTO);
 
     // if version not provided, get latest and increment
     if (featuregroupDTO.getVersion() == null) {

diff --git a/...src/test/io/hops/hopsworks/common/featurestore/utils/TestFeatureGroupInputValidation.java b/...src/test/io/hops/hopsworks/common/featurestore/utils/TestFeatureGroupInputValidation.java
@@ -86,6 +86,24 @@ public void testverifySchemaProvided_fail() throws Exception {
     featureGroupInputValidation.verifySchemaProvided(featuregroupDTO);
   }
 
+  @Test
+  public void verifyNoDuplicatedFeatures_success() throws Exception {
+    CachedFeaturegroupDTO featuregroupDTO = new CachedFeaturegroupDTO();
+    featuregroupDTO.setFeatures(features);
+
+    featureGroupInputValidation.verifyNoDuplicatedFeatures(featuregroupDTO);
+  }
+
+  @Test(expected = FeaturestoreException.class)
+  public void verifyNoDuplicatedFeatures_fail() throws Exception {
+    CachedFeaturegroupDTO featuregroupDTO = new CachedFeaturegroupDTO();
+    List<FeatureGroupFeatureDTO> featuresWithDuplicate = new ArrayList<>(features);
+    featuresWithDuplicate.add(new FeatureGroupFeatureDTO("feature", "TIMESTAMP", "", true, false, "10", null));
+    featuregroupDTO.setFeatures(featuresWithDuplicate);
+
+    featureGroupInputValidation.verifyNoDuplicatedFeatures(featuregroupDTO);
+  }
+
   @Test(expected = FeaturestoreException.class)
   public void testVerifyFeatureOfflineTypeProvided_null() throws Exception {
     FeatureGroupFeatureDTO featureDTO = new FeatureGroupFeatureDTO("feature_name", null);

diff --git a/hopsworks-rest-utils/src/main/java/io/hops/hopsworks/restutils/RESTCodes.java b/hopsworks-rest-utils/src/main/java/io/hops/hopsworks/restutils/RESTCodes.java
@@ -1675,7 +1675,8 @@ public enum FeaturestoreErrorCode implements RESTErrorCode {
         "An error occurred while constructing validation report directory path",
         Response.Status.INTERNAL_SERVER_ERROR),
     SPINE_GROUP_ON_RIGHT_SIDE_OF_JOIN_NOT_ALLOWED(223, "Spine groups cannot be used on the right side" +
-      "of a feature view join.", Response.Status.BAD_REQUEST);
+      "of a feature view join.", Response.Status.BAD_REQUEST),
+    FEATURE_GROUP_DUPLICATE_FEATURE(224, "Feature list contains duplicate", Response.Status.BAD_REQUEST);
 
     private int code;
     private String message;