addressing comments

Signed-off-by: bharath-techie <[email protected]>
opensearch-project · Nov 19, 2024 · 3edb512 · 3edb512
1 parent 2d5d391
commit 3edb512
Show file tree

Hide file tree

Showing 7 changed files with 43 additions and 28 deletions.
diff --git a/...n/java/org/opensearch/index/codec/composite/composite912/Composite912DocValuesWriter.java b/...n/java/org/opensearch/index/codec/composite/composite912/Composite912DocValuesWriter.java
@@ -33,10 +33,11 @@
 import org.opensearch.index.compositeindex.datacube.startree.builder.StarTreesBuilder;
 import org.opensearch.index.compositeindex.datacube.startree.index.CompositeIndexValues;
 import org.opensearch.index.compositeindex.datacube.startree.index.StarTreeValues;
+import org.opensearch.index.fielddata.IndexNumericFieldData;
+import org.opensearch.index.fielddata.plain.SortedSetOrdinalsIndexFieldData;
 import org.opensearch.index.mapper.CompositeMappedFieldType;
 import org.opensearch.index.mapper.DocCountFieldMapper;
-import org.opensearch.index.mapper.IpFieldMapper;
-import org.opensearch.index.mapper.KeywordFieldMapper;
+import org.opensearch.index.mapper.MappedFieldType;
 import org.opensearch.index.mapper.MapperService;
 
 import java.io.IOException;
@@ -263,23 +264,38 @@ public SortedSetDocValues getSortedSet(FieldInfo field) {
                         return DocValues.emptySortedSet();
                     }
                 });
-            }
-            // TODO : change this logic to evaluate for sortedNumericField specifically
-            else {
+            } else if (isSortedNumericField(compositeField)) {
                 fieldProducerMap.put(compositeField, new EmptyDocValuesProducer() {
                     @Override
                     public SortedNumericDocValues getSortedNumeric(FieldInfo field) {
                         return DocValues.emptySortedNumeric();
                     }
                 });
+            } else {
+                throw new IllegalStateException(
+                    String.format("Unsupported DocValues field associated with the composite field : %s", compositeField)
+                );
             }
         }
         compositeFieldSet.remove(compositeField);
     }
 
     private boolean isSortedSetField(String field) {
-        return mapperService.fieldType(field) instanceof KeywordFieldMapper.KeywordFieldType
-            || mapperService.fieldType(field) instanceof IpFieldMapper.IpFieldType;
+        MappedFieldType ft = mapperService.fieldType(field);
+        assert ft.isAggregatable();
+        return ft.fielddataBuilder(
+            "",
+            () -> { throw new UnsupportedOperationException("SearchLookup not available"); }
+        ) instanceof SortedSetOrdinalsIndexFieldData.Builder;
+    }
+
+    private boolean isSortedNumericField(String field) {
+        MappedFieldType ft = mapperService.fieldType(field);
+        assert ft.isAggregatable();
+        return ft.fielddataBuilder(
+            "",
+            () -> { throw new UnsupportedOperationException("SearchLookup not available"); }
+        ) instanceof IndexNumericFieldData.Builder;
     }
 
     @Override
@@ -372,5 +388,4 @@ private static SegmentWriteState getSegmentWriteState(SegmentWriteState segmentW
             segmentWriteState.segmentSuffix
         );
     }
-
 }
diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/DimensionFactory.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/DimensionFactory.java
@@ -25,7 +25,7 @@
 
 import static org.opensearch.index.compositeindex.datacube.DateDimension.CALENDAR_INTERVALS;
 import static org.opensearch.index.compositeindex.datacube.IpDimension.IP;
-import static org.opensearch.index.compositeindex.datacube.KeywordDimension.KEYWORD;
+import static org.opensearch.index.compositeindex.datacube.OrdinalDimension.ORDINAL;
 
 /**
  * Dimension factory class mainly used to parse and create dimension from the mappings
@@ -45,8 +45,8 @@ public static Dimension parseAndCreateDimension(
                 return parseAndCreateDateDimension(name, dimensionMap, c);
             case NumericDimension.NUMERIC:
                 return new NumericDimension(name);
-            case KEYWORD:
-                return new KeywordDimension(name);
+            case ORDINAL:
+                return new OrdinalDimension(name);
             case IP:
                 return new IpDimension(name);
             default:
@@ -72,8 +72,8 @@ public static Dimension parseAndCreateDimension(
                 return parseAndCreateDateDimension(name, dimensionMap, c);
             case NUMERIC:
                 return new NumericDimension(name);
-            case KEYWORD:
-                return new KeywordDimension(name);
+            case ORDINAL:
+                return new OrdinalDimension(name);
             case IP:
                 return new IpDimension(name);
             default:

diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/DimensionType.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/DimensionType.java
@@ -30,10 +30,10 @@ public enum DimensionType {
     DATE,
 
     /**
-     * Represents a keyword dimension type.
-     * This is used for dimensions that contain keyword ordinals.
+     * Represents dimension types which uses ordinals.
+     * This is used for dimensions that contain sortedSet ordinals.
      */
-    KEYWORD,
+    ORDINAL,
 
     /**
      * Represents an IP dimension type.

diff --git a/...ositeindex/datacube/KeywordDimension.java → ...ositeindex/datacube/OrdinalDimension.java b/...ositeindex/datacube/KeywordDimension.java → ...ositeindex/datacube/OrdinalDimension.java
@@ -24,11 +24,11 @@
  * @opensearch.experimental
  */
 @ExperimentalApi
