[analytics-engine] Skip unsupported field types when building Calcite schema

sandbox/libs/analytics-api/src/main/java/org/opensearch/analytics/schema/OpenSearchSchemaBuilder.java

@@ -64,32 +64,31 @@ public static SchemaPlus buildSchema(ClusterState clusterState) {
     }
 
     /**
-     * Maps an OpenSearch field type string to a Calcite SqlTypeName.
+     * Maps an OpenSearch field type string to a Calcite SqlTypeName, or {@code null} when the type
+     * has no scalar Calcite representation here (geo_point, geo_shape, nested, completion, …) or
+     * is unrecognized. Callers omit the column from the schema so a query referencing it surfaces
+     * a Calcite "column not found" via the validator rather than a planner-time crash.
      *
      * <p>Type mapping:
      * <ul>
      *   <li>keyword/text/match_only_text -> VARCHAR</li>
-     *   <li>long -> BIGINT</li>
-     *   <li>unsigned_long -> BIGINT</li>
-     *   <li>integer -> INTEGER</li>
-     *   <li>short -> SMALLINT</li>
-     *   <li>byte -> TINYINT</li>
-     *   <li>double -> DOUBLE</li>
-     *   <li>float -> REAL</li>
-     *   <li>half_float -> REAL</li>
-     *   <li>scaled_float -> BIGINT</li>
+     *   <li>long/unsigned_long/scaled_float -> BIGINT</li>
+     *   <li>integer -> INTEGER, short -> SMALLINT, byte -> TINYINT</li>
+     *   <li>double -> DOUBLE, float/half_float -> REAL</li>
      *   <li>boolean -> BOOLEAN</li>
-     *   <li>date -> TIMESTAMP</li>
-     *   <li>date_nanos -> TIMESTAMP</li>
-     *   <li>ip -> VARBINARY</li>
-     *   <li>binary -> VARBINARY</li>
-     *   <li>nested/object -> skip (not mapped)</li>
-     *   <li>unknown -> throws IllegalArgumentException</li>
+     *   <li>date/date_nanos -> TIMESTAMP</li>
+     *   <li>ip/binary -> VARBINARY</li>
+     *   <li>everything else (geo_point, geo_shape, nested, object, flat_object, completion,
+     *       constant_keyword, wildcard, alias, dense_vector, sparse_vector, percolator,
+     *       *_range, token_count, version, plus genuinely unknown plugin types) -> {@code null}</li>
      * </ul>
      *
      * @param opensearchType the OpenSearch field type string
      */
     public static SqlTypeName mapFieldType(String opensearchType) {
+        if (opensearchType == null) {
+            return null;
+        }
         switch (opensearchType) {
             case "keyword":
             case "text":
@@ -132,7 +131,7 @@ public static SqlTypeName mapFieldType(String opensearchType) {
                 // on-disk byte form the planner expects.
                 return SqlTypeName.VARBINARY;
             default:
-                throw new IllegalArgumentException("Unsupported OpenSearch field type: " + opensearchType);
+                return null;
         }
     }
 
@@ -172,6 +171,12 @@ private static void addLeafFields(
                 continue;
             }
             SqlTypeName sqlType = mapFieldType(fieldType);
+            if (sqlType == null) {
+                // Unsupported (geo_point/shape/completion/…) or unknown plugin type. Drop the
+                // column; a query referencing it surfaces a Calcite "column not found" via the
+                // validator rather than a planning-time IllegalArgumentException.
+                continue;
+            }
             builder.add(fieldName, typeFactory.createTypeWithNullability(typeFactory.createSqlType(sqlType), true));
         }
     }

sandbox/plugins/analytics-engine/src/test/java/org/opensearch/analytics/engine/OpenSearchSchemaBuilderTests.java

@@ -8,11 +8,18 @@
 
 package org.opensearch.analytics.engine;
 
+import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeField;
 import org.apache.calcite.schema.SchemaPlus;
 import org.apache.calcite.schema.Table;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.parser.SqlParser;
 import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.tools.FrameworkConfig;
+import org.apache.calcite.tools.Frameworks;
+import org.apache.calcite.tools.Planner;
 import org.opensearch.Version;
 import org.opensearch.analytics.schema.OpenSearchSchemaBuilder;
 import org.opensearch.cluster.ClusterName;
@@ -21,6 +28,7 @@
 import org.opensearch.cluster.metadata.Metadata;
 import org.opensearch.test.OpenSearchTestCase;
 
