Fix variable expression serde

Variables were serialized as "name(type)". This conflicts with some special literal naming convention like "$literal$array(integer)", which is a legit name. Fix this by using angle brackets "name<type>".
chunzhao · Jul 16, 2019 · cbe20f5 · cbe20f5
1 parent cb78200
commit cbe20f5
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 5 deletions.
diff --git a/presto-main/src/main/java/com/facebook/presto/sql/Serialization.java b/presto-main/src/main/java/com/facebook/presto/sql/Serialization.java
@@ -37,6 +37,10 @@
 
 public final class Serialization
 {
+    // for variable SerDe; variable names might contain "()"; use angle brackets to avoid conflict
+    private static final char VARIABLE_TYPE_OPEN_BRACKET = '<';
+    private static final char VARIABLE_TYPE_CLOSE_BRACKET = '>';
+
     private Serialization() {}
 
     public static class ExpressionSerializer
@@ -95,7 +99,8 @@ public static class VariableReferenceExpressionSerializer
         public void serialize(VariableReferenceExpression value, JsonGenerator jsonGenerator, SerializerProvider serializers)
                 throws IOException
         {
-            jsonGenerator.writeFieldName(format("%s(%s)", value.getName(), value.getType()));
+            // serialize variable as "name<type>"
+            jsonGenerator.writeFieldName(format("%s%s%s%s", value.getName(), VARIABLE_TYPE_OPEN_BRACKET, value.getType(), VARIABLE_TYPE_CLOSE_BRACKET));
         }
     }
 
@@ -112,11 +117,10 @@ public VariableReferenceExpressionDeserializer(TypeManager typeManager)
 
         @Override
         public Object deserializeKey(String key, DeserializationContext ctxt)
-                throws IOException
         {
-            int p = key.indexOf("(");
-            if (p <= 0 || key.charAt(key.length() - 1) != ')') {
-                throw new IllegalArgumentException(format("Expect key to be of format 'name(type)', found %s", key));
+            int p = key.indexOf(VARIABLE_TYPE_OPEN_BRACKET);
+            if (p <= 0 || key.charAt(key.length() - 1) != VARIABLE_TYPE_CLOSE_BRACKET) {
+                throw new IllegalArgumentException(format("Expect key to be of format 'name<type>', found %s", key));
             }
             return new VariableReferenceExpression(key.substring(0, p), typeManager.getType(parseTypeSignature(key.substring(p + 1, key.length() - 1))));
         }

diff --git a/presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestQueries.java b/presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestQueries.java
@@ -572,6 +572,9 @@ public void testUnnest()
         assertQuery("SELECT b FROM UNNEST(ARRAY[1, 2, 3], ARRAY[4, 5]) t(a, b)", "SELECT * FROM VALUES 4, 5, NULL");
         assertQuery("SELECT count(*) FROM UNNEST(ARRAY[1, 2, 3], ARRAY[4, 5])", "SELECT 3");
         assertQuery("SELECT a FROM UNNEST(ARRAY['kittens', 'puppies']) t(a)", "SELECT * FROM VALUES ('kittens'), ('puppies')");
+        assertQuery(
+                "WITH unioned AS ( SELECT 1 UNION ALL SELECT 2 ) SELECT * FROM unioned CROSS JOIN UNNEST(ARRAY[3]) steps (step)",
+                "SELECT * FROM (VALUES (1, 3), (2, 3))");
         assertQuery("" +
                         "SELECT c " +
                         "FROM UNNEST(ARRAY[1, 2, 3], ARRAY[4, 5]) t(a, b) " +