[GitHub] drill pull request #514: DRILL-4694: CTAS in JSON format produces extraneous...

2016-06-03 Thread parthchandra
GitHub user parthchandra opened a pull request:

https://github.com/apache/drill/pull/514

DRILL-4694: CTAS in JSON format produces extraneous NULL fields

   Changed behavior of JSON CTAS to skip fields if the value is null. Added 
an option "store.json.writer.skip_null_fields" to enable old behavior.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/parthchandra/drill DRILL-4694

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/drill/pull/514.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #514


commit 68ff4950f8302725aadf72a50074f6eef735738b
Author: Parth Chandra 
Date:   2016-06-02T00:19:03Z

DRILL-4694: CTAS in JSON format produces extraneous NULL fields
   Changed behavior of JSON CTAS to skip fields if the value is null. Added 
an option "store.json.writer.skip_null_fields" to enable old behavior.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #514: DRILL-4694: CTAS in JSON format produces extraneous...

2016-06-03 Thread amansinha100
Github user amansinha100 commented on a diff in the pull request:

https://github.com/apache/drill/pull/514#discussion_r65795019
  
--- Diff: 
exec/java-exec/src/main/codegen/templates/JsonOutputRecordWriter.java ---
@@ -61,7 +62,13 @@
 
 @Override
 public void startField() throws IOException {
+  <#if mode.prefix = "Nullable" >
--- End diff --

should be == 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #514: DRILL-4694: CTAS in JSON format produces extraneous...

2016-06-03 Thread amansinha100
Github user amansinha100 commented on a diff in the pull request:

https://github.com/apache/drill/pull/514#discussion_r65795030
  
--- Diff: 
exec/java-exec/src/main/codegen/templates/JsonOutputRecordWriter.java ---
@@ -120,7 +127,13 @@ public void writeField() throws IOException {
   <#elseif mode.prefix == "Repeated" >
 gen.write${typeName}(i, reader);
   <#else>
+<#if mode.prefix = "Nullable" >
--- End diff --

same as above


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #514: DRILL-4694: CTAS in JSON format produces extraneous...

2016-06-03 Thread amansinha100
Github user amansinha100 commented on a diff in the pull request:

https://github.com/apache/drill/pull/514#discussion_r65795093
  
--- Diff: exec/java-exec/src/test/resources/json/ctas_alltypes_map_out.json 
---
@@ -0,0 +1,41 @@
+{
--- End diff --

minor point: in TestJsonReader lately we were embedding the data generation 
in the code instead of creating new files..but for small files like these I 
suppose this is fine. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #514: DRILL-4694: CTAS in JSON format produces extraneous...

2016-06-03 Thread amansinha100
Github user amansinha100 commented on a diff in the pull request:

https://github.com/apache/drill/pull/514#discussion_r65795257
  
--- Diff: exec/java-exec/src/test/java/org/apache/drill/TestCTASJson.java 
---
@@ -0,0 +1,129 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill;
+
+
+import org.apache.drill.common.util.TestTools;
+import org.apache.drill.exec.ExecConstants;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class TestCTASJson extends PlanTestBase {
+  static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(TestCTASJson.class);
+
+  static final String WORKING_PATH = TestTools.getWorkingPath();
+  static final String TEST_RES_PATH = WORKING_PATH + "/src/test/resources";
+
+  @Test
+  /**
+   * Test a source json file that contains records that are maps with 
fields of all types.
+   * Some records have missing fields. CTAS should skip the missing fields
+   */ public void testctas_alltypes_map() throws Exception {
+String testName = "ctas_alltypes_map";
+test("use dfs_test.tmp");
+test("alter session set store.format = 'json' ");
+test("alter session set store.json.writer.skip_null_fields = true"); 
// DEFAULT
+test("create table " + testName + "_json as select * from cp.`json/" + 
testName + ".json`");
+
+final String query = "select * from `" + testName + "_json` t1 ";
+
+testBuilder()
+.sqlQuery(query)
+.ordered()
+.jsonBaselineFile("json/" + testName + ".json")
+.build()
+.run();
+
+test("drop table " + testName + "_json" );
+  }
+
+  @Test
+  /**
+   * Test a source json file that contains records that are maps with 
fields of all types.
+   * Some records have missing fields. CTAS should NOT skip the missing 
fields
+   */
+  public void testctas_alltypes_map_noskip() throws Exception {
+String testName = "ctas_alltypes_map";
+test("use dfs_test.tmp");
+test("alter session set store.format = 'json' ");
+test("alter session set store.json.writer.skip_null_fields = false"); 
// CHANGE from default
+test("create table " + testName + "_json as select * from cp.`json/" + 
testName + ".json`");
+
+final String query = "select * from `" + testName + "_json` t1 ";
+
+testBuilder()
+.sqlQuery(query)
+.ordered()
+.jsonBaselineFile("json/" + testName + "_out.json")
+.build()
+.run();
+
+test("drop table " + testName + "_json" );
+  }
+
+  @Test
+  /**
+   * Test a source json file that contains records that are maps with 
fields of all types.
+   * Some records have missing fields. CTAS should skip the missing fields
+   */ public void testctas_alltypes_repeatedmap() throws Exception {
+String testName = "ctas_alltypes_repeated_map";
+test("use dfs_test.tmp");
+test("alter session set store.format = 'json' ");
+test("alter session set store.json.writer.skip_null_fields = true"); 
// DEFAULT
+test("create table " + testName + "_json as select * from cp.`json/" + 
testName + ".json`");
+
+final String query = "select * from `" + testName + "_json` t1 ";
+
+testBuilder()
+.sqlQuery(query)
+.ordered()
+.jsonBaselineFile("json/" + testName + ".json")
+.build()
+.run();
+
+test("drop table " + testName + "_json" );
+
+  }
+
+  @Test
+  /**
+   * Test a source json file that contains records that are maps with 
fields of all types.
+   * Some records have missing fields. CTAS should NOT skip the missing 
fields
+   */
+  public void testctas_alltypes_repeated_map_noskip() throws Exception {
+String testName = "ctas_alltypes_repeated_map";
+test("use dfs_test.tmp");
+test("alter session set store.for

[GitHub] drill pull request #514: DRILL-4694: CTAS in JSON format produces extraneous...

2016-06-06 Thread parthchandra
Github user parthchandra commented on a diff in the pull request:

https://github.com/apache/drill/pull/514#discussion_r65928727
  
--- Diff: 
exec/java-exec/src/main/codegen/templates/JsonOutputRecordWriter.java ---
@@ -61,7 +62,13 @@
 
 @Override
 public void startField() throws IOException {
+  <#if mode.prefix = "Nullable" >
--- End diff --

Dumb mistake. Thanks for catching it!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #514: DRILL-4694: CTAS in JSON format produces extraneous...

2016-07-22 Thread parthchandra
Github user parthchandra closed the pull request at:

https://github.com/apache/drill/pull/514


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---