You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by js...@apache.org on 2015/05/13 06:53:32 UTC
[1/2] drill git commit: DRILL-2976: Default extended types option for
json writer is false
Repository: drill
Updated Branches:
refs/heads/master 83d8ebe60 -> 4199e6bb3
DRILL-2976: Default extended types option for json writer is false
Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/4c8ccedf
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/4c8ccedf
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/4c8ccedf
Branch: refs/heads/master
Commit: 4c8ccedf387a8e3539f8eaabdcbc7945655d11e9
Parents: 83d8ebe
Author: Sudheesh Katkam <sk...@maprtech.com>
Authored: Wed May 6 17:09:35 2015 -0700
Committer: Jason Altekruse <al...@gmail.com>
Committed: Tue May 12 19:48:10 2015 -0700
----------------------------------------------------------------------
.../src/main/java/org/apache/drill/exec/ExecConstants.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/drill/blob/4c8ccedf/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
index 7bbb815..8a24e8d 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
@@ -125,7 +125,7 @@ public interface ExecConstants {
public static String JSON_ALL_TEXT_MODE = "store.json.all_text_mode";
public static BooleanValidator JSON_READER_ALL_TEXT_MODE_VALIDATOR = new BooleanValidator(JSON_ALL_TEXT_MODE, false);
- public static final BooleanValidator JSON_EXTENDED_TYPES = new BooleanValidator("store.json.extended_types", true);
+ public static final BooleanValidator JSON_EXTENDED_TYPES = new BooleanValidator("store.json.extended_types", false);
public static final DoubleValidator TEXT_ESTIMATED_ROW_SIZE = new RangeDoubleValidator(
"store.text.estimated_row_size_bytes", 1, Long.MAX_VALUE, 100.0);
[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.
Posted by js...@apache.org.
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 <al...@gmail.com>
Authored: Fri May 8 16:45:15 2015 -0700
Committer: Jason Altekruse <al...@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()));
+ }
}
}