drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject [2/2] drill git commit: DRILL-2976: Part 2 - disable extended json in the default convert_toJSON method, the functionality is still accessible in a new convert_toComplexJSON method.
Date Wed, 13 May 2015 04:53:33 GMT
DRILL-2976: Part 2 - disable extended json in the default convert_toJSON method, the functionality
is still accessible in a new convert_toComplexJSON method.

Added unit tests for both versions of the function (in the case of the simple json version
I added result verification to an old test)

Address Mehant's review comments.

Fix failing unit test to turn on now disabled option for using extended types in written json
files.


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/4199e6bb
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/4199e6bb
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/4199e6bb

Branch: refs/heads/master
Commit: 4199e6bb37804bdf1925f050aaa60fd263b00f98
Parents: 4c8cced
Author: Jason Altekruse <altekrusejason@gmail.com>
Authored: Fri May 8 16:45:15 2015 -0700
Committer: Jason Altekruse <altekrusejason@gmail.com>
Committed: Tue May 12 19:48:11 2015 -0700

----------------------------------------------------------------------
 .../org/apache/drill/common/types/Types.java    |  1 +
 .../exec/expr/fn/impl/conv/JsonConvertTo.java   | 18 ++++--
 .../physical/impl/TestConvertFunctions.java     | 62 +++++++++++++++++++-
 .../complex/writer/TestExtendedTypes.java       | 32 ++++++----
 4 files changed, 94 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/4199e6bb/common/src/main/java/org/apache/drill/common/types/Types.java
