Repository: tajo
Updated Branches:
  refs/heads/branch-0.10.1 f9a531c0f -> 9d331f9af


TAJO-1556: "insert into select" with reordered column list does not work.

Signed-off-by: Jihoon Son <[email protected]>


Project: http://git-wip-us.apache.org/repos/asf/tajo/repo
Commit: http://git-wip-us.apache.org/repos/asf/tajo/commit/9d331f9a
Tree: http://git-wip-us.apache.org/repos/asf/tajo/tree/9d331f9a
Diff: http://git-wip-us.apache.org/repos/asf/tajo/diff/9d331f9a

Branch: refs/heads/branch-0.10.1
Commit: 9d331f9af11d77f16d332029ad7debe932689cf2
Parents: f9a531c
Author: Yongjin Choi <[email protected]>
Authored: Wed May 6 18:47:38 2015 +0900
Committer: Jihoon Son <[email protected]>
Committed: Wed May 6 18:47:38 2015 +0900

----------------------------------------------------------------------
 CHANGES                                         |  3 +++
 .../java/org/apache/tajo/catalog/Schema.java    | 12 ++++++---
 .../tajo/engine/planner/TestLogicalPlanner.java | 23 +++++++++++++++--
 .../tajo/engine/query/TestInsertQuery.java      | 19 ++++++++++++++
 .../TestInsertQuery/nation_diff_col_order.ddl   |  1 +
 .../testInsertWithDifferentColumnOrder.sql      |  1 +
 .../testInsertWithDifferentColumnOrder.result   | 27 ++++++++++++++++++++
 .../org/apache/tajo/plan/LogicalPlanner.java    |  9 ++++---
 8 files changed, 86 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tajo/blob/9d331f9a/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index a184d6c..de1144e 100644
--- a/CHANGES
+++ b/CHANGES
@@ -33,6 +33,9 @@ Release 0.10.1 - unreleased
 
   BUG FIXES
 
+    TAJO-1556: "insert into select" with reordered column list does not work.
+    (Contributed by Yongjin Choi, Committed by jihoon)
+
     TAJO-1534: DelimitedTextFile return null instead of a NullDatum. (jinho)
 
     TAJO-1574: Fix NPE on natural join. 

http://git-wip-us.apache.org/repos/asf/tajo/blob/9d331f9a/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/Schema.java
----------------------------------------------------------------------
diff --git 
a/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/Schema.java
 
