tajo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hyun...@apache.org
Subject git commit: TAJO-360: If there is no matched function, catalog causes NPE. (hyoungjunkim via hyunsik)
Date Wed, 22 Jan 2014 03:18:24 GMT
Updated Branches:
  refs/heads/master 9a9d6eaf4 -> 358dbace5


TAJO-360: If there is no matched function, catalog causes NPE. (hyoungjunkim via hyunsik)


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

Branch: refs/heads/master
Commit: 358dbace5b48cfbc139b46c96752a66b0f817908
Parents: 9a9d6ea
Author: Hyunsik Choi <hyunsik@apache.org>
Authored: Wed Jan 22 12:17:29 2014 +0900
Committer: Hyunsik Choi <hyunsik@apache.org>
Committed: Wed Jan 22 12:17:57 2014 +0900

----------------------------------------------------------------------
 CHANGES.txt                                     |  3 ++
 .../tajo/catalog/AbstractCatalogClient.java     | 19 ++++++++---
 .../org/apache/tajo/catalog/CatalogServer.java  | 16 +++++----
 .../org/apache/tajo/catalog/TestCatalog.java    | 10 ++++--
 .../exception/UndefinedFunctionException.java   | 34 --------------------
 .../tajo/engine/planner/ExprAnnotator.java      | 12 +++----
 .../tajo/engine/eval/TestSQLExpression.java     |  6 ++++
 .../TestStringOperatorsAndFunctions.java        |  2 ++
 .../org/apache/tajo/rpc/ServerCallable.java     |  5 ++-
 9 files changed, 51 insertions(+), 56 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/358dbace/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 7999e5b..e4feb6a 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -222,6 +222,9 @@ Release 0.8.0 - unreleased
 
   BUG FIXES
 
+    TAJO-360: If there is no matched function, catalog causes NPE.
+    (hyoungjunkim via hyunsik)
+
     TAJO-537: After TAJO-522, still OutOfMemoryError: unable to create new
     native thread. (Min Zhou  via hyunsik)
 

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/358dbace/tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java
----------------------------------------------------------------------
diff --git a/tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java
b/tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java
index 9241806..322faed 100644
--- a/tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java
+++ b/tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java
@@ -22,6 +22,7 @@ import com.google.protobuf.ServiceException;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.tajo.catalog.CatalogProtocol.CatalogProtocolService;
+import org.apache.tajo.catalog.exception.NoSuchFunctionException;
 import org.apache.tajo.catalog.proto.CatalogProtos.*;
 import org.apache.tajo.common.TajoDataTypes.DataType;
 import org.apache.tajo.conf.TajoConf;
@@ -316,17 +317,25 @@ public abstract class AbstractCatalogClient implements CatalogService
{
       builder.addParameterTypes(type);
     }
 
-    FunctionDescProto descProto;
+    FunctionDescProto descProto = null;
     try {
       descProto = new ServerCallable<FunctionDescProto>(conf, catalogServerAddr, CatalogProtocol.class,
false) {
         public FunctionDescProto call(NettyClientBase client) throws ServiceException {
-          CatalogProtocolService.BlockingInterface stub = getStub(client);
-          return stub.getFunctionMeta(null, builder.build());
+          try {
+            CatalogProtocolService.BlockingInterface stub = getStub(client);
+            return stub.getFunctionMeta(null, builder.build());
+          } catch (NoSuchFunctionException e) {
+            abort();
+            throw e;
+          }
         }
       }.withRetries();
     } catch (ServiceException e) {
       LOG.error(e.getMessage(), e);
-      return null;
+    }
+
+    if (descProto == null) {
+      throw new NoSuchFunctionException(signature);
     }
     if(descProto == null) {
       LOG.error("No matched function:" + signature + "," + funcType + "," + paramTypes);
@@ -336,7 +345,7 @@ public abstract class AbstractCatalogClient implements CatalogService
{
       return new FunctionDesc(descProto);
     } catch (ClassNotFoundException e) {
       LOG.error(e);
-      return null;
+      throw new NoSuchFunctionException(signature);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/358dbace/tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java
----------------------------------------------------------------------
diff --git a/tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java
b/tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java
index d14a3a9..38686e3 100644
--- a/tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java
+++ b/tajo-catalog/tajo-catalog-server/src/main/java/org/apache/tajo/catalog/CatalogServer.java
@@ -447,7 +447,7 @@ public class CatalogServer extends AbstractService {
           }
         }
       }