----------------------------------------------------------------------
diff --git a/common/src/main/java/org/apache/drill/common/types/Types.java b/common/src/main/java/org/apache/drill/common/types/Types.java
index cec433f..df484b7 100644
--- a/common/src/main/java/org/apache/drill/common/types/Types.java
+++ b/common/src/main/java/org/apache/drill/common/types/Types.java
@@ -376,6 +376,7 @@ public class Types {
       return MinorType.VARBINARY;
     case "json":
     case "simplejson":
+    case "extendedjson":
       return MinorType.LATE;
     case "null":
     case "any":

http://git-wip-us.apache.org/repos/asf/drill/blob/4199e6bb/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/conv/JsonConvertTo.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/conv/JsonConvertTo.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/conv/JsonConvertTo.java
index 1d2292e..772999d 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/conv/JsonConvertTo.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/conv/JsonConvertTo.java
@@ -31,13 +31,21 @@ import org.apache.drill.exec.expr.annotations.Param;
 import org.apache.drill.exec.expr.holders.VarBinaryHolder;
 import org.apache.drill.exec.vector.complex.reader.FieldReader;
 
+/**
+ * The two functions defined here convert_toJSON and convert_toEXTENDEDJSON are almost
+ * identical. For now, the default behavior is to use simple JSON (see DRILL-2976). Until
the issues with
+ * extended JSON types are resolved, the convert_toJSON/convert_toSIMPLEJSON is consider
the default. The default
+ * will possibly change in the future, as the extended types can accurately serialize more
types supported
+ * by Drill.
+ * TODO(DRILL-2906) - review the default once issues with extended JSON are resolved
+ */
 public class JsonConvertTo {
 
  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(JsonConvertTo.class);
 
   private JsonConvertTo(){}
 
-  @FunctionTemplate(name = "convert_toJSON", scope = FunctionScope.SIMPLE, nulls = NullHandling.NULL_IF_NULL)
+  @FunctionTemplate(names = { "convert_toJSON", "convert_toSIMPLEJSON" } , scope = FunctionScope.SIMPLE,
nulls = NullHandling.NULL_IF_NULL)
   public static class ConvertToJson implements DrillSimpleFunc{
 
     @Param FieldReader input;
@@ -52,7 +60,7 @@ public class JsonConvertTo {
 
       java.io.ByteArrayOutputStream stream = new java.io.ByteArrayOutputStream();
       try {
-        org.apache.drill.exec.vector.complex.fn.JsonWriter jsonWriter = new org.apache.drill.exec.vector.complex.fn.JsonWriter(stream,
true, true);
+        org.apache.drill.exec.vector.complex.fn.JsonWriter jsonWriter = new org.apache.drill.exec.vector.complex.fn.JsonWriter(stream,
true, false);
 
         jsonWriter.write(input);
       } catch (Exception e) {
@@ -67,8 +75,8 @@ public class JsonConvertTo {
     }
   }
 
-  @FunctionTemplate(name = "convert_toSIMPLEJSON", scope = FunctionScope.SIMPLE, nulls =
NullHandling.NULL_IF_NULL)
-  public static class ConvertToSimpleJson implements DrillSimpleFunc{
+  @FunctionTemplate(name = "convert_toEXTENDEDJSON", scope = FunctionScope.SIMPLE, nulls
= NullHandling.NULL_IF_NULL)
+  public static class ConvertToExtendedJson implements DrillSimpleFunc{
 
     @Param FieldReader input;
     @Output VarBinaryHolder out;
@@ -82,7 +90,7 @@ public class JsonConvertTo {
 
       java.io.ByteArrayOutputStream stream = new java.io.ByteArrayOutputStream();
       try {
-        org.apache.drill.exec.vector.complex.fn.JsonWriter jsonWriter = new org.apache.drill.exec.vector.complex.fn.JsonWriter(stream,
true, false);
+        org.apache.drill.exec.vector.complex.fn.JsonWriter jsonWriter = new org.apache.drill.exec.vector.complex.fn.JsonWriter(stream,
true, true);
 
         jsonWriter.write(input);
       } catch (Exception e) {

http://git-wip-us.apache.org/repos/asf/drill/blob/4199e6bb/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
index 7a52130..a911e29 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
@@ -17,6 +17,8 @@
  */
 package org.apache.drill.exec.physical.impl;
 
+import static org.apache.drill.TestBuilder.listOf;
+import static org.apache.drill.TestBuilder.mapOf;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -30,6 +32,7 @@ import java.util.List;
 import mockit.Injectable;
 
 import org.apache.drill.BaseTestQuery;
+import org.apache.drill.TestBuilder;
 import org.apache.drill.exec.compile.ClassTransformer;
 import org.apache.drill.exec.compile.ClassTransformer.ScalarReplacementOption;
 import org.apache.drill.exec.expr.fn.impl.DateUtility;
@@ -74,9 +77,62 @@ public class TestConvertFunctions extends BaseTestQuery {
 
   @Test
   public void test_JSON_convertTo_empty_list_drill_1416() throws Exception {
-    test("select cast(convert_to(rl[1], 'JSON') as varchar(100)) from cp.`/store/json/input2.json`");
-    test("select convert_from(convert_to(rl[1], 'JSON'), 'JSON') from cp.`/store/json/input2.json`");
-    test("select convert_from(convert_to(rl[1], 'JSON'), 'JSON') from cp.`/store/json/json_project_null_object_from_list.json`");
+
+    String listStr = "[ 4, 6 ]";
+    testBuilder()
+        .sqlQuery("select cast(convert_to(rl[1], 'JSON') as varchar(100)) as json_str from
cp.`/store/json/input2.json`")
+        .unOrdered()
+        .baselineColumns("json_str")
+        .baselineValues(listStr)
+        .baselineValues("[ ]")
+        .baselineValues(listStr)
+        .baselineValues(listStr)
+        .go();
+
+    Object listVal = listOf(4l, 6l);
+    testBuilder()
+        .sqlQuery("select convert_from(convert_to(rl[1], 'JSON'), 'JSON') list_col from cp.`/store/json/input2.json`")
+        .unOrdered()
+        .baselineColumns("list_col")
+        .baselineValues(listVal)
+        .baselineValues(listOf())
+        .baselineValues(listVal)
+        .baselineValues(listVal)
+        .go();
+
+    Object mapVal1 = mapOf("f1", 4l, "f2", 6l);
+    Object mapVal2 = mapOf("f1", 11l);
+    testBuilder()
+        .sqlQuery("select convert_from(convert_to(rl[1], 'JSON'), 'JSON') as map_col from
cp.`/store/json/json_project_null_object_from_list.json`")
+        .unOrdered()
+        .baselineColumns("map_col")
+        .baselineValues(mapVal1)
+        .baselineValues(mapOf())
+        .baselineValues(mapVal2)
+        .baselineValues(mapVal1)
+        .go();
+  }
+
+  @Test
+  public void testConvertToComplexJSON() throws Exception {
+
+    String result1 =
+        "[ {\n" +
+            "  \"$numberLong\" : 4\n" +
+            "}, {\n" +
+            "  \"$numberLong\" : 6\n" +
+            "} ]";
+    String result2 = "[ ]";
+
+    testBuilder()
+        .sqlQuery("select cast(convert_to(rl[1], 'EXTENDEDJSON') as varchar(100)) as json_str
from cp.`/store/json/input2.json`")
+        .unOrdered()
+        .baselineColumns("json_str")
+        .baselineValues(result1)
+        .baselineValues(result2)
+        .baselineValues(result1)
+        .baselineValues(result1)
+        .go();
   }
 
   @Test

http://git-wip-us.apache.org/repos/asf/drill/blob/4199e6bb/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestExtendedTypes.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestExtendedTypes.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestExtendedTypes.java
index 9d0af41..f403108 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestExtendedTypes.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestExtendedTypes.java
@@ -26,6 +26,7 @@ import java.util.regex.Pattern;
 
 import org.apache.drill.BaseTestQuery;
 import org.apache.drill.common.util.TestTools;
+import org.apache.drill.exec.ExecConstants;
 import org.junit.Test;
 
 public class TestExtendedTypes extends BaseTestQuery {
@@ -39,19 +40,28 @@ public class TestExtendedTypes extends BaseTestQuery {
         Matcher.quoteReplacement(TestTools.getWorkingPath()));
 
     final String newTable = "TestExtendedTypes/newjson";
-    testNoResult("ALTER SESSION SET `store.format` = 'json'");
+    try {
+      testNoResult(String.format("ALTER SESSION SET `%s` = 'json'", ExecConstants.OUTPUT_FORMAT_VALIDATOR.getOptionName()));
+      testNoResult(String.format("ALTER SESSION SET `%s` = true", ExecConstants.JSON_EXTENDED_TYPES.getOptionName()));
 
-    // create table
-    test("create table dfs_test.tmp.`%s` as select * from dfs.`%s`", newTable, originalFile);
+      // create table
+      test("create table dfs_test.tmp.`%s` as select * from dfs.`%s`", newTable, originalFile);
 
-    // check query of table.
-    test("select * from dfs_test.tmp.`%s`", newTable);
-
-    // check that original file and new file match.
-    final byte[] originalData = Files.readAllBytes(Paths.get(originalFile));
-    final byte[] newData = Files.readAllBytes(Paths.get(BaseTestQuery.getDfsTestTmpSchemaLocation()
+ '/' + newTable
-        + "/0_0_0.json"));
-    assertEquals(new String(originalData), new String(newData));
+      // check query of table.
+      test("select * from dfs_test.tmp.`%s`", newTable);
 
+      // check that original file and new file match.
+      final byte[] originalData = Files.readAllBytes(Paths.get(originalFile));
+      final byte[] newData = Files.readAllBytes(Paths.get(BaseTestQuery.getDfsTestTmpSchemaLocation()
+ '/' + newTable
+          + "/0_0_0.json"));
+      assertEquals(new String(originalData), new String(newData));
+    } finally {
+      testNoResult(String.format("ALTER SESSION SET `%s` = '%s'",
+          ExecConstants.OUTPUT_FORMAT_VALIDATOR.getOptionName(),
+          ExecConstants.OUTPUT_FORMAT_VALIDATOR.getDefault().getValue()));
+      testNoResult(String.format("ALTER SESSION SET `%s` = %s",
+          ExecConstants.JSON_EXTENDED_TYPES.getOptionName(),
+          ExecConstants.JSON_EXTENDED_TYPES.getDefault().getValue()));
+    }
   }
 }


Mime
View raw message