b/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/Schema.java
index 71c1b01..078f8a9 100644
--- 
a/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/Schema.java
+++ 
b/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/Schema.java
@@ -121,13 +121,19 @@ public class Schema implements ProtoObject<SchemaProto>, 
Cloneable, GsonObject {
   }
 
   public Column getColumn(Column column) {
+    int idx = getIndex(column);
+    return idx >= 0 ? fields.get(idx) : null;
+  }
+
+  public int getIndex(Column column) {
     if (!contains(column)) {
-      return null;
+      return -1;
     }
+
     if (column.hasQualifier()) {
-      return fields.get(fieldsByQualifiedName.get(column.getQualifiedName()));
+      return fieldsByQualifiedName.get(column.getQualifiedName());
     } else {
-      return fields.get(fieldsByName.get(column.getSimpleName()).get(0));
+      return fieldsByName.get(column.getSimpleName()).get(0);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/tajo/blob/9d331f9a/tajo-core/src/test/java/org/apache/tajo/engine/planner/TestLogicalPlanner.java
----------------------------------------------------------------------
diff --git 
a/tajo-core/src/test/java/org/apache/tajo/engine/planner/TestLogicalPlanner.java
 
b/tajo-core/src/test/java/org/apache/tajo/engine/planner/TestLogicalPlanner.java
index 0b59bc7..af0aa6a 100644
--- 
a/tajo-core/src/test/java/org/apache/tajo/engine/planner/TestLogicalPlanner.java
+++ 
b/tajo-core/src/test/java/org/apache/tajo/engine/planner/TestLogicalPlanner.java
@@ -1106,7 +1106,7 @@ public class TestLogicalPlanner {
   // Table descriptions
   //
   // employee (name text, empid int4, deptname text)
-  // dept (deptname text, nameger text)
+  // dept (deptname text, manager text)
   // score (deptname text, score inet4)
 
   static final String [] insertStatements = {
@@ -1115,7 +1115,8 @@ public class TestLogicalPlanner {
       "insert into employee (name, deptname) select * from dept",           // 
2
       "insert into location '/tmp/data' select name, empid from employee",  // 
3
       "insert overwrite into employee (name, deptname) select * from dept", // 
4
-      "insert overwrite into LOCATION '/tmp/data' select * from dept"       // 
5
+      "insert overwrite into LOCATION '/tmp/data' select * from dept",      // 
5
+      "insert into employee (deptname, name) select deptname, manager from 
dept"  // 6
   };
 
   @Test
@@ -1198,6 +1199,24 @@ public class TestLogicalPlanner {
     assertTrue(insertNode.hasPath());
   }
 
+  @Test
+  public final void testInsertInto6() throws PlanningException {
+    QueryContext qc = new QueryContext(util.getConfiguration(), session);
+
+    Expr expr = sqlAnalyzer.parse(insertStatements[6]);
+    LogicalPlan plan = planner.createPlan(qc, expr);
+    assertEquals(1, plan.getQueryBlocks().size());
+    InsertNode insertNode = getInsertNode(plan);
+
+    ProjectionNode subquery = insertNode.getChild();
+    Target[] targets = subquery.getTargets();
+    // targets MUST be manager, NULL as empid, deptname
+    assertEquals(targets[0].getNamedColumn().getQualifiedName(), 
"default.dept.manager");
+    assertEquals(targets[1].getAlias(), "empid");
+    assertEquals(targets[1].getEvalTree().getType(), EvalType.CONST);
+    assertEquals(targets[2].getNamedColumn().getQualifiedName(), 
"default.dept.deptname");
+  }
+
   private static InsertNode getInsertNode(LogicalPlan plan) {
     LogicalRootNode root = plan.getRootBlock().getRoot();
     assertEquals(NodeType.INSERT, root.getChild().getType());

http://git-wip-us.apache.org/repos/asf/tajo/blob/9d331f9a/tajo-core/src/test/java/org/apache/tajo/engine/query/TestInsertQuery.java
----------------------------------------------------------------------
diff --git 
a/tajo-core/src/test/java/org/apache/tajo/engine/query/TestInsertQuery.java 
b/tajo-core/src/test/java/org/apache/tajo/engine/query/TestInsertQuery.java
index 0799d22..72cbf87 100644
--- a/tajo-core/src/test/java/org/apache/tajo/engine/query/TestInsertQuery.java
+++ b/tajo-core/src/test/java/org/apache/tajo/engine/query/TestInsertQuery.java
@@ -817,4 +817,23 @@ public class TestInsertQuery extends QueryTestCaseBase {
     assertNotNull(resultDatas);
     assertEquals(expected, resultDatas);
   }
+
+  @Test
+  public final void testInsertWithDifferentColumnOrder() throws Exception {
+    ResultSet res = executeFile("nation_diff_col_order.ddl");
+    res.close();
+
+    CatalogService catalog = testingCluster.getMaster().getCatalog();
+    assertTrue(catalog.existsTable(getCurrentDatabase(), "nation_diff"));
+
+    try {
+      res = executeFile("testInsertWithDifferentColumnOrder.sql");
+      res.close();
+
+      res = executeString("select * from nation_diff");
+      assertResultSet(res);
+    } finally {
+      executeString("drop table nation_diff purge;");
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/tajo/blob/9d331f9a/tajo-core/src/test/resources/queries/TestInsertQuery/nation_diff_col_order.ddl
----------------------------------------------------------------------
diff --git 
a/tajo-core/src/test/resources/queries/TestInsertQuery/nation_diff_col_order.ddl
 
b/tajo-core/src/test/resources/queries/TestInsertQuery/nation_diff_col_order.ddl
new file mode 100644
index 0000000..6998304
--- /dev/null
+++ 
b/tajo-core/src/test/resources/queries/TestInsertQuery/nation_diff_col_order.ddl
@@ -0,0 +1 @@
+create table nation_diff (n_nationkey int8, n_name text, n_regionkey int8, 
n_comment text);
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tajo/blob/9d331f9a/tajo-core/src/test/resources/queries/TestInsertQuery/testInsertWithDifferentColumnOrder.sql
----------------------------------------------------------------------
diff --git 
a/tajo-core/src/test/resources/queries/TestInsertQuery/testInsertWithDifferentColumnOrder.sql
 
b/tajo-core/src/test/resources/queries/TestInsertQuery/testInsertWithDifferentColumnOrder.sql
new file mode 100644
index 0000000..ad360f9
--- /dev/null
+++ 
b/tajo-core/src/test/resources/queries/TestInsertQuery/testInsertWithDifferentColumnOrder.sql
@@ -0,0 +1 @@
+insert overwrite into nation_diff (n_comment, n_name) select n_comment, n_name 
from default.nation;
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tajo/blob/9d331f9a/tajo-core/src/test/resources/results/TestInsertQuery/testInsertWithDifferentColumnOrder.result
----------------------------------------------------------------------
diff --git 
a/tajo-core/src/test/resources/results/TestInsertQuery/testInsertWithDifferentColumnOrder.result
 
b/tajo-core/src/test/resources/results/TestInsertQuery/testInsertWithDifferentColumnOrder.result
new file mode 100644
index 0000000..4cd3b81
--- /dev/null
+++ 
b/tajo-core/src/test/resources/results/TestInsertQuery/testInsertWithDifferentColumnOrder.result
@@ -0,0 +1,27 @@
+n_nationkey,n_name,n_regionkey,n_comment
+-------------------------------
+null,ALGERIA,null, haggle. carefully final deposits detect slyly agai
+null,ARGENTINA,null,al foxes promise slyly according to the regular accounts. 
bold requests alon
+null,BRAZIL,null,y alongside of the pending deposits. carefully special 
packages are about the ironic forges. slyly special 
+null,CANADA,null,eas hang ironic, silent packages. slyly regular packages are 
furiously over the tithes. fluffily bold
+null,EGYPT,null,y above the carefully unusual theodolites. final dugouts are 
quickly across the furiously regular d
+null,ETHIOPIA,null,ven packages wake quickly. regu
+null,FRANCE,null,refully final requests. regular, ironi
+null,GERMANY,null,l platelets. regular accounts x-ray: unusual, regular acco
+null,INDIA,null,ss excuses cajole slyly across the packages. deposits print 
aroun
+null,INDONESIA,null, slyly express asymptotes. regular deposits haggle slyly. 
carefully ironic hockey players sleep blithely. carefull
+null,IRAN,null,efully alongside of the slyly final dependencies. 
+null,IRAQ,null,nic deposits boost atop the quickly final requests? quickly 
regula
+null,JAPAN,null,ously. final, express gifts cajole a
+null,JORDAN,null,ic deposits are blithely about the carefully regular pa
+null,KENYA,null, pending excuses haggle furiously deposits. pending, express 
pinto beans wake fluffily past t
+null,MOROCCO,null,rns. blithely bold courts among the closely regular packages 
use furiously bold platelets?
+null,MOZAMBIQUE,null,s. ironic, unusual asymptotes wake blithely r
+null,PERU,null,platelets. blithely pending dependencies use fluffily across 
the even pinto beans. carefully silent accoun
+null,CHINA,null,c dependencies. furiously express notornis sleep slyly regular 
accounts. ideas sleep. depos
+null,ROMANIA,null,ular asymptotes are about the furious multipliers. express 
dependencies nag above the ironically ironic account
+null,SAUDI ARABIA,null,ts. silent requests haggle. closely express packages 
sleep across the blithely
+null,VIETNAM,null,hely enticingly express accounts. even, final 
+null,RUSSIA,null, requests against the platelets use never according to the 
quickly regular pint
+null,UNITED KINGDOM,null,eans boost carefully special requests. accounts are. 
carefull
+null,UNITED STATES,null,y final packages. slow foxes cajole quickly. quickly 
silent platelets breach ironic accounts. unusual pinto be
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tajo/blob/9d331f9a/tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java
----------------------------------------------------------------------
diff --git a/tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java 
b/tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java
index 14fea08..a262100 100644
--- a/tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java
+++ b/tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java
@@ -1514,7 +1514,7 @@ public class LogicalPlanner extends 
BaseAlgebraVisitor<LogicalPlanner.PlanContex
     // we use only a sequence of preceding columns of target table's schema
     // as target columns.
     //
-    // For example, consider a target table and an 'insert into' query are 
give as follows:
+    // For example, consider a target table and an 'insert into' query are 
given as follows:
     //
     // CREATE TABLE TB1                  (col1 int,  col2 int, col3 long);
     //                                      ||          ||
@@ -1576,11 +1576,12 @@ public class LogicalPlanner extends 
BaseAlgebraVisitor<LogicalPlanner.PlanContex
       // Modifying projected columns by adding NULL constants
       // It is because that table appender does not support target columns to 
be written.
       List<Target> targets = TUtil.newList();
-      for (int i = 0, j = 0; i < tableSchema.size(); i++) {
+      for (int i = 0; i < tableSchema.size(); i++) {
         Column column = tableSchema.getColumn(i);
 
-        if(targetColumns.contains(column) && j < 
projectionNode.getTargets().length) {
-          targets.add(projectionNode.getTargets()[j++]);
+        int idxInProjectionNode = targetColumns.getIndex(column);
+        if (idxInProjectionNode >= 0 && idxInProjectionNode < 
projectionNode.getTargets().length) {
+          targets.add(projectionNode.getTargets()[idxInProjectionNode]);
         } else {
           targets.add(new Target(new ConstEval(NullDatum.get()), 
column.getSimpleName()));
         }

Reply via email to