From 2f0d0898ce975a6ff824defe85c2b7209068aae8 Mon Sep 17 00:00:00 2001
From: Hyunsik Choi <hyunsik@apache.org>
Date: Tue, 26 May 2015 16:47:16 -0700
Subject: [PATCH] TAJO-1623: INSERT INTO with wrong target columns causes NPE.

---
 .../java/org/apache/tajo/QueryTestCaseBase.java | 16 ++++++++++++----
 .../engine/planner/TestQueryValidation.java     | 17 ++++++++++++-----
 .../org/apache/tajo/plan/LogicalPlanner.java    |  6 ++++++
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/tajo-core/src/test/java/org/apache/tajo/QueryTestCaseBase.java b/tajo-core/src/test/java/org/apache/tajo/QueryTestCaseBase.java
index 8d0ff12e98..9a92e909a0 100644
--- a/tajo-core/src/test/java/org/apache/tajo/QueryTestCaseBase.java
+++ b/tajo-core/src/test/java/org/apache/tajo/QueryTestCaseBase.java
@@ -306,24 +306,32 @@ private static VerificationState verify(String query) throws PlanningException {
     return state;
   }
 
-  public void assertValidSQL(String fileName) throws PlanningException, IOException {
-    Path queryFilePath = getQueryFilePath(fileName);
-    String query = FileUtil.readTextFile(new File(queryFilePath.toUri()));
+  public void assertValidSQL(String query) throws PlanningException, IOException {
     VerificationState state = verify(query);
     if (state.getErrorMessages().size() > 0) {
       fail(state.getErrorMessages().get(0));
     }
   }
 
-  public void assertInvalidSQL(String fileName) throws PlanningException, IOException {
+  public void assertValidSQLFromFile(String fileName) throws PlanningException, IOException {
     Path queryFilePath = getQueryFilePath(fileName);
     String query = FileUtil.readTextFile(new File(queryFilePath.toUri()));
+    assertValidSQL(query);
+  }
+
+  public void assertInvalidSQL(String query) throws PlanningException, IOException {
     VerificationState state = verify(query);
     if (state.getErrorMessages().size() == 0) {
       fail(PreLogicalPlanVerifier.class.getSimpleName() + " cannot catch any verification error: " + query);
     }
   }
 
+  public void assertInvalidSQLFromFile(String fileName) throws PlanningException, IOException {
+    Path queryFilePath = getQueryFilePath(fileName);
+    String query = FileUtil.readTextFile(new File(queryFilePath.toUri()));
+    assertInvalidSQL(query);
+  }
+
   public void assertPlanError(String fileName) throws PlanningException, IOException {
     Path queryFilePath = getQueryFilePath(fileName);
     String query = FileUtil.readTextFile(new File(queryFilePath.toUri()));
diff --git a/tajo-core/src/test/java/org/apache/tajo/engine/planner/TestQueryValidation.java b/tajo-core/src/test/java/org/apache/tajo/engine/planner/TestQueryValidation.java
index b6827a2e4b..fd60a5e53f 100644
--- a/tajo-core/src/test/java/org/apache/tajo/engine/planner/TestQueryValidation.java
+++ b/tajo-core/src/test/java/org/apache/tajo/engine/planner/TestQueryValidation.java
@@ -25,19 +25,26 @@
 import java.io.IOException;
 
 public class TestQueryValidation extends QueryTestCaseBase {
+
+  @Test
+  public void testInsertWithWrongTargetColumn() throws Exception {
+    executeString("CREATE TABLE T1 (col1 int, col2 int)").close();
+    assertInvalidSQL("INSERT INTO T1 (col1, col3) select l_orderkey, l_partkey from default.lineitem");
+  }
+
   @Test
   public void testLimitClauses() throws PlanningException, IOException {
     // select * from lineitem limit 3;
-    assertValidSQL("valid_limit_1.sql");
+    assertValidSQLFromFile("valid_limit_1.sql");
 
     // select * from lineitem limit l_orderkey;
-    assertInvalidSQL("invalid_limit_1.sql");
+    assertInvalidSQLFromFile("invalid_limit_1.sql");
   }
 
   @Test
   public void testGroupByClauses() throws PlanningException, IOException {
     // select l_orderkey from lineitem group by l_orderkey;
-    assertValidSQL("valid_groupby_1.sql");
+    assertValidSQLFromFile("valid_groupby_1.sql");
 
     // select * from lineitem group by l_orderkey;
     assertPlanError("error_groupby_1.sql");
@@ -48,12 +55,12 @@ public void testGroupByClauses() throws PlanningException, IOException {
   @Test
   public void testCaseWhenExprs() throws PlanningException, IOException {
     // See TAJO-1098
-    assertInvalidSQL("invalid_casewhen_1.sql");
+    assertInvalidSQLFromFile("invalid_casewhen_1.sql");
   }
 
   @Test
   public void testUnsupportedStoreType() throws PlanningException, IOException {
     // See TAJO-1249
-    assertInvalidSQL("invalid_store_format.sql");
+    assertInvalidSQLFromFile("invalid_store_format.sql");
   }
 }
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 cec0760e6a..5571cdf0f2 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
@@ -1564,6 +1564,12 @@ private InsertNode buildInsertIntoTablePlan(PlanContext context, InsertNode inse
       Schema targetColumns = new Schema();
       for (int i = 0; i < targets.length; i++) {
         Column targetColumn = desc.getLogicalSchema().getColumn(targets[i]);
+
+        if (targetColumn == null) {
+          throw new PlanningException("column \"" + targets[i] + "\" of relation \"" +
+              desc.getName() + "\" does not exist");
+        }
+
         targetColumns.addColumn(targetColumn);
       }
       insertNode.setTargetSchema(targetColumns);