[analytics-backend-datafusion] Normalize CHAR/VARCHAR literals to Str in VirtualTable conversion

sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/DataFusionFragmentConvertor.java

@@ -61,6 +61,7 @@
 
 import io.substrait.expression.AggregateFunctionInvocation;
 import io.substrait.expression.Expression;
+import io.substrait.expression.ExpressionCreator;
 import io.substrait.expression.FunctionArg;
 import io.substrait.expression.ImmutableAggregateFunctionInvocation;
 import io.substrait.extension.ExtensionCollector;
@@ -70,6 +71,7 @@
 import io.substrait.isthmus.TypeConverter;
 import io.substrait.isthmus.expression.AggregateFunctionConverter;
 import io.substrait.isthmus.expression.FunctionMappings;
+import io.substrait.isthmus.expression.LiteralConverter;
 import io.substrait.isthmus.expression.ScalarFunctionConverter;
 import io.substrait.isthmus.expression.WindowFunctionConverter;
 import io.substrait.plan.Plan;
@@ -83,8 +85,10 @@
 import io.substrait.relation.Project;
 import io.substrait.relation.Rel;
 import io.substrait.relation.Sort;
+import io.substrait.relation.VirtualTableScan;
 import io.substrait.type.NamedStruct;
 import io.substrait.type.Type;
+import io.substrait.type.TypeCreator;
 import io.substrait.type.proto.TypeProtoConverter;
 
 /** Converts Calcite RelNode fragments to Substrait protobuf bytes for the DataFusion Rust runtime. */
@@ -769,15 +773,78 @@ protected ImmutableList<FunctionMappings.Sig> getSigs() {
             windowConverter,
             typeConverter
         );
+        LiteralConverter literalConverter = new LiteralConverter(typeConverter);
         return new SubstraitRelVisitor(converterProvider) {
             @Override
             public Rel visit(org.apache.calcite.rel.core.Aggregate aggregate) {
                 Rel rel = super.visit(aggregate);
                 return rel instanceof Aggregate agg ? addNullArgFilters(aggregate, agg) : rel;
             }
+
+            @Override
+            public Rel visit(org.apache.calcite.rel.core.Values values) {
+                if (values.getTuples().isEmpty()) {
+                    // No row literals means no per-value char length to conflict with the schema;
+                    // isthmus's default zero-row VirtualTable is fine.
+                    return super.visit(values);
+                }
+                return virtualTableWithStrLiterals(values, typeConverter, literalConverter);
+            }
         };
     }
 
