You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tajo.apache.org by hy...@apache.org on 2014/01/22 04:18:24 UTC
git commit: TAJO-360: If there is no matched function,
catalog causes NPE. (hyoungjunkim via hyunsik)
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 <hy...@apache.org>
Authored: Wed Jan 22 12:17:29 2014 +0900
Committer: Hyunsik Choi <hy...@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();