-public class KeywordDimension implements Dimension {
-    public static final String KEYWORD = "keyword";
+public class OrdinalDimension implements Dimension {
+    public static final String ORDINAL = "ordinal";
     private final String field;
 
-    public KeywordDimension(String field) {
+    public OrdinalDimension(String field) {
         this.field = field;
     }
 
@@ -62,7 +62,7 @@ public DocValuesType getDocValuesType() {
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
         builder.field(CompositeDataCubeFieldType.NAME, field);
-        builder.field(CompositeDataCubeFieldType.TYPE, KEYWORD);
+        builder.field(CompositeDataCubeFieldType.TYPE, ORDINAL);
         builder.endObject();
         return builder;
     }
@@ -71,7 +71,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
     public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
-        KeywordDimension dimension = (KeywordDimension) o;
+        OrdinalDimension dimension = (OrdinalDimension) o;
         return Objects.equals(field, dimension.getField());
     }
 

diff --git a/server/src/main/java/org/opensearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/KeywordFieldMapper.java
@@ -259,7 +259,7 @@ public KeywordFieldMapper build(BuilderContext context) {
 
         @Override
         public Optional<DimensionType> getSupportedDataCubeDimensionType() {
-            return Optional.of(DimensionType.KEYWORD);
+            return Optional.of(DimensionType.ORDINAL);
         }
     }
 

diff --git a/...nsearch/index/compositeindex/datacube/startree/builder/StarTreeBuilderFlushFlowTests.java b/...nsearch/index/compositeindex/datacube/startree/builder/StarTreeBuilderFlushFlowTests.java
@@ -21,10 +21,10 @@
 import org.opensearch.index.codec.composite.composite912.Composite912DocValuesFormat;
 import org.opensearch.index.compositeindex.datacube.Dimension;
 import org.opensearch.index.compositeindex.datacube.IpDimension;
-import org.opensearch.index.compositeindex.datacube.KeywordDimension;
 import org.opensearch.index.compositeindex.datacube.Metric;
 import org.opensearch.index.compositeindex.datacube.MetricStat;
 import org.opensearch.index.compositeindex.datacube.NumericDimension;
+import org.opensearch.index.compositeindex.datacube.OrdinalDimension;
 import org.opensearch.index.compositeindex.datacube.startree.StarTreeDocument;
 import org.opensearch.index.compositeindex.datacube.startree.StarTreeField;
 import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration;
@@ -533,8 +533,8 @@ private StarTreeField getStarTreeFieldWithMultipleMetrics() {
     }
 
     private StarTreeField getStarTreeFieldWithKeywordField(boolean isIp) {
-        Dimension d1 = isIp ? new IpDimension("field1") : new KeywordDimension("field1");
-        Dimension d2 = isIp ? new IpDimension("field3") : new KeywordDimension("field3");
+        Dimension d1 = isIp ? new IpDimension("field1") : new OrdinalDimension("field1");
+        Dimension d2 = isIp ? new IpDimension("field3") : new OrdinalDimension("field3");
         Metric m1 = new Metric("field2", List.of(MetricStat.SUM));
         Metric m2 = new Metric("field2", List.of(MetricStat.VALUE_COUNT));
         Metric m3 = new Metric("field2", List.of(MetricStat.AVG));

diff --git a/...rg/opensearch/index/compositeindex/datacube/startree/builder/StarTreeBuilderTestCase.java b/...rg/opensearch/index/compositeindex/datacube/startree/builder/StarTreeBuilderTestCase.java
@@ -33,10 +33,10 @@
 import org.opensearch.index.compositeindex.datacube.DateDimension;
 import org.opensearch.index.compositeindex.datacube.Dimension;
 import org.opensearch.index.compositeindex.datacube.IpDimension;
-import org.opensearch.index.compositeindex.datacube.KeywordDimension;
 import org.opensearch.index.compositeindex.datacube.Metric;
 import org.opensearch.index.compositeindex.datacube.MetricStat;
 import org.opensearch.index.compositeindex.datacube.NumericDimension;
+import org.opensearch.index.compositeindex.datacube.OrdinalDimension;
 import org.opensearch.index.compositeindex.datacube.startree.StarTreeDocument;
 import org.opensearch.index.compositeindex.datacube.startree.StarTreeField;
 import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration;
@@ -354,8 +354,8 @@ protected StarTreeMetadata getStarTreeMetadata(
     }
 
     protected StarTreeField getStarTreeFieldWithKeywords(boolean ip) {
-        Dimension d1 = ip ? new IpDimension("field1") : new KeywordDimension("field1");
-        Dimension d2 = ip ? new IpDimension("field3") : new KeywordDimension("field3");
+        Dimension d1 = ip ? new IpDimension("field1") : new OrdinalDimension("field1");
+        Dimension d2 = ip ? new IpDimension("field3") : new OrdinalDimension("field3");
         Metric m1 = new Metric("field2", List.of(MetricStat.VALUE_COUNT, MetricStat.SUM));
         List<Dimension> dims = List.of(d1, d2);
         List<Metric> metrics = List.of(m1);