Don't allow NaN as default value for Float and Double in Schemas (#11661

)
apache · Sep 26, 2023 · 1b5de10 · 1b5de10
1 parent b7cda01
commit 1b5de10
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 0 deletions.
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/util/SchemaUtilsTest.java b/pinot-core/src/test/java/org/apache/pinot/core/util/SchemaUtilsTest.java
@@ -161,6 +161,32 @@ public void testCompatibilityWithTableConfig() {
         .addDateTime(TIME_COLUMN, DataType.LONG, "1:MILLISECONDS:EPOCH", "1:HOURS")
         .addSingleValueDimension("colA", DataType.STRING).build();
     SchemaUtils.validate(schema, Lists.newArrayList(tableConfig));
+
+    schema = new Schema.SchemaBuilder().setSchemaName(TABLE_NAME).addMetric("double", DataType.DOUBLE, "NaN").build();
+    tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME).build();
+    try {
+      SchemaUtils.validate(schema, Lists.newArrayList(tableConfig));
+      Assert.fail("Should fail schema validation, as double has NaN default value");
+    } catch (IllegalStateException e) {
+      // expected
+      Assert.assertTrue(e.getMessage().startsWith("NaN as null default value is not managed yet for"));
+    }
+
+    schema = new Schema.SchemaBuilder().setSchemaName(TABLE_NAME).addMetric("float", DataType.FLOAT, "NaN").build();
+    tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME).build();
+    try {
+      SchemaUtils.validate(schema, Lists.newArrayList(tableConfig));
+      Assert.fail("Should fail schema validation, as float has NaN default value");
+    } catch (IllegalStateException e) {
+      // expected
+      Assert.assertTrue(e.getMessage().startsWith("NaN as null default value is not managed yet for"));
+    }
+
+    schema =
+        new Schema.SchemaBuilder().setSchemaName(TABLE_NAME).addSingleValueDimension("string", DataType.STRING, "NaN")
+            .build();
+    tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME).build();
+    SchemaUtils.validate(schema, Lists.newArrayList(tableConfig));
   }
 
   private Map<String, String> getStreamConfigs() {

diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/SchemaUtils.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/SchemaUtils.java
@@ -144,6 +144,10 @@ public static void validate(Schema schema, boolean isIgnoreCase) {
         if (fieldSpec.getFieldType().equals(FieldSpec.FieldType.DATE_TIME)) {
           validateDateTimeFieldSpec((DateTimeFieldSpec) fieldSpec);
         }
+        if (fieldSpec.getDataType().equals(FieldSpec.DataType.FLOAT) || fieldSpec.getDataType()
+            .equals(FieldSpec.DataType.DOUBLE)) {
+          validateDefaultIsNotNaN(fieldSpec);
+        }
       }
     }
     Preconditions.checkState(Collections.disjoint(transformedColumns, argumentColumns),
@@ -157,6 +161,12 @@ public static void validate(Schema schema, boolean isIgnoreCase) {
     }
   }
 
+  private static void validateDefaultIsNotNaN(FieldSpec fieldSpec) {
+    Preconditions.checkState(!fieldSpec.getDefaultNullValueString().equals("NaN"),
+            "NaN as null default value is not managed yet for %s",
+            fieldSpec.getName());
+  }
+
   /**
    * Validates that the schema is compatible with the given table config
    */