+    /**
+     * Builds the Substrait {@link VirtualTableScan} for an inline {@link org.apache.calcite.rel.core.Values}
+     * leaf, normalising every CHAR/VARCHAR column to a length-independent {@code Str} in both the base
+     * schema and the row literals.
+     */
+    private static Rel virtualTableWithStrLiterals(
+        org.apache.calcite.rel.core.Values values,
+        TypeConverter typeConverter,
+        LiteralConverter literalConverter
+    ) {
+        RelDataType rowType = values.getRowType();
+        List<RelDataTypeField> fields = rowType.getFieldList();
+
+        List<String> names = new ArrayList<>(fields.size());
+        List<Type> fieldTypes = new ArrayList<>(fields.size());
+        for (RelDataTypeField f : fields) {
+            names.add(f.getName());
+            if (isCharacter(f.getType())) {
+                fieldTypes.add(TypeCreator.of(f.getType().isNullable()).STRING);
+            } else {
+                fieldTypes.add(typeConverter.toSubstrait(f.getType()));
+            }
+        }
+        NamedStruct schema = NamedStruct.of(names, TypeCreator.REQUIRED.struct(fieldTypes));
+
+        List<Expression.NestedStruct> rows = new ArrayList<>(values.getTuples().size());
+        for (List<RexLiteral> tuple : values.getTuples()) {
+            List<Expression> cells = new ArrayList<>(tuple.size());
+            for (int c = 0; c < tuple.size(); c++) {
+                RexLiteral lit = tuple.get(c);
+                RelDataType cellType = fields.get(c).getType();
+                if (isCharacter(cellType)) {
+                    if (lit.isNull()) {
+                        cells.add(ExpressionCreator.typedNull(TypeCreator.of(true).STRING));
+                    } else {
+                        cells.add(ExpressionCreator.string(cellType.isNullable(), lit.getValueAs(String.class)));
+                    }
+                } else {
+                    cells.add(literalConverter.convert(lit));
+                }
+            }
+            rows.add(ExpressionCreator.nestedStruct(false, cells));
+        }
+
+        return VirtualTableScan.builder().initialSchema(schema).addAllRows(rows).build();
+    }
+
+    private static boolean isCharacter(RelDataType type) {
+        SqlTypeName t = type.getSqlTypeName();
+        return t == SqlTypeName.CHAR || t == SqlTypeName.VARCHAR;
+    }
+
     /**
      * Adds an {@code is_not_null} {@code preMeasureFilter} to each measure whose {@link LocalAggOp}
      * declares {@link LocalAggOp#filtersNullArgs} — so the converter stays generic and only the op

sandbox/plugins/analytics-backend-datafusion/src/test/java/org/opensearch/be/datafusion/DataFusionFragmentConvertorTests.java

@@ -36,6 +36,9 @@
 import org.opensearch.analytics.planner.rel.OpenSearchStageInputScan;
 import org.opensearch.analytics.spi.DelegatedPredicateFunction;
 import org.opensearch.test.OpenSearchTestCase;
+import org.apache.calcite.rel.logical.LogicalValues;
+import org.apache.calcite.rex.RexLiteral;
+import com.google.common.collect.ImmutableList;
 
 import java.util.List;
 
@@ -849,4 +852,65 @@ public void testAttachFragmentOnTop_PreservesLiftedWindowProjectLayer() throws E
         assertTrue("lower's input must be the rewired stage-scan", innerOfLower.hasRead());
     }
 
+
+    // -- VirtualTable (inline Values) CHAR/VARCHAR -> Str normalization (PR #22554) --
+
+    /**
+     * A Values with a character column whose rows differ in length must convert to a VirtualTable
+     * whose schema column AND every row cell are Str, not fixed_char. Before the fix this tripped
+     * VirtualTableScan's row-conforms-to-schema check (FixedChar(5) vs FixedChar(3)) and
+     * convertFragment threw IllegalStateException.
+     */
+    public void testVirtualTable_MultiLengthCharColumn_NormalizedToStr() throws Exception {
+        LogicalValues values = charValues("Alice", "Bob");
+        ReadRel read = rootRel(decodeSubstrait(newConvertor().convertFragment(values))).getRead();
+        assertTrue("must be a VirtualTable", read.hasVirtualTable());
+        assertTrue("char schema column must be Str, not fixed_char", read.getBaseSchema().getStruct().getTypes(0).hasString());
+        assertEquals("two rows", 2, read.getVirtualTable().getExpressionsCount());
+        for (int r = 0; r < 2; r++) {
+            Expression cell = read.getVirtualTable().getExpressions(r).getFields(0);
+            assertTrue("row " + r + " cell must be a string literal", cell.getLiteral().hasString());
+        }
+    }
+
+    /** A mixed character + integer Values: char column -> Str; the integer column is left to isthmus and stays i32. */
+    public void testVirtualTable_MixedCharAndInt_IntStaysNumeric() throws Exception {
+        RelDataType charType = typeFactory.createTypeWithNullability(typeFactory.createSqlType(SqlTypeName.CHAR, 5), true);
+        RelDataType intType = typeFactory.createSqlType(SqlTypeName.INTEGER);
+        RelDataType rowType = typeFactory.builder().add("name", charType).add("age", intType).build();
+        ImmutableList<ImmutableList<RexLiteral>> tuples = ImmutableList.of(
+            ImmutableList.of(charLiteral("Alice"), (RexLiteral) rexBuilder.makeLiteral(30, intType, false)),
+            ImmutableList.of(charLiteral("Bob"), (RexLiteral) rexBuilder.makeLiteral(25, intType, false)));
+        LogicalValues values = (LogicalValues) LogicalValues.create(cluster, rowType, tuples);
+        ReadRel read = rootRel(decodeSubstrait(newConvertor().convertFragment(values))).getRead();
+        assertTrue("char column -> Str", read.getBaseSchema().getStruct().getTypes(0).hasString());
+        assertTrue("int column stays numeric (i32)", read.getBaseSchema().getStruct().getTypes(1).hasI32());
+        Expression.Nested.Struct row0 = read.getVirtualTable().getExpressions(0);
+        assertTrue("char cell -> string literal", row0.getFields(0).getLiteral().hasString());
+        assertEquals("int cell stays i32", 30, row0.getFields(1).getLiteral().getI32());
+    }
+
+    /** A zero-row Values takes the guarded branch (super.visit) and still lowers to a ReadRel. */
+    public void testVirtualTable_EmptyValues_DelegatesToSuper() throws Exception {
+        RelDataType charType = typeFactory.createTypeWithNullability(typeFactory.createSqlType(SqlTypeName.CHAR, 5), true);
+        RelDataType rowType = typeFactory.builder().add("name", charType).build();
+        LogicalValues empty = (LogicalValues) LogicalValues.createEmpty(cluster, rowType);
+        assertTrue("empty Values still lowers to a ReadRel", rootRel(decodeSubstrait(newConvertor().convertFragment(empty))).hasRead());
+    }
+
+    /** Builds a single-column CHAR Values, one row per value (each CHAR of the value's own length). */
+    private LogicalValues charValues(String... values) {
+        RelDataType colType = typeFactory.createTypeWithNullability(typeFactory.createSqlType(SqlTypeName.CHAR, 5), true);
+        RelDataType rowType = typeFactory.builder().add("name", colType).build();
+        java.util.List<ImmutableList<RexLiteral>> tuples = new java.util.ArrayList<>();
+        for (String v : values) {
+            tuples.add(ImmutableList.of(charLiteral(v)));
+        }
+        return (LogicalValues) LogicalValues.create(cluster, rowType, ImmutableList.copyOf(tuples));
+    }
+
+    /** A CHAR literal of the value's own length (e.g. 'Alice' -> CHAR(5), 'Bob' -> CHAR(3)). */
+    private RexLiteral charLiteral(String v) {
+        return (RexLiteral) rexBuilder.makeLiteral(v, typeFactory.createSqlType(SqlTypeName.CHAR, v.length()), false);
+    }
 }

sandbox/qa/analytics-engine-rest/src/test/resources/datasets/extensive_coverage/ppl/expected/q198.json

@@ -0,0 +1,12 @@
+{
+  "schema": [
+    { "name": "name", "type": "string" },
+    { "name": "age", "type": "string" }
+  ],
+  "datarows": [
+    [ "Alice", "30" ],
+    [ "Bob", "25" ]
+  ],
+  "total": 2,
+  "size": 2
+}

sandbox/qa/analytics-engine-rest/src/test/resources/datasets/extensive_coverage/ppl/q198.ppl

@@ -0,0 +1 @@
+makeresults format=json data='[{"name":"Alice","age":"30"},{"name":"Bob","age":"25"}]' | fields name, age