+import java.util.LinkedHashMap;
 import java.util.Map;
 
 public class OpenSearchSchemaBuilderTests extends OpenSearchTestCase {
@@ -155,11 +163,192 @@ public void testMapFieldTypeForAllSupportedTypes() {
     }
 
     /**
-     * Test that unknown field types throw IllegalArgumentException naming the offending type.
+     * Unsupported / unknown field types degrade to a null SqlTypeName. The schema builder drops
+     * such columns so a downstream query referencing one fails at validation time with
+     * "column not found", not at planning time with an IllegalArgumentException.
      */
-    public void testUnknownFieldTypeThrows() {
-        IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> OpenSearchSchemaBuilder.mapFieldType("geo_point"));
-        assertTrue("Exception message should mention the unsupported type, was: " + ex.getMessage(), ex.getMessage().contains("geo_point"));
+    public void testUnsupportedFieldTypesReturnNull() {
+        for (String unsupported : new String[] {
+            "geo_point",
+            "geo_shape",
+            "point",
+            "shape",
+            "completion",
+            "constant_keyword",
+            "wildcard",
+            "alias",
+            "flat_object",
+            "dense_vector",
+            "sparse_vector",
+            "percolator",
+            "integer_range",
+            "long_range",
+            "date_range",
+            "ip_range",
+            "token_count",
+            "version",
+            "made_up_plugin_type" }) {
+            assertNull("Expected null for unsupported type [" + unsupported + "]", OpenSearchSchemaBuilder.mapFieldType(unsupported));
+        }
+    }
+
+    /**
+     * Indices that mix supported and unsupported fields produce a schema containing only the
+     * supported ones — the unsupported columns are dropped silently rather than aborting the
+     * entire schema build for the index.
+     */
+    public void testIndexWithUnsupportedFieldsDropsThoseColumns() throws Exception {
+        ClusterState clusterState = buildClusterState(
+            Map.of("mixed_index", Map.of("name", "keyword", "location", "geo_point", "shape_field", "geo_shape", "age", "long"))
+        );
+
+        SchemaPlus schema = OpenSearchSchemaBuilder.buildSchema(clusterState);
+        Table table = schema.getTable("mixed_index");
+        assertNotNull("Table mixed_index should exist despite unsupported fields", table);
+
+        RelDataType rowType = table.getRowType(new org.apache.calcite.jdbc.JavaTypeFactoryImpl());
+        assertEquals("Only the 2 supported fields should be present", 2, rowType.getFieldCount());
+        assertFieldType(rowType, "name", SqlTypeName.VARCHAR);
+        assertFieldType(rowType, "age", SqlTypeName.BIGINT);
+        assertNull("geo_point column must be dropped", rowType.getField("location", true, false));
+        assertNull("geo_shape column must be dropped", rowType.getField("shape_field", true, false));
+    }
+
+    /**
+     * Index whose every field is unsupported still emits a Calcite Table — with an empty rowType.
+     * The build itself must not throw; downstream validation rejects any column reference
+     * naturally because the rowType has zero columns.
+     */
+    public void testAllUnsupportedFieldsProduceEmptyRowType() throws Exception {
+        ClusterState clusterState = buildClusterState(
+            Map.of("all_unsupported", Map.of("loc", "geo_point", "shape", "geo_shape", "feat", "completion"))
+        );
+
+        SchemaPlus schema = OpenSearchSchemaBuilder.buildSchema(clusterState);
+        Table table = schema.getTable("all_unsupported");
+        assertNotNull("Table must exist even when every field is unsupported", table);
+
+        RelDataType rowType = table.getRowType(new org.apache.calcite.jdbc.JavaTypeFactoryImpl());
+        assertEquals("All-unsupported index produces 0-column rowType", 0, rowType.getFieldCount());
+    }
+
+    /**
+     * Nested object containing geo_point + supported leaves. Confirms the recursive path in
+     * addLeafFields drops the unsupported leaf without aborting the recursion or losing
+     * sibling supported columns.
+     */
+    public void testNestedObjectWithUnsupportedLeafDropsOnlyThatLeaf() throws Exception {
+        String mapping = "{\"properties\":{"
+            + "\"customer\":{\"properties\":{"
+            + "\"id\":{\"type\":\"keyword\"},"
+            + "\"home\":{\"type\":\"geo_point\"},"
+            + "\"age\":{\"type\":\"integer\"}"
+            + "}}"
+            + "}}";
+        ClusterState clusterState = buildClusterStateRaw("nested_geo", mapping);
+
+        SchemaPlus schema = OpenSearchSchemaBuilder.buildSchema(clusterState);
+        Table table = schema.getTable("nested_geo");
+        assertNotNull(table);
+
+        RelDataType rowType = table.getRowType(new org.apache.calcite.jdbc.JavaTypeFactoryImpl());
+        assertEquals("Only 2 supported nested leaves should remain", 2, rowType.getFieldCount());
+        assertFieldType(rowType, "customer.id", SqlTypeName.VARCHAR);
+        assertFieldType(rowType, "customer.age", SqlTypeName.INTEGER);
+        assertNull("nested geo_point leaf must be dropped", rowType.getField("customer.home", true, false));
+    }
+
+    /**
+     * Calcite end-to-end: parsing and validating a query that projects a SUPPORTED column on an
+     * index whose mapping contains an unsupported field must succeed. This is the scan_viability
+     * bug reproducer pattern — geo_point in mapping but not in projection.
+     */
+    public void testCalciteValidatesQuerySkippingUnsupportedField() throws Exception {
+        ClusterState clusterState = buildClusterState(Map.of("kai_repro", Map.of("name", "keyword", "loc", "geo_point", "age", "long")));
+        SchemaPlus schema = OpenSearchSchemaBuilder.buildSchema(clusterState);
+
+        RelNode rel = parseValidateConvert(schema, "SELECT name, age FROM kai_repro");
+        assertNotNull("Calcite should produce a RelNode for projection-of-supported-only", rel);
+        assertEquals("Result must have exactly the 2 projected columns", 2, rel.getRowType().getFieldCount());
+        // Calcite uppercases unquoted identifiers by default; compare case-insensitively.
+        assertEquals("name", rel.getRowType().getFieldList().get(0).getName().toLowerCase(java.util.Locale.ROOT));
+        assertEquals("age", rel.getRowType().getFieldList().get(1).getName().toLowerCase(java.util.Locale.ROOT));
+    }
+
+    /**
+     * Calcite end-to-end: projecting a DROPPED unsupported column must surface as a validator
+     * error naming the column — not as IllegalArgumentException, NPE, or silent null.
+     * This is the load-bearing contract behind the schema-degradation fix.
+     */
+    public void testCalciteRejectsQueryReferencingDroppedColumn() throws Exception {
+        ClusterState clusterState = buildClusterState(Map.of("kai_repro", Map.of("name", "keyword", "loc", "geo_point")));
+        SchemaPlus schema = OpenSearchSchemaBuilder.buildSchema(clusterState);
+
+        Exception ex = expectThrows(Exception.class, () -> parseValidateConvert(schema, "SELECT loc FROM kai_repro"));
+        // Calcite wraps validator errors; walk the cause chain for the column name.
+        String fullMessage = collectMessages(ex);
+        assertTrue(
+            "Validator error must mention dropped column 'loc'; got: " + fullMessage,
+            fullMessage.toLowerCase(java.util.Locale.ROOT).contains("loc")
+        );
+        assertFalse(
+            "Must NOT surface as IllegalArgumentException 'Unsupported OpenSearch field type'; got: " + fullMessage,
+            fullMessage.contains("Unsupported OpenSearch field type")
+        );
+    }
+
+    /**
+     * Field with no "type" property and no "properties" sub-map (malformed mapping). The
+     * existing object-recursion branch enters but finds nothing to add, so the field is
+     * silently skipped — must not throw.
+     */
+    public void testMalformedFieldWithoutTypeOrPropertiesIsSkipped() throws Exception {
+        String mapping = "{\"properties\":{"
+            + "\"normal\":{\"type\":\"keyword\"},"
+            + "\"weird\":{\"index\":\"not_analyzed\"}"  // no "type", no "properties"
+            + "}}";
+        ClusterState clusterState = buildClusterStateRaw("malformed_idx", mapping);
+
+        SchemaPlus schema = OpenSearchSchemaBuilder.buildSchema(clusterState);
+        Table table = schema.getTable("malformed_idx");
+        assertNotNull(table);
+
+        RelDataType rowType = table.getRowType(new org.apache.calcite.jdbc.JavaTypeFactoryImpl());
+        assertEquals("Only the well-formed field should survive", 1, rowType.getFieldCount());
+        assertFieldType(rowType, "normal", SqlTypeName.VARCHAR);
+    }
+
+    /**
+     * Multi-fields (`fields`) sub-mapping is currently NOT walked — only the top-level
+     * "type" matters. An unsupported multi-field subtype thus has zero effect on the parent
+     * column. Pinning current behavior so a future change to walk `fields` is intentional.
+     */
+    public void testMultiFieldsSubtypeDoesNotAffectParent() throws Exception {
+        String mapping = "{\"properties\":{"
+            + "\"name\":{\"type\":\"keyword\",\"fields\":{"
+            + "\"raw\":{\"type\":\"keyword\"},"
+            + "\"loc\":{\"type\":\"geo_point\"}"
+            + "}}"
+            + "}}";
+        ClusterState clusterState = buildClusterStateRaw("multifield_idx", mapping);
+
+        SchemaPlus schema = OpenSearchSchemaBuilder.buildSchema(clusterState);
+        Table table = schema.getTable("multifield_idx");
+        assertNotNull(table);
+
+        RelDataType rowType = table.getRowType(new org.apache.calcite.jdbc.JavaTypeFactoryImpl());
+        assertEquals("Parent field is preserved; multi-fields subtree is ignored", 1, rowType.getFieldCount());
+        assertFieldType(rowType, "name", SqlTypeName.VARCHAR);
+        assertNull("Multi-field 'raw' is not currently surfaced as a column", rowType.getField("name.raw", true, false));
+        assertNull("Multi-field 'loc' is not currently surfaced as a column", rowType.getField("name.loc", true, false));
+    }
+
+    /**
+     * Defensive: mapFieldType called with null must return null (treated as "unknown") rather
+     * than NPE. Mirrors the analytics-framework FieldType.fromMappingType convention.
+     */
+    public void testMapFieldTypeReturnsNullOnNullInput() {
+        assertNull(OpenSearchSchemaBuilder.mapFieldType(null));
     }
 
     // --- helpers ---
@@ -178,10 +367,23 @@ private ClusterState buildClusterState(Map<String, Map<String, String>> indices)
         return ClusterState.builder(new ClusterName("test")).metadata(metadataBuilder.build()).build();
     }
 