-      return null;
+      throw new NoSuchFunctionException(signature);
     }
 
     private FunctionDescProto findFunction(String signature, FunctionType type, List<DataType>
params) {
@@ -458,7 +458,7 @@ public class CatalogServer extends AbstractService {
           }
         }
       }
-      return null;
+      throw new NoSuchFunctionException(signature);
     }
 
     private FunctionDescProto findFunction(FunctionDescProto target) {
@@ -471,9 +471,13 @@ public class CatalogServer extends AbstractService {
       FunctionSignature signature = FunctionSignature.create(funcDesc);
 
       if (functions.containsKey(funcDesc.getSignature())) {
-        FunctionDescProto found = findFunction(funcDesc);
-        if (found != null) {
-          throw new AlreadyExistsFunctionException(signature.toString());
+        try {
+          FunctionDescProto found = findFunction(funcDesc);
+          if (found != null) {
+            throw new AlreadyExistsFunctionException(signature.toString());
+          }
+        } catch (NoSuchFunctionException e) {
+          //create function
         }
       }
 
@@ -508,7 +512,7 @@ public class CatalogServer extends AbstractService {
               request.getParameterTypesList());
           return desc;
         }
-        return null;
+        throw new NoSuchFunctionException(request.getSignature());
       } else {
         FunctionDescProto function = findFunction(request.getSignature(), request.getParameterTypesList());
         return function;

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/358dbace/tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java
----------------------------------------------------------------------
diff --git a/tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java
b/tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java
index 00417f8..a562531 100644
--- a/tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java
+++ b/tajo-catalog/tajo-catalog-server/src/test/java/org/apache/tajo/catalog/TestCatalog.java
@@ -19,6 +19,7 @@
 package org.apache.tajo.catalog;
 
 import org.apache.hadoop.fs.Path;
+import org.apache.tajo.catalog.exception.NoSuchFunctionException;
 import org.apache.tajo.catalog.function.Function;
 import org.apache.tajo.catalog.partition.PartitionDesc;
 import org.apache.tajo.catalog.partition.Specifier;
@@ -88,7 +89,7 @@ public class TestCatalog {
     assertFalse(catalog.existsTable("getTable"));
 	}
 	
-	@Test(expected = Throwable.class)
+	@Test
 	public void testAddTableNoName() throws Exception {
 	  schema1 = new Schema();
     schema1.addColumn(FieldName1, Type.BLOB);
@@ -99,7 +100,7 @@ public class TestCatalog {
 	  TableDesc desc = new TableDesc();
 	  desc.setMeta(info);
 	  
-	  catalog.addTable(desc);
+	  assertFalse(catalog.addTable(desc));
 	}
 
   static IndexDesc desc1;
@@ -189,6 +190,11 @@ public class TestCatalog {
 		assertEquals(retrived.getFuncType(),FunctionType.UDF);
 	}
 
+  @Test(expected = NoSuchFunctionException.class)
+  public final void testSuchFunctionException() throws Exception {
+    catalog.getFunction("test123", CatalogUtil.newSimpleDataTypeArray(Type.INT4));
+  }
+
   @Test
   public final void testDropFunction() throws Exception {
     assertFalse(catalog.containFunction("test3", CatalogUtil.newSimpleDataTypeArray(Type.INT4)));

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/358dbace/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/exception/UndefinedFunctionException.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/exception/UndefinedFunctionException.java
b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/exception/UndefinedFunctionException.java
deleted file mode 100644
index 36d9c5d..0000000
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/exception/UndefinedFunctionException.java
+++ /dev/null
@@ -1,34 +0,0 @@
-/**
- * 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.tajo.engine.exception;
-
-
-public class UndefinedFunctionException extends InvalidQueryException {
-	private static final long serialVersionUID = 113593927391549716L;
-
-	/**
-	 * @param signature
-	 */
-	public UndefinedFunctionException(String signature) {
-		super("Error: call to undefined function "+signature+"()");	
-	}
-}

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/358dbace/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprAnnotator.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprAnnotator.java
b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprAnnotator.java
index 545caa2..ffa8f7a 100644
--- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprAnnotator.java
+++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprAnnotator.java
@@ -23,11 +23,11 @@ import org.apache.tajo.catalog.CatalogService;
 import org.apache.tajo.catalog.CatalogUtil;
 import org.apache.tajo.catalog.Column;
 import org.apache.tajo.catalog.FunctionDesc;
+import org.apache.tajo.catalog.exception.NoSuchFunctionException;
 import org.apache.tajo.catalog.proto.CatalogProtos;
 import org.apache.tajo.common.TajoDataTypes;
 import org.apache.tajo.datum.*;
 import org.apache.tajo.engine.eval.*;
-import org.apache.tajo.engine.exception.UndefinedFunctionException;
 import org.apache.tajo.engine.function.AggFunction;
 import org.apache.tajo.engine.function.GeneralFunction;
 import org.apache.tajo.engine.planner.logical.NodeType;
@@ -390,12 +390,12 @@ public class ExprAnnotator extends BaseAlgebraVisitor<ExprAnnotator.Context,
Eva
     stack.pop(); // <--- Pop
 
     if (!catalog.containFunction(expr.getSignature(), paramTypes)) {
-      throw new UndefinedFunctionException(CatalogUtil.getCanonicalName(expr.getSignature(),
paramTypes));
+      throw new NoSuchFunctionException(CatalogUtil.getCanonicalName(expr.getSignature(),
paramTypes));
     }
 
     FunctionDesc funcDesc = catalog.getFunction(expr.getSignature(), paramTypes);
     if (funcDesc == null) {
-      throw new UndefinedFunctionException(CatalogUtil.getCanonicalName(expr.getSignature(),
paramTypes));
+      throw new NoSuchFunctionException(CatalogUtil.getCanonicalName(expr.getSignature(),
paramTypes));
     }
     try {
     CatalogProtos.FunctionType functionType = funcDesc.getFuncType();
@@ -429,7 +429,7 @@ public class ExprAnnotator extends BaseAlgebraVisitor<ExprAnnotator.Context,
Eva
     FunctionDesc countRows = catalog.getFunction("count", CatalogProtos.FunctionType.AGGREGATION,
         new TajoDataTypes.DataType[] {});
     if (countRows == null) {
-      throw new UndefinedFunctionException(CatalogUtil.
+      throw new NoSuchFunctionException(CatalogUtil.
           getCanonicalName(countRows.getSignature(), new TajoDataTypes.DataType[]{}));
     }
 
@@ -439,7 +439,7 @@ public class ExprAnnotator extends BaseAlgebraVisitor<ExprAnnotator.Context,
Eva
       return new AggregationFunctionCallEval(countRows, (AggFunction) countRows.newInstance(),
           new EvalNode[] {});
     } catch (InternalException e) {
-      throw new UndefinedFunctionException(CatalogUtil.
+      throw new NoSuchFunctionException(CatalogUtil.
           getCanonicalName(countRows.getSignature(), new TajoDataTypes.DataType[]{}));
     }
   }
@@ -462,7 +462,7 @@ public class ExprAnnotator extends BaseAlgebraVisitor<ExprAnnotator.Context,
Eva
     }
 
     if (!catalog.containFunction(setFunction.getSignature(), functionType, paramTypes)) {
-      throw new UndefinedFunctionException(CatalogUtil. getCanonicalName(setFunction.getSignature(),
paramTypes));
+      throw new NoSuchFunctionException(CatalogUtil. getCanonicalName(setFunction.getSignature(),
paramTypes));
     }
 
     FunctionDesc funcDesc = catalog.getFunction(setFunction.getSignature(), functionType,
paramTypes);

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/358dbace/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/eval/TestSQLExpression.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/eval/TestSQLExpression.java
b/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/eval/TestSQLExpression.java
index f5d9042..6520f9c 100644
--- a/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/eval/TestSQLExpression.java
+++ b/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/eval/TestSQLExpression.java
@@ -19,6 +19,7 @@
 package org.apache.tajo.engine.eval;
 
 import org.apache.tajo.catalog.Schema;
+import org.apache.tajo.catalog.exception.NoSuchFunctionException;
 import org.apache.tajo.datum.TimestampDatum;
 import org.junit.Test;
 
@@ -28,6 +29,11 @@ import static org.apache.tajo.common.TajoDataTypes.Type.TEXT;
 
 public class TestSQLExpression extends ExprTestBase {
 
+  @Test(expected = NoSuchFunctionException.class)
+  public void testNoSuchFunction() throws IOException {
+    testSimpleEval("select test123('abc') col1 ", new String[]{"abc"});
+  }
+
   @Test
   public void testCast() throws IOException {
     testSimpleEval("select cast (1 as char)", new String[] {"1"});

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/358dbace/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/function/TestStringOperatorsAndFunctions.java
----------------------------------------------------------------------
diff --git a/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/function/TestStringOperatorsAndFunctions.java
b/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/function/TestStringOperatorsAndFunctions.java
index 0237cf0..f1d0ce3 100644
--- a/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/function/TestStringOperatorsAndFunctions.java
+++ b/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/function/TestStringOperatorsAndFunctions.java
@@ -21,12 +21,14 @@ package org.apache.tajo.engine.function;
 
 import org.apache.commons.lang.StringEscapeUtils;
 import org.apache.tajo.catalog.Schema;
+import org.apache.tajo.catalog.exception.NoSuchFunctionException;
 import org.apache.tajo.engine.eval.ExprTestBase;
 import org.junit.Test;
 
 import java.io.IOException;
 
 import static org.apache.tajo.common.TajoDataTypes.Type.*;
+import static org.junit.Assert.fail;
 
 public class TestStringOperatorsAndFunctions extends ExprTestBase {
 

http://git-wip-us.apache.org/repos/asf/incubator-tajo/blob/358dbace/tajo-rpc/src/main/java/org/apache/tajo/rpc/ServerCallable.java
----------------------------------------------------------------------
diff --git a/tajo-rpc/src/main/java/org/apache/tajo/rpc/ServerCallable.java b/tajo-rpc/src/main/java/org/apache/tajo/rpc/ServerCallable.java
index 7bcac36..143c9f7 100644
--- a/tajo-rpc/src/main/java/org/apache/tajo/rpc/ServerCallable.java
+++ b/tajo-rpc/src/main/java/org/apache/tajo/rpc/ServerCallable.java
@@ -78,8 +78,7 @@ public abstract class ServerCallable<T> {
    *
    * @param <T> the type of the return value
    * @return an object of type T
-   * @throws java.io.IOException if a remote or network exception occurs
-   * @throws RuntimeException other unspecified error
+   * @throws com.google.protobuf.ServiceException if a remote or network exception occurs
    */
   public T withRetries() throws ServiceException {
     //TODO configurable
@@ -105,7 +104,7 @@ public abstract class ServerCallable<T> {
           throw new ServiceException(t.getMessage(), t);
         }
         if (tries == numRetries - 1) {
-          throw new RetriesExhaustedException(tries, exceptions);
+          throw new ServiceException("Giving up after tries=" + tries, t);
         }
       } finally {
         afterCall();


Mime
View raw message