fix(isthmus): convert BinaryLiteral to VARBINARY

BREAKING CHANGE: BinaryLiteral is no longer converted to BINARY<length>)
substrait-io · Sep 20, 2023 · 55cd81d · 55cd81d
1 parent b5ee4e0
commit 55cd81d
Show file tree

Hide file tree

Showing 2 changed files with 3 additions and 4 deletions.
diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java
@@ -151,10 +151,10 @@ public RexNode visit(Expression.FixedBinaryLiteral expr) throws RuntimeException
 
   @Override
   public RexNode visit(Expression.BinaryLiteral expr) throws RuntimeException {
-    // Calcite RexLiteral only takes ByteString
     return rexBuilder.makeLiteral(
         new ByteString(expr.value().toByteArray()),
-        typeConverter.toCalcite(typeFactory, expr.getType()));
+        typeConverter.toCalcite(typeFactory, expr.getType()),
+        true);
   }
 
   @Override

diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java
@@ -85,9 +85,8 @@ void tStr() {
 
   @Test
   void tBinary() {
-    // TODO: varbinary vs fixed length binary
     var val = "my test".getBytes(StandardCharsets.UTF_8);
-    test(
+    bitest(
         binary(false, val),
         c(new org.apache.calcite.avatica.util.ByteString(val), SqlTypeName.VARBINARY));
   }