+    private ClusterState buildClusterStateRaw(String indexName, String mappingJson) throws Exception {
+        IndexMetadata indexMetadata = IndexMetadata.builder(indexName)
+            .settings(settings(Version.CURRENT))
+            .numberOfShards(1)
+            .numberOfReplicas(0)
+            .putMapping(mappingJson)
+            .build();
+        Metadata metadata = Metadata.builder().put(indexMetadata, false).build();
+        return ClusterState.builder(new ClusterName("test")).metadata(metadata).build();
+    }
+
     private IndexMetadata buildIndexMetadata(String indexName, Map<String, String> fieldTypes) throws Exception {
+        // LinkedHashMap preserves declaration order so the assertions on field positions are stable.
+        Map<String, String> ordered = new LinkedHashMap<>(fieldTypes);
         StringBuilder mappingJson = new StringBuilder("{\"properties\":{");
         boolean first = true;
-        for (Map.Entry<String, String> field : fieldTypes.entrySet()) {
+        for (Map.Entry<String, String> field : ordered.entrySet()) {
             if (!first) mappingJson.append(",");
             mappingJson.append("\"").append(field.getKey()).append("\":{\"type\":\"").append(field.getValue()).append("\"}");
             first = false;
@@ -195,4 +397,32 @@ private IndexMetadata buildIndexMetadata(String indexName, Map<String, String> f
             .putMapping(mappingJson.toString())
             .build();
     }
+
+    /** Parse + validate + convert SQL against the given Calcite schema; throws on validator error. */
+    private static RelNode parseValidateConvert(SchemaPlus schema, String sql) throws Exception {
+        FrameworkConfig config = Frameworks.newConfigBuilder()
+            .parserConfig(SqlParser.config().withCaseSensitive(false))
+            .defaultSchema(schema)
+            .operatorTable(SqlStdOperatorTable.instance())
+            .build();
+        Planner planner = Frameworks.getPlanner(config);
+        try {
+            SqlNode parsed = planner.parse(sql);
+            SqlNode validated = planner.validate(parsed);
+            return planner.rel(validated).project();
+        } finally {
+            planner.close();
+        }
+    }
+
+    /** Concatenate the message of the throwable and every cause for substring assertions. */
+    private static String collectMessages(Throwable t) {
+        StringBuilder sb = new StringBuilder();
+        for (Throwable cur = t; cur != null; cur = cur.getCause()) {
+            if (cur.getMessage() != null) {
+                sb.append(cur.getMessage()).append(" | ");
+            }
+        }
+        return sb.toString();
+    }
 }