You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by hyunsik <gi...@git.apache.org> on 2015/08/08 16:22:38 UTC
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
GitHub user hyunsik opened a pull request:
https://github.com/apache/tajo/pull/680
TAJO-1748: Refine client APIs to throw specific exceptions.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hyunsik/tajo TAJO-1748
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tajo/pull/680.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 #680
----
commit 874227f26e89f0327e0e54c42a3e0af6da701f1f
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-08-07T13:12:20Z
Refined catalog APIs' exceptions.
commit 070191e871a0a3d73e093863fd7c3f4b78d32a52
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-08-07T15:49:49Z
Add unit tests for QueryClient and CatalogAdminClient.
commit e80a72223056086d08e4a10b1f1e85b40128eae2
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-08-08T03:06:49Z
Cleanup CLI errors.
commit 12052252d6b4b6057ecf2870b0493159025f9969
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-08-08T07:43:14Z
Improved TajoCli's error handling through client API v1.
commit 0989cd5da73413877c19fe0d01e0d517e6c18ec9
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-08-08T14:05:48Z
Add error handling to client v2 for failed and killed queries.
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36612511
--- Diff: tajo-client/src/main/java/org/apache/tajo/client/v2/LegacyClientDelegate.java ---
@@ -72,10 +72,20 @@ public int executeUpdate(String sql) throws TajoException {
}
@Override
- public ResultSet executeSQL(String sql) throws TajoException {
+ public ResultSet executeSQL(String sql) throws TajoException, QueryFailedException, QueryKilledException {
--- End diff --
The last two exceptions extend TajoException, so they looks not necessary.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36936509
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -503,7 +536,7 @@ public final boolean createTable(final TableDesc desc) {
}
@Override
- public boolean dropTable(String tableName) {
+ public void dropTable(String tableName) throws UndefinedTableException, InsufficientPrivilegeException {
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36609654
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -430,27 +454,32 @@ public final PartitionDescProto getPartition(final String databaseName, final St
}
@Override
- public boolean addPartitions(String databaseName, String tableName, List<PartitionDescProto> partitions
- , boolean ifNotExists) {
+ public void addPartitions(String databaseName, String tableName, List<PartitionDescProto> partitions,
+ boolean ifNotExists) throws UndefinedTableException, DuplicatePartitionException,
+ UndefinedPartitionMethodException {
--- End diff --
```UndefinedDatabaseException``` also should be handled.
In addition, ```DuplicatePartitionException``` is missed at CatalogServer.addPartitions(). It would be great if you add 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36937458
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -805,11 +851,19 @@ public final boolean containFunction(final String signature, FunctionType funcTy
}
@Override
- public final boolean alterTable(final AlterTableDesc desc) {
+ public final void alterTable(final AlterTableDesc desc) throws UndefinedTableException, DuplicateColumnException,
+ InsufficientPrivilegeException, DuplicateTableException {
try {
final BlockingInterface stub = getStub();
- return isSuccess(stub.alterTable(null, desc.getProto()));
+ final ReturnState state = stub.alterTable(null, desc.getProto());
+
+ throwsIfThisError(state, DuplicateColumnException.class);
+ throwsIfThisError(state, DuplicateTableException.class);
+ throwsIfThisError(state, DuplicateColumnException.class);
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36826443
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -343,14 +358,18 @@ public TableDesc getTableDesc(String qualifiedName) {
}
@Override
- public final PartitionMethodDesc getPartitionMethod(final String databaseName, final String tableName) {
+ public final PartitionMethodDesc getPartitionMethod(final String databaseName, final String tableName)
+ throws UndefinedPartitionMethodException, UndefinedTableException {
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36937817
--- Diff: tajo-common/src/main/proto/errors.proto ---
@@ -154,11 +157,15 @@ enum ResultCode {
CAT_UPGRADE_REQUIRED = 901; // Migration
CAT_CANNOT_CONNECT = 902; // Cannot connect metadata server
- // MetaData Connector (MDC)
- MDC_NO_MATCHED_DATATYPE = 910; // No matched data type between Tajo and connector
+ // Linked Metadata (MDC)
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36937521
--- Diff: tajo-common/src/main/java/org/apache/tajo/exception/DuplicateFunctionException.java ---
@@ -0,0 +1,34 @@
+/**
+ * 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.exception;
+
+import org.apache.tajo.error.Errors;
+import org.apache.tajo.rpc.protocolrecords.PrimitiveProtos.ReturnState;
+
+public class DuplicateFunctionException extends TajoException {
+ private static final long serialVersionUID = 3224521585413794703L;
+
+ public DuplicateFunctionException(ReturnState state) {
+ super(state);
+ }
+
+ public DuplicateFunctionException(String signature) {
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36612232
--- Diff: tajo-client/src/main/java/org/apache/tajo/client/CatalogAdminClientImpl.java ---
@@ -164,21 +166,22 @@ public TableDesc createExternalTable(final String tableName, final Schema schema
throw new RuntimeException(e);
}
- if (isThisError(res.getState(), Errors.ResultCode.DUPLICATE_TABLE)) {
- throw new DuplicateTableException(res.getState());
- }
+ throwsIfThisError(res.getState(), DuplicateTableException.class);
+ throwsIfThisError(res.getState(), InsufficientPrivilegeException.class);
+ throwsIfThisError(res.getState(), UnavailableTableLocationException.class);
ensureOk(res.getState());
return CatalogUtil.newTableDesc(res.getTable());
}
@Override
- public boolean dropTable(String tableName) throws UndefinedTableException {
- return dropTable(tableName, false);
+ public void dropTable(String tableName) throws UndefinedTableException, InsufficientPrivilegeException {
+ dropTable(tableName, false);
}
@Override
- public boolean dropTable(final String tableName, final boolean purge) throws UndefinedTableException {
+ public void dropTable(final String tableName, final boolean purge)
+ throws UndefinedTableException, InsufficientPrivilegeException {
--- End diff --
I misunderstood some codes. Please forget this comment.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36936503
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -471,31 +500,35 @@ public boolean addPartitions(String databaseName, String tableName, List<Partiti
@Override
public final Collection<FunctionDesc> getFunctions() {
List<FunctionDesc> list = new ArrayList<FunctionDesc>();
+
try {
- GetFunctionsResponse response;
- BlockingInterface stub = getStub();
- response = stub.getFunctions(null, NullProto.newBuilder().build());
- int size = response.getFunctionDescCount();
- for (int i = 0; i < size; i++) {
+ final BlockingInterface stub = getStub();
+ final GetFunctionsResponse response = stub.getFunctions(null, NullProto.newBuilder().build());
+
+ ensureOk(response.getState());
+ for (int i = 0; i < response.getFunctionDescCount(); i++) {
try {
list.add(new FunctionDesc(response.getFunctionDesc(i)));
} catch (ClassNotFoundException e) {
- LOG.error(e, e);
- return list;
+ throw new RuntimeException(e);
}
}
return list;
+
} catch (ServiceException e) {
throw new RuntimeException(e);
}
}
@Override
- public final boolean createTable(final TableDesc desc) {
+ public final void createTable(final TableDesc desc) throws DuplicateTableException, InsufficientPrivilegeException {
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36610170
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -754,26 +801,19 @@ public final FunctionDesc getFunction(final String signature, FunctionType funcT
builder.addParameterTypes(type);
}
- FunctionResponse response = null;
try {
final BlockingInterface stub = getStub();
- response = stub.getFunctionMeta(null, builder.build());
- } catch (ServiceException se) {
- throw new RuntimeException(se);
- }
-
- if (isThisError(response.getState(), ResultCode.UNDEFINED_FUNCTION)) {
- throw new UndefinedFunctionException(signature, paramTypes);
- } else if (isThisError(response.getState(), ResultCode.AMBIGUOUS_FUNCTION)) {
- throw new AmbiguousFunctionException(signature, paramTypes);
- }
+ final FunctionResponse response = stub.getFunctionMeta(null, builder.build());
--- End diff --
If there are several functions of the given name, getFunctionMeta() always throws a function which has the nearest argument among them instead of throwing ```AmbiguousFunctionException```.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36937450
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -805,11 +851,19 @@ public final boolean containFunction(final String signature, FunctionType funcTy
}
@Override
- public final boolean alterTable(final AlterTableDesc desc) {
+ public final void alterTable(final AlterTableDesc desc) throws UndefinedTableException, DuplicateColumnException,
+ InsufficientPrivilegeException, DuplicateTableException {
try {
final BlockingInterface stub = getStub();
- return isSuccess(stub.alterTable(null, desc.getProto()));
+ final ReturnState state = stub.alterTable(null, desc.getProto());
--- End diff --
I added more exceptions.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36609330
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -343,14 +358,18 @@ public TableDesc getTableDesc(String qualifiedName) {
}
@Override
- public final PartitionMethodDesc getPartitionMethod(final String databaseName, final String tableName) {
+ public final PartitionMethodDesc getPartitionMethod(final String databaseName, final String tableName)
+ throws UndefinedPartitionMethodException, UndefinedTableException {
--- End diff --
It seems that ```UndefinedDatabaseException``` can also occur.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/680#issuecomment-130586950
Thanks for updating your patch.
I have more comments.
* AbstractDBStore.getTableSpaceInfo() should throw UndefinedTablespaceException instead of TajoInternalError.
* AbstractDBStore.getTableName() should throw UndefinedTableException instead of TajoInternalError.
* AbstractDBStore.getIndexByName() should throw UndefinedIndexException instead of TajoInternalError.
* AbstractDBStore.getIndexByColumns() should throw UndefinedIndexException instead of TajoInternalError.
* AbstractDBStore.updateTableStats() should throw UndefinedTableException instead of TajoInternalError.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36609449
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -372,7 +400,8 @@ public final boolean existPartitionMethod(final String databaseName, final Strin
@Override
public final PartitionDescProto getPartition(final String databaseName, final String tableName,
- final String partitionName) throws UndefinedPartitionException {
+ final String partitionName)
+ throws UndefinedPartitionException, UndefinedPartitionMethodException {
--- End diff --
It seems that ```UndefinedDatabaseException``` and ```UndefinedTableExeption``` can also occur.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36608669
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -140,13 +143,14 @@ public final Boolean existTablespace(final String tablespaceName) {
}
@Override
- public TablespaceProto getTablespace(final String tablespaceName) {
+ public TablespaceProto getTablespace(final String tablespaceName) throws UndefinedTableException {
try {
final BlockingInterface stub = getStub();
final GetTablespaceResponse response = stub.getTablespace(null, ProtoUtil.convertString(tablespaceName));
- ensureOk(response.getState());
+ throwsIfThisError(response.getState(), UndefinedTableException.class);
--- End diff --
This method should throw UndefinedTablespaceException.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36827524
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -380,15 +409,11 @@ public final PartitionDescProto getPartition(final String databaseName, final St
.setTableName(tableName)
.setPartitionName(partitionName)
.build();
-
final GetPartitionDescResponse response = stub.getPartitionByPartitionName(null, request);
- if (ReturnStateUtil.isThisError(response.getState(), ResultCode.UNDEFINED_PARTITION)) {
- throw new UndefinedPartitionException(partitionName);
- }
-
+ throwsIfThisError(response.getState(), UndefinedPartitionMethodException.class);
+ throwsIfThisError(response.getState(), UndefinedPartitionException.class);
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36609840
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -720,15 +764,18 @@ public final boolean createFunction(final FunctionDesc funcDesc) {
}
@Override
- public final boolean dropFunction(final String signature) {
+ public final void dropFunction(final String signature) throws UndefinedFunctionException, InsufficientPrivilegeException {
--- End diff --
This line exceeds the row width limit.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36937512
--- Diff: tajo-client/src/main/java/org/apache/tajo/client/v2/ClientDelegate.java ---
@@ -31,7 +32,7 @@
int executeUpdate(String sql) throws TajoException;
- ResultSet executeSQL(String sql) throws TajoException;
+ ResultSet executeSQL(String sql) throws TajoException, QueryFailedException;
--- End diff --
It means a documentation. This exceptions will let users to know what exception should be handled.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/680#issuecomment-130632994
+1 thank you for the nice work!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36612229
--- Diff: tajo-client/src/main/java/org/apache/tajo/client/CatalogAdminClientImpl.java ---
@@ -164,21 +166,22 @@ public TableDesc createExternalTable(final String tableName, final Schema schema
throw new RuntimeException(e);
}
- if (isThisError(res.getState(), Errors.ResultCode.DUPLICATE_TABLE)) {
- throw new DuplicateTableException(res.getState());
- }
+ throwsIfThisError(res.getState(), DuplicateTableException.class);
+ throwsIfThisError(res.getState(), InsufficientPrivilegeException.class);
--- End diff --
I misunderstood some codes. Please forget this comment.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36609456
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -380,15 +409,11 @@ public final PartitionDescProto getPartition(final String databaseName, final St
.setTableName(tableName)
.setPartitionName(partitionName)
.build();
-
final GetPartitionDescResponse response = stub.getPartitionByPartitionName(null, request);
- if (ReturnStateUtil.isThisError(response.getState(), ResultCode.UNDEFINED_PARTITION)) {
- throw new UndefinedPartitionException(partitionName);
- }
-
+ throwsIfThisError(response.getState(), UndefinedPartitionMethodException.class);
+ throwsIfThisError(response.getState(), UndefinedPartitionException.class);
--- End diff --
It seems that UndefinedDatabaseException and UndefinedTableExeption can also occur.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/680#issuecomment-129375429
@hyunsik, thanks for your work. Please consider my comments.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36936522
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -720,15 +764,18 @@ public final boolean createFunction(final FunctionDesc funcDesc) {
}
@Override
- public final boolean dropFunction(final String signature) {
+ public final void dropFunction(final String signature) throws UndefinedFunctionException, InsufficientPrivilegeException {
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tajo/pull/680
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36609754
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -471,31 +500,35 @@ public boolean addPartitions(String databaseName, String tableName, List<Partiti
@Override
public final Collection<FunctionDesc> getFunctions() {
List<FunctionDesc> list = new ArrayList<FunctionDesc>();
+
try {
- GetFunctionsResponse response;
- BlockingInterface stub = getStub();
- response = stub.getFunctions(null, NullProto.newBuilder().build());
- int size = response.getFunctionDescCount();
- for (int i = 0; i < size; i++) {
+ final BlockingInterface stub = getStub();
+ final GetFunctionsResponse response = stub.getFunctions(null, NullProto.newBuilder().build());
+
+ ensureOk(response.getState());
+ for (int i = 0; i < response.getFunctionDescCount(); i++) {
try {
list.add(new FunctionDesc(response.getFunctionDesc(i)));
} catch (ClassNotFoundException e) {
- LOG.error(e, e);
- return list;
+ throw new RuntimeException(e);
}
}
return list;
+
} catch (ServiceException e) {
throw new RuntimeException(e);
}
}
@Override
- public final boolean createTable(final TableDesc desc) {
+ public final void createTable(final TableDesc desc) throws DuplicateTableException, InsufficientPrivilegeException {
--- End diff --
```UndefinedDatabaseException``` should be handled.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36936501
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -430,27 +454,32 @@ public final PartitionDescProto getPartition(final String databaseName, final St
}
@Override
- public boolean addPartitions(String databaseName, String tableName, List<PartitionDescProto> partitions
- , boolean ifNotExists) {
+ public void addPartitions(String databaseName, String tableName, List<PartitionDescProto> partitions,
+ boolean ifNotExists) throws UndefinedTableException, DuplicatePartitionException,
+ UndefinedPartitionMethodException {
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36612494
--- Diff: tajo-client/src/main/java/org/apache/tajo/client/v2/ClientDelegate.java ---
@@ -31,7 +32,7 @@
int executeUpdate(String sql) throws TajoException;
- ResultSet executeSQL(String sql) throws TajoException;
+ ResultSet executeSQL(String sql) throws TajoException, QueryFailedException;
--- End diff --
QueryFailedException extends TajoException, so it seems that it is not necessary.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36609776
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -503,7 +536,7 @@ public final boolean createTable(final TableDesc desc) {
}
@Override
- public boolean dropTable(String tableName) {
+ public void dropTable(String tableName) throws UndefinedTableException, InsufficientPrivilegeException {
--- End diff --
```UndefinedDatabaseException``` should be handled.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36608658
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -140,13 +143,14 @@ public final Boolean existTablespace(final String tablespaceName) {
}
@Override
- public TablespaceProto getTablespace(final String tablespaceName) {
+ public TablespaceProto getTablespace(final String tablespaceName) throws UndefinedTableException {
--- End diff --
This method should throw UndefinedTablespaceException.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36610932
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -805,11 +851,19 @@ public final boolean containFunction(final String signature, FunctionType funcTy
}
@Override
- public final boolean alterTable(final AlterTableDesc desc) {
+ public final void alterTable(final AlterTableDesc desc) throws UndefinedTableException, DuplicateColumnException,
+ InsufficientPrivilegeException, DuplicateTableException {
try {
final BlockingInterface stub = getStub();
- return isSuccess(stub.alterTable(null, desc.getProto()));
+ final ReturnState state = stub.alterTable(null, desc.getProto());
--- End diff --
CatalogServer.alterTable() throws only ```UndefinedTableException```. Did you add handling other exceptions for future work? Even though you did, I think that ```DuplicateTableException``` does not occur.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36612071
--- Diff: tajo-client/src/main/java/org/apache/tajo/client/CatalogAdminClientImpl.java ---
@@ -164,21 +166,22 @@ public TableDesc createExternalTable(final String tableName, final Schema schema
throw new RuntimeException(e);
}
- if (isThisError(res.getState(), Errors.ResultCode.DUPLICATE_TABLE)) {
- throw new DuplicateTableException(res.getState());
- }
+ throwsIfThisError(res.getState(), DuplicateTableException.class);
+ throwsIfThisError(res.getState(), InsufficientPrivilegeException.class);
--- End diff --
The above two exceptions should be properly thrown when TajoMasterClientService.createExternalTable() is called.
For more detailed, these exceptions should be caught at the following code (TajoMasterClientService::887), and then thrown properly to handle them here.
```
TableDesc desc = context.getGlobalEngine().getDDLExecutor().getCreateTableExecutor().create(
queryContext,
request.getName(),
null,
schema,
meta,
path.toUri(),
true,
partitionDesc,
false
);
```
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36610946
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -805,11 +851,19 @@ public final boolean containFunction(final String signature, FunctionType funcTy
}
@Override
- public final boolean alterTable(final AlterTableDesc desc) {
+ public final void alterTable(final AlterTableDesc desc) throws UndefinedTableException, DuplicateColumnException,
+ InsufficientPrivilegeException, DuplicateTableException {
try {
final BlockingInterface stub = getStub();
- return isSuccess(stub.alterTable(null, desc.getProto()));
+ final ReturnState state = stub.alterTable(null, desc.getProto());
+
+ throwsIfThisError(state, DuplicateColumnException.class);
+ throwsIfThisError(state, DuplicateTableException.class);
+ throwsIfThisError(state, DuplicateColumnException.class);
--- End diff --
```DuplicateColumnException``` is handled twice.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36937244
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -754,26 +801,19 @@ public final FunctionDesc getFunction(final String signature, FunctionType funcT
builder.addParameterTypes(type);
}
- FunctionResponse response = null;
try {
final BlockingInterface stub = getStub();
- response = stub.getFunctionMeta(null, builder.build());
- } catch (ServiceException se) {
- throw new RuntimeException(se);
- }
-
- if (isThisError(response.getState(), ResultCode.UNDEFINED_FUNCTION)) {
- throw new UndefinedFunctionException(signature, paramTypes);
- } else if (isThisError(response.getState(), ResultCode.AMBIGUOUS_FUNCTION)) {
- throw new AmbiguousFunctionException(signature, paramTypes);
- }
+ final FunctionResponse response = stub.getFunctionMeta(null, builder.build());
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36612799
--- Diff: tajo-common/src/main/java/org/apache/tajo/exception/DuplicateFunctionException.java ---
@@ -0,0 +1,34 @@
+/**
+ * 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.exception;
+
+import org.apache.tajo.error.Errors;
+import org.apache.tajo.rpc.protocolrecords.PrimitiveProtos.ReturnState;
+
+public class DuplicateFunctionException extends TajoException {
+ private static final long serialVersionUID = 3224521585413794703L;
+
+ public DuplicateFunctionException(ReturnState state) {
+ super(state);
+ }
+
+ public DuplicateFunctionException(String signature) {
--- End diff --
Please fix the indent.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/680#issuecomment-130545281
I reflected the comments and rebased.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36826146
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -140,13 +143,14 @@ public final Boolean existTablespace(final String tablespaceName) {
}
@Override
- public TablespaceProto getTablespace(final String tablespaceName) {
+ public TablespaceProto getTablespace(final String tablespaceName) throws UndefinedTableException {
try {
final BlockingInterface stub = getStub();
final GetTablespaceResponse response = stub.getTablespace(null, ProtoUtil.convertString(tablespaceName));
- ensureOk(response.getState());
+ throwsIfThisError(response.getState(), UndefinedTableException.class);
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36827502
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -359,11 +378,20 @@ public final PartitionMethodDesc getPartitionMethod(final String databaseName, f
}
@Override
- public final boolean existPartitionMethod(final String databaseName, final String tableName) {
+ public final boolean existPartitionMethod(final String databaseName, final String tableName)
+ throws UndefinedTableException {
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36612157
--- Diff: tajo-client/src/main/java/org/apache/tajo/client/CatalogAdminClientImpl.java ---
@@ -164,21 +166,22 @@ public TableDesc createExternalTable(final String tableName, final Schema schema
throw new RuntimeException(e);
}
- if (isThisError(res.getState(), Errors.ResultCode.DUPLICATE_TABLE)) {
- throw new DuplicateTableException(res.getState());
- }
+ throwsIfThisError(res.getState(), DuplicateTableException.class);
+ throwsIfThisError(res.getState(), InsufficientPrivilegeException.class);
+ throwsIfThisError(res.getState(), UnavailableTableLocationException.class);
ensureOk(res.getState());
return CatalogUtil.newTableDesc(res.getTable());
}
@Override
- public boolean dropTable(String tableName) throws UndefinedTableException {
- return dropTable(tableName, false);
+ public void dropTable(String tableName) throws UndefinedTableException, InsufficientPrivilegeException {
+ dropTable(tableName, false);
}
@Override
- public boolean dropTable(final String tableName, final boolean purge) throws UndefinedTableException {
+ public void dropTable(final String tableName, final boolean purge)
+ throws UndefinedTableException, InsufficientPrivilegeException {
--- End diff --
Similar to the case of the above ```createExternalTable```, these exceptions should be thrown properly at TajoMasterClientService.dropTable().
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36937509
--- Diff: tajo-client/src/main/java/org/apache/tajo/client/v2/LegacyClientDelegate.java ---
@@ -72,10 +72,20 @@ public int executeUpdate(String sql) throws TajoException {
}
@Override
- public ResultSet executeSQL(String sql) throws TajoException {
+ public ResultSet executeSQL(String sql) throws TajoException, QueryFailedException, QueryKilledException {
--- End diff --
It means a documentation. This exceptions will let users to know what exception should be handled.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36827516
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -372,7 +400,8 @@ public final boolean existPartitionMethod(final String databaseName, final Strin
@Override
public final PartitionDescProto getPartition(final String databaseName, final String tableName,
- final String partitionName) throws UndefinedPartitionException {
+ final String partitionName)
+ throws UndefinedPartitionException, UndefinedPartitionMethodException {
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36826136
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -140,13 +143,14 @@ public final Boolean existTablespace(final String tablespaceName) {
}
@Override
- public TablespaceProto getTablespace(final String tablespaceName) {
+ public TablespaceProto getTablespace(final String tablespaceName) throws UndefinedTableException {
--- End diff --
fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/680#issuecomment-130588527
I reflected the comments. Thank you for careful review!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36612885
--- Diff: tajo-common/src/main/proto/errors.proto ---
@@ -154,11 +157,15 @@ enum ResultCode {
CAT_UPGRADE_REQUIRED = 901; // Migration
CAT_CANNOT_CONNECT = 902; // Cannot connect metadata server
- // MetaData Connector (MDC)
- MDC_NO_MATCHED_DATATYPE = 910; // No matched data type between Tajo and connector
+ // Linked Metadata (MDC)
--- End diff --
MDC should be changed to LMD.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1748: Refine client APIs to throw specific...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/680#discussion_r36609336
--- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
@@ -359,11 +378,20 @@ public final PartitionMethodDesc getPartitionMethod(final String databaseName, f
}
@Override
- public final boolean existPartitionMethod(final String databaseName, final String tableName) {
+ public final boolean existPartitionMethod(final String databaseName, final String tableName)
+ throws UndefinedTableException {
--- End diff --
It seems that ```UndefinedDatabaseException``` can also occur.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---