fix(core)!: reject varchar literals exceeding their declared length

core/src/main/java/io/substrait/expression/Expression.java

@@ -823,6 +823,23 @@ abstract class VarCharLiteral implements Literal {
      */
     public abstract int length();
 
+    /**
+     * Validates that the value fits within the declared maximum length in Unicode code points.
+     *
+     * @throws IllegalArgumentException if the value exceeds the declared length
+     */
+    @Value.Check
+    protected void check() {
+      int characters = value().codePointCount(0, value().length());
+      if (characters > length()) {
+        throw new IllegalArgumentException(
+            "VarCharLiteral value has "
+                + characters
+                + " characters, exceeding declared length "
+                + length());
+      }
+    }
+
     @Override
     public Type getType() {
       return Type.withNullability(nullable()).varChar(length());

core/src/test/java/io/substrait/expression/VarCharLiteralTest.java

@@ -0,0 +1,60 @@
+package io.substrait.expression;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import io.substrait.TestBase;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+class VarCharLiteralTest extends TestBase {
+
+  @ParameterizedTest
+  @CsvSource({"abc, 3", "abc, 5", "'a😀', 2", "'é', 2", "'', 0"})
+  void fittingValuesRoundTrip(String value, int length) {
+    for (boolean nullable : new boolean[] {false, true}) {
+      Expression.VarCharLiteral literal = ExpressionCreator.varChar(nullable, value, length);
+      assertEquals(value, literal.value());
+      assertEquals(length, literal.length());
+      assertEquals(nullable, literal.nullable());
+      verifyRoundTrip(literal);
+    }
+  }
+
+  @ParameterizedTest
+  @ValueSource(booleans = {false, true})
+  void builderRejectsOverlongValue(boolean nullable) {
+    assertThrows(
+        IllegalArgumentException.class,
+        () ->
+            Expression.VarCharLiteral.builder()
+                .nullable(nullable)
+                .value("abcdef")
+                .length(3)
+                .build());
+  }
+
+  @ParameterizedTest
+  @CsvSource({"abcdef, 3", "'a😀', 1", "'é', 1"})
+  void factoryRejectsOverlongValue(String value, int length) {
+    assertThrows(
+        IllegalArgumentException.class, () -> ExpressionCreator.varChar(false, value, length));
+  }
+
+  @ParameterizedTest
+  @ValueSource(booleans = {false, true})
+  void protoRejectsOverlongValue(boolean nullable) {
+    io.substrait.proto.Expression proto =
+        io.substrait.proto.Expression.newBuilder()
+            .setLiteral(
+                io.substrait.proto.Expression.Literal.newBuilder()
+                    .setNullable(nullable)
+                    .setVarChar(
+                        io.substrait.proto.Expression.Literal.VarChar.newBuilder()
+                            .setValue("abcdef")
+                            .setLength(3)))
+            .build();
+    assertThrows(IllegalArgumentException.class, () -> protoExpressionConverter.from(proto));
+  }
+}

core/src/test/java/io/substrait/type/proto/GenericRoundtripTest.java

@@ -88,6 +88,11 @@ private static List<Object> instantiateParams(Method m)
       }
       l.add(valGenerator(param));
     }
+    if (m.getReturnType().equals(Expression.VarCharLiteral.class)) {
+      // The declared width must fit the generated value, rather than be an independent integer.
+      String value = (String) l.get(1);
+      l.set(2, value.codePointCount(0, value.length()));
+    }
     return l;
   }
 

isthmus/src/test/java/io/substrait/isthmus/LiteralNullabilityRoundtripTest.java

@@ -117,23 +117,27 @@ void nullableLiteralBecomesNullabilityCast() {
   /**
    * A cast doing more than nullability is not this representation, so the tuple unwrap leaves it
    * alone -- and a row whose values are not literals to Calcite is not a Values row at all, so it
-   * takes the projection encoding with the cast still on it. The literal here is malformed input:
-   * its value is longer than the length it declares, and nothing rejects it, here or in the POJO
-   * that holds it. What this pins is only the cast, which is the part the conversion decides.
+   * takes the projection encoding with the cast still on it.
    */
   @Test
   void tupleUnwrapLeavesATruncatingCastAlone() {
-    VirtualTableScan overlong =
+    VirtualTableScan castRow =
         VirtualTableScan.builder()
             .initialSchema(NamedStruct.of(List.of("A"), R.struct(R.varChar(3))))
             .addRows(
                 ExpressionCreator.nestedStruct(
-                    false, ExpressionCreator.varChar(false, "abcdef", 3)))
+                    false,
+                    ExpressionCreator.cast(
+                        R.varChar(3),
+                        ExpressionCreator.varChar(false, "abcdef", 6),
+                        Expression.FailureBehavior.THROW_EXCEPTION)))
             .build();
 
+    RelNode calcite = substraitToCalcite.convert(castRow);
     assertEquals(
-        "VirtualTable(rows=[[{ CAST('abcdef'):VARCHAR(3) NOT NULL }]])\n",
-        RelOptUtil.toString(substraitToCalcite.convert(overlong)));
+        "VirtualTable(rows=[[{ CAST('abcdef':VARCHAR(6)):VARCHAR(3) NOT NULL }]])\n",
+        RelOptUtil.toString(calcite));
+    assertEquals(castRow, SubstraitRelVisitor.convert(calcite, converterProvider));
   }
 
   /**