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/07/02 13:56:31 UTC

[GitHub] tajo pull request: TAJO-1670: Refactor client errors and exception...

GitHub user hyunsik opened a pull request:

    https://github.com/apache/tajo/pull/621

    TAJO-1670: Refactor client errors and exceptions. 

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/hyunsik/tajo TAJO-1670

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/tajo/pull/621.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 #621
    
----
commit 47cc1d44fd1b038050745a08cfd2f18d1a1b5e2a
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-06-29T05:40:06Z

    Initial work

commit d58ffaded8b51f9d82837899343d5dc8b4fca971
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-06-29T12:21:10Z

    Add initial error code.

commit ec34cf8abb3ade2ad4caf27a2f6cbf74f7debc48
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-06-29T12:29:19Z

    Added ErrorUtil.

commit 4717446702290f4575662ba41eba6389fa515999
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-06-29T12:29:48Z

    Merge branch 'errors' of github.com:hyunsik/tajo into errors

commit f1a1e7d096e568cefa6f48f8ba7195eb3c7ffd05
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-06-30T22:15:25Z

    Initial work for new error propagation.

commit 0f0ffd80fefb20a77b69be2bae619161d461fb60
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-07-01T17:16:33Z

    Fixed many exceptions.

commit 4402c6818e9986650c4cec763b5ecc3ae5eab572
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-07-01T23:15:17Z

    Fixed many bugs.

commit 5c55013511433452ae8751ae4fd669696bea218a
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-07-01T23:46:52Z

    Fixed all unit tests.

commit 2beb04bbd1ef7f4ccff4cd5555c6126369d28c4d
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-07-02T00:09:47Z

    Add SQLExceptionUtil and SQLState system.

commit 305ed51f58c3590a4dccb67cbe307a9afb6549cd
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-07-02T04:22:56Z

    Improved client API.

commit 16a8d25acf49d93848c5b3fc16beecf8c1759dab
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-07-02T09:14:02Z

    Refactor client APIs to throw SQLException and fixed all unit tests.

commit aa4404fd0fceabf8495dbc591a31fce0e78ed17d
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-07-02T09:15:07Z

    Removed duplicate exception.

commit 2376e3002f3a329302a3fd9fe7ae509582b75c5e
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-07-02T09:51:05Z

    Applied ResponseState to all client APIs.

commit 254c044f190a7a37130716969c487cbb1a250f96
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-07-02T11:54:09Z

    Fixed method names.

commit de6a445e262e1c9246a9c8af0c234773e8e41612
Author: Hyunsik Choi <hy...@apache.org>
Date:   2015-07-02T11:56:00Z

    Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-1670
    
    Conflicts:
    	tajo-core/src/test/java/org/apache/tajo/ws/rs/resources/TestQueryResultResource.java

----


---
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-1670: Refactor client errors and exception...

Posted by blrunner <gi...@git.apache.org>.
Github user blrunner commented on the pull request:

    https://github.com/apache/tajo/pull/621#issuecomment-120697936
  
    Thanks you for your great work.
    Could you rebase this patch against the master branch?
    I failed to merge it with following messages.
    
    {code:xml}
    $ git pull https://github.com/hyunsik/tajo TAJO-1670
    remote: Counting objects: 1050, done.
    remote: Total 1050 (delta 392), reused 392 (delta 392), pack-reused 657
    Receiving objects: 100% (1050/1050), 195.43 KiB | 189.00 KiB/s, done.
    Resolving deltas: 100% (490/490), completed with 153 local objects.
    From https://github.com/hyunsik/tajo
     * branch            TAJO-1670  -> FETCH_HEAD
    Removing tajo-plan/src/main/java/org/apache/tajo/plan/logical/NoSuchColumnException.java
    Auto-merging tajo-core/src/test/java/org/apache/tajo/ws/rs/resources/TestQueryResultResource.java
    Auto-merging tajo-core/src/test/java/org/apache/tajo/ws/rs/resources/TestQueryResource.java
    Auto-merging tajo-core/src/main/java/org/apache/tajo/ws/rs/resources/QueryResource.java
    CONFLICT (content): Merge conflict in tajo-core/src/main/java/org/apache/tajo/ws/rs/resources/QueryResource.java
    Auto-merging tajo-common/src/main/java/org/apache/tajo/exception/TajoRuntimeException.java
    Auto-merging tajo-common/src/main/java/org/apache/tajo/exception/TajoInternalError.java
    Auto-merging tajo-common/src/main/java/org/apache/tajo/exception/TajoExceptionInterface.java
    Auto-merging tajo-common/src/main/java/org/apache/tajo/exception/TajoError.java
    Auto-merging tajo-common/src/main/java/org/apache/tajo/exception/InvalidAddressException.java
    Auto-merging tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/UndefinedTablespaceException.java
    Auto-merging tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/UndefinedTableException.java
    Auto-merging tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/UndefinedPartitionException.java
    Auto-merging tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/UndefinedIndexException.java
    Auto-merging tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/UndefinedFunctionException.java
    Auto-merging tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/UndefinedDatabaseException.java
    Removing tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/NoSuchPartitionException.java
    Removing tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/NoSuchColumnException.java
    Auto-merging tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/DuplicateTablespaceException.java
    Auto-merging tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/DuplicateFunctionException.java
    Auto-merging tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/DuplicateDatabaseException.java
    Auto-merging tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/DuplicateColumnException.java
    Removing tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/AlreadyExistsTableException.java
    Automatic merge failed; fix conflicts and then commit the result.
    {code}


---
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-1670: Refactor client errors and exception...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/tajo/pull/621


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963761
  
    --- Diff: tajo-common/src/main/proto/errors.proto ---
    @@ -0,0 +1,327 @@
    +/**
    + * 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 tajo.error;
    +option java_package = "org.apache.tajo.error";
    +
    +import "stacktrace.proto";
    +
    +// Unstable - this is still evolving.
    +
    +enum ResultCode {
    +  // Class
    +  // 00 - Successful Completion
    +  OK                            = 0;
    +
    +  WARNING                       = 100; // Warning
    +
    +  // General Errors
    +  INTERNAL_ERROR                = 201; // Error caused by internal bugs (See also TajoInternalException.java)
    +  NOT_IMPLEMENTED               = 202; // Planned, but not implemented yet.
    +  FEATURE_NOT_SUPPORTED         = 203; // SQLState: 0A000 - Unsupported feature (usually for unreasonable feature)
    +  INVALID_RPC_CALL              = 204; // When invalid RPC call is invoked (e.g., wrong message and absent fields)
    +
    +  // Query Management and Scheduler
    +  NO_SUCH_QUERYID               = 300; // No query id in TajoMaster
    +  NO_DATA                       = 301; // No data due to query fail or error
    +  INCOMPLETE_QUERY              = 302; // It occurs when a client requests something of a completed query.
    +
    +  // Session
    +  INVALID_SESSION               = 401; // Session already was invalid
    +  NO_SUCH_SESSION_VARIABLE      = 402; // Session variable not found
    +  INVALID_SESSION_VARIABLE      = 403; // Session variable is invalid (type mismatch or out of range)
    +
    +  // Data Exception (SQLState Class - 22)
    +  DIVISION_BY_ZERO              = 451; // SQLState: 22012 - Division by zero
    +
    +
    +  // Section: Class 42 - Syntax Error or Access Rule Violation
    +  SYNTAX_ERROR_OR_ACCESS_RULE_VIOLATION = 601; // SQLState: 42000
    +  SYNTAX_ERROR                          = 602; // SQLState: 42601
    +  INSUFFICIENT_PRIVILEGE                = 603; // SQLState: 42501
    +
    +  UNDEFINED_TABLESPACE                  = 500; // ?
    +  UNDEFINED_DATABASE                    = 501; // ?
    +  UNDEFINED_SCHEMA                      = 502; // ?
    +  UNDEFINED_TABLE                       = 503; // ?
    +  UNDEFINED_COLUMN                      = 504; // SQLState: 42703
    +  UNDEFINED_FUNCTION                    = 505; // SQLState: 42883
    +  UNDEFINED_INDEX                       = 506; // ?
    +  UNDEFINED_INDEX_NAME                  = 507; // ?
    +  UNDEFINED_PARTITION                   = 508; // ?
    +  UNDEFINED_PARTITION_METHOD            = 509; // ?
    +  UNDEFINED_OPERATOR                    = 510; // SQLState: 42883 (=UNDEFINED_FUNCTION)
    +
    +  DUPLICATE_TABLESPACE                  = 510;
    +  DUPLICATE_DATABASE                    = 511; // SQLState: 42P04
    +  DUPLICATE_SCHEMA                      = 512; // SQLState: 42P06
    +  DUPLICATE_TABLE                       = 513; // SQLState: 42P07
    +  DUPLICATE_COLUMN                      = 514; // SQLState: 42701
    +  DUPLICATE_ALIAS                       = 515; // SQLState: 42712
    +  DUPLICATE_FUNCTION                    = 516; // SQLState: 42723
    +  DUPLICATE_INDEX                       = 517; // SQLState: ?
    +  DUPLICATE_PARTITION                   = 518; // SQLState: ?
    +
    +  AMBIGUOUS_TABLE                       = 521; // ?
    +  AMBIGUOUS_COLUMN                      = 522; // SQLState: 42702;
    +  AMBIGUOUS_FUNCTION                    = 523; // SQLState: 42725;
    +
    +  CANNOT_CAST                           = 604; // SQLState: 42846 - Cast from source type to target type is not supported.
    +  GROUPING_ERROR                        = 605; // SQLState: 42803
    +  WINDOWING_ERROR                       = 606; // SQLState: 42P20 - PgSQL implementation-defined
    +  INVALID_RECURSION                     = 607; // SQLState: 42P19 - PgSQL implementation-defined
    +  SET_OPERATION_SCHEMA_MISMATCH         = 608; // SQLState: 42601 (=SYNTAX_ERROR)
    +  SET_OPERATION_DATATYPE_MISMATCH       = 609; // SQLState: 42601 (=SYNTAX_ERROR)
    +  INVALID_FOREIGN_KEY                   = 621; // SQLState: 42830
    +  INVALID_NAME                          = 622; // SQLState: 42602
    +  INVALID_COLUMN_DEFINITION             = 631; // SQLState: 42611
    +  NAME_TOO_LONG                         = 623; // SQLState: 42622
    +  RESERVED_NAME                         = 624; // SQLState: 42939
    +  DATATYPE_MISMATCH                     = 625; // SQLState: 42804
    +  INDETERMINATE_DATATYPE                = 626; // SQLState: 42P18 - PgSQL implementation -defined
    +
    +
    +
    +  // Expressions
    +  INVALID_EXPRESSION                    = 701;
    +  INVALID_CAST                          = 702;
    +  INVALID_DATATYPE                      = 703;
    +
    +  //NUMERIC_OVERFLOW                    = 803;  // Numeric value overflow
    +  //VALUE_LARGER_THAN_PRECISION         = 804;  // Value larger than column precision
    --- End diff --
    
    I'll remove them. Actually, this error table is still being evolving. I'll change many things in in later patch.


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954085
  
    --- Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/TMCConnectionException.java ---
    @@ -0,0 +1,32 @@
    +/*
    + * 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.catalog.exception;
    +
    +import org.apache.tajo.error.Errors.ResultCode;
    +import org.apache.tajo.exception.TajoError;
    +
    +/**
    + * Tajo Metadata Connector's connection error
    + */
    +public class TMCConnectionException extends TajoError {
    --- End diff --
    
    IMHO, it is better if the class name is intuitive even though its length is quite long. 
    In addition, ```TM``` is usually used for ```TajoMaster``` in our architecture. I think that this name makes a lot of confusion.


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963723
  
    --- Diff: tajo-common/src/main/proto/errors.proto ---
    @@ -0,0 +1,327 @@
    +/**
    + * 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 tajo.error;
    +option java_package = "org.apache.tajo.error";
    +
    +import "stacktrace.proto";
    +
    +// Unstable - this is still evolving.
    +
    +enum ResultCode {
    +  // Class
    +  // 00 - Successful Completion
    +  OK                            = 0;
    +
    +  WARNING                       = 100; // Warning
    +
    +  // General Errors
    +  INTERNAL_ERROR                = 201; // Error caused by internal bugs (See also TajoInternalException.java)
    +  NOT_IMPLEMENTED               = 202; // Planned, but not implemented yet.
    +  FEATURE_NOT_SUPPORTED         = 203; // SQLState: 0A000 - Unsupported feature (usually for unreasonable feature)
    +  INVALID_RPC_CALL              = 204; // When invalid RPC call is invoked (e.g., wrong message and absent fields)
    +
    +  // Query Management and Scheduler
    +  NO_SUCH_QUERYID               = 300; // No query id in TajoMaster
    --- End diff --
    
    I agreed.


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954140
  
    --- Diff: tajo-common/src/main/proto/errors.proto ---
    @@ -0,0 +1,327 @@
    +/**
    + * 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 tajo.error;
    +option java_package = "org.apache.tajo.error";
    +
    +import "stacktrace.proto";
    +
    +// Unstable - this is still evolving.
    +
    +enum ResultCode {
    +  // Class
    +  // 00 - Successful Completion
    +  OK                            = 0;
    +
    +  WARNING                       = 100; // Warning
    +
    +  // General Errors
    +  INTERNAL_ERROR                = 201; // Error caused by internal bugs (See also TajoInternalException.java)
    +  NOT_IMPLEMENTED               = 202; // Planned, but not implemented yet.
    +  FEATURE_NOT_SUPPORTED         = 203; // SQLState: 0A000 - Unsupported feature (usually for unreasonable feature)
    +  INVALID_RPC_CALL              = 204; // When invalid RPC call is invoked (e.g., wrong message and absent fields)
    +
    +  // Query Management and Scheduler
    +  NO_SUCH_QUERYID               = 300; // No query id in TajoMaster
    +  NO_DATA                       = 301; // No data due to query fail or error
    +  INCOMPLETE_QUERY              = 302; // It occurs when a client requests something of a completed query.
    +
    +  // Session
    +  INVALID_SESSION               = 401; // Session already was invalid
    +  NO_SUCH_SESSION_VARIABLE      = 402; // Session variable not found
    +  INVALID_SESSION_VARIABLE      = 403; // Session variable is invalid (type mismatch or out of range)
    +
    +  // Data Exception (SQLState Class - 22)
    +  DIVISION_BY_ZERO              = 451; // SQLState: 22012 - Division by zero
    +
    +
    +  // Section: Class 42 - Syntax Error or Access Rule Violation
    +  SYNTAX_ERROR_OR_ACCESS_RULE_VIOLATION = 601; // SQLState: 42000
    +  SYNTAX_ERROR                          = 602; // SQLState: 42601
    +  INSUFFICIENT_PRIVILEGE                = 603; // SQLState: 42501
    +
    +  UNDEFINED_TABLESPACE                  = 500; // ?
    +  UNDEFINED_DATABASE                    = 501; // ?
    +  UNDEFINED_SCHEMA                      = 502; // ?
    +  UNDEFINED_TABLE                       = 503; // ?
    +  UNDEFINED_COLUMN                      = 504; // SQLState: 42703
    +  UNDEFINED_FUNCTION                    = 505; // SQLState: 42883
    +  UNDEFINED_INDEX                       = 506; // ?
    +  UNDEFINED_INDEX_NAME                  = 507; // ?
    +  UNDEFINED_PARTITION                   = 508; // ?
    +  UNDEFINED_PARTITION_METHOD            = 509; // ?
    +  UNDEFINED_OPERATOR                    = 510; // SQLState: 42883 (=UNDEFINED_FUNCTION)
    --- End diff --
    
    UNDEFINED_OPERATOR and DUPLICATE_TABLESPACE have the same error code of 510.


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963584
  
    --- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
    @@ -574,25 +652,26 @@ public final FunctionDesc getFunction(final String signature, FunctionType funcT
           builder.addParameterTypes(type);
         }
     
    -    FunctionDescProto descProto = null;
    +    FunctionDescResponse response = null;
         try {
    -      CatalogProtocolService.BlockingInterface stub = getStub();
    -      descProto = stub.getFunctionMeta(null, builder.build());
    -    } catch (NoSuchFunctionException e) {
    -      LOG.debug(e.getMessage());
    -    } catch (ServiceException e) {
    -      LOG.error(e.getMessage(), e);
    +      final BlockingInterface stub = getStub();
    +      response = stub.getFunctionMeta(null, builder.build());
    +    } catch (ServiceException se) {
    +      throw new RuntimeException(se);
         }
     
    -    if (descProto == null) {
    -      throw new NoSuchFunctionException(signature, paramTypes);
    +    if (isThisError(response.getState(), ResultCode.UNDEFINED_FUNCTION)) {
    +      throw new UndefinedFunctionException(signature, paramTypes);
    +    } else if (isThisError(response.getState(), ResultCode.AMBIGUOUS_FUNCTION)) {
    --- End diff --
    
    There is a mismatch. DuplicateFunctionException should be AmbiguousFunctionException. It's my intension. 'Duplicate function exception' will occur only at CREATE FUNCTION.


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34856173
  
    --- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
    @@ -51,113 +55,132 @@ public AbstractCatalogClient(TajoConf conf) {
         this.conf = conf;
       }
     
    -  abstract CatalogProtocolService.BlockingInterface getStub() throws ServiceException;
    +  abstract BlockingInterface getStub() throws ServiceException;
     
       @Override
       public final Boolean createTablespace(final String tablespaceName, final String tablespaceUri) {
    +
         try {
    -      CatalogProtocolService.BlockingInterface stub = getStub();
    +      final BlockingInterface stub = getStub();
    +      final CreateTablespaceRequest request = CreateTablespaceRequest.newBuilder()
    +          .setTablespaceName(tablespaceName)
    +          .setTablespaceUri(tablespaceUri)
    +          .build();
     
    -      CreateTablespaceRequest.Builder builder = CreateTablespaceRequest.newBuilder();
    -      builder.setTablespaceName(tablespaceName);
    -      builder.setTablespaceUri(tablespaceUri);
    -      return stub.createTablespace(null, builder.build()).getValue();
    -    } catch (Exception e) {
    -      LOG.error(e.getMessage(), e);
    -      return Boolean.FALSE;
    +      return isSuccess(stub.createTablespace(null, request));
    +
    +    } catch (ServiceException e) {
    +      throw new RuntimeException(e);
    --- End diff --
    
    Thank you for your understanding.


---
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-1670: Refactor client errors and exception...

Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34639663
  
    --- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
    @@ -51,113 +55,132 @@ public AbstractCatalogClient(TajoConf conf) {
         this.conf = conf;
       }
     
    -  abstract CatalogProtocolService.BlockingInterface getStub() throws ServiceException;
    +  abstract BlockingInterface getStub() throws ServiceException;
     
       @Override
       public final Boolean createTablespace(final String tablespaceName, final String tablespaceUri) {
    +
         try {
    -      CatalogProtocolService.BlockingInterface stub = getStub();
    +      final BlockingInterface stub = getStub();
    +      final CreateTablespaceRequest request = CreateTablespaceRequest.newBuilder()
    +          .setTablespaceName(tablespaceName)
    +          .setTablespaceUri(tablespaceUri)
    +          .build();
     
    -      CreateTablespaceRequest.Builder builder = CreateTablespaceRequest.newBuilder();
    -      builder.setTablespaceName(tablespaceName);
    -      builder.setTablespaceUri(tablespaceUri);
    -      return stub.createTablespace(null, builder.build()).getValue();
    -    } catch (Exception e) {
    -      LOG.error(e.getMessage(), e);
    -      return Boolean.FALSE;
    +      return isSuccess(stub.createTablespace(null, request));
    +
    +    } catch (ServiceException e) {
    +      throw new RuntimeException(e);
    --- End diff --
    
    What do you think to add a ClientRuntimeException or TajoRuntimeException ?


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954118
  
    --- Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/CatalogExceptionUtil.java ---
    @@ -0,0 +1,63 @@
    +/*
    + * 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.catalog.exception;
    +
    +import org.apache.tajo.common.TajoDataTypes;
    +import org.apache.tajo.error.Errors;
    +import org.apache.tajo.error.Errors.ResultCode;
    +import org.apache.tajo.function.FunctionUtil;
    +
    +import java.util.Collection;
    +
    +public class CatalogExceptionUtil {
    --- End diff --
    
    I think that it would be better if all methods in this class have "Exception" as their the postfix.


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963737
  
    --- Diff: tajo-common/src/main/proto/errors.proto ---
    @@ -0,0 +1,327 @@
    +/**
    + * 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 tajo.error;
    +option java_package = "org.apache.tajo.error";
    +
    +import "stacktrace.proto";
    +
    +// Unstable - this is still evolving.
    +
    +enum ResultCode {
    +  // Class
    +  // 00 - Successful Completion
    +  OK                            = 0;
    +
    +  WARNING                       = 100; // Warning
    +
    +  // General Errors
    +  INTERNAL_ERROR                = 201; // Error caused by internal bugs (See also TajoInternalException.java)
    +  NOT_IMPLEMENTED               = 202; // Planned, but not implemented yet.
    +  FEATURE_NOT_SUPPORTED         = 203; // SQLState: 0A000 - Unsupported feature (usually for unreasonable feature)
    +  INVALID_RPC_CALL              = 204; // When invalid RPC call is invoked (e.g., wrong message and absent fields)
    +
    +  // Query Management and Scheduler
    +  NO_SUCH_QUERYID               = 300; // No query id in TajoMaster
    +  NO_DATA                       = 301; // No data due to query fail or error
    +  INCOMPLETE_QUERY              = 302; // It occurs when a client requests something of a completed query.
    +
    +  // Session
    +  INVALID_SESSION               = 401; // Session already was invalid
    +  NO_SUCH_SESSION_VARIABLE      = 402; // Session variable not found
    +  INVALID_SESSION_VARIABLE      = 403; // Session variable is invalid (type mismatch or out of range)
    +
    +  // Data Exception (SQLState Class - 22)
    +  DIVISION_BY_ZERO              = 451; // SQLState: 22012 - Division by zero
    +
    +
    +  // Section: Class 42 - Syntax Error or Access Rule Violation
    +  SYNTAX_ERROR_OR_ACCESS_RULE_VIOLATION = 601; // SQLState: 42000
    +  SYNTAX_ERROR                          = 602; // SQLState: 42601
    +  INSUFFICIENT_PRIVILEGE                = 603; // SQLState: 42501
    +
    +  UNDEFINED_TABLESPACE                  = 500; // ?
    --- End diff --
    
    I agreed.


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963650
  
    --- Diff: tajo-catalog/tajo-catalog-client/src/main/proto/CatalogProtocol.proto ---
    @@ -24,53 +24,128 @@ option java_generate_equals_and_hash = true;
     import "CatalogProtos.proto";
     import "PrimitiveProtos.proto";
     
    +message GetTablespacesResponse {
    +  required ReturnState state = 1;
    +  repeated TablespaceProto tablespace = 2;
    +}
    +
    +message GetTablespaceResponse {
    +  required ReturnState state = 1;
    +  optional TablespaceProto tablespace = 2;
    +}
    +
    +message GetDatabasesResponse {
    +  required ReturnState state = 1;
    +  repeated DatabaseProto database = 2;
    +}
    +
    +message TableDescResponse {
    +  required ReturnState state = 1;
    +  optional TableDescProto table = 2;
    +}
    +
    +message GetTablesResponse {
    +  required ReturnState state = 1;
    +  repeated TableDescriptorProto table = 2;
    +}
    +
    +message FunctionDescResponse {
    --- End diff --
    
    My intension was to merge it into FunctionResponse in ClientProtos.proto because duplicate proto type is used in multiple places. But, I finished the merge yet. I'll merge them in this patch.


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:

    https://github.com/apache/tajo/pull/621#issuecomment-122747767
  
    I got it. Thanks for your consideration!


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:

    https://github.com/apache/tajo/pull/621#issuecomment-121734623
  
    Also, I'm writing a documentation for error propagation and exception usage guide in Tajo.
    https://cwiki.apache.org/confluence/display/TAJO/Exception+Propagation
    
    I hope that it would be helpful for you guys to understand my changes.


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963614
  
    --- Diff: tajo-catalog/tajo-catalog-client/src/main/proto/CatalogProtocol.proto ---
    @@ -24,53 +24,128 @@ option java_generate_equals_and_hash = true;
     import "CatalogProtos.proto";
     import "PrimitiveProtos.proto";
     
    +message GetTablespacesResponse {
    +  required ReturnState state = 1;
    +  repeated TablespaceProto tablespace = 2;
    +}
    +
    +message GetTablespaceResponse {
    +  required ReturnState state = 1;
    +  optional TablespaceProto tablespace = 2;
    +}
    +
    +message GetDatabasesResponse {
    +  required ReturnState state = 1;
    +  repeated DatabaseProto database = 2;
    +}
    +
    +message TableDescResponse {
    --- End diff --
    
    I agreed.


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954134
  
    --- Diff: tajo-common/src/main/java/org/apache/tajo/exception/TajoException.java ---
    @@ -0,0 +1,40 @@
    +/*
    + * 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.ResultCode;
    +
    +public class TajoException extends Exception implements TajoExceptionInterface {
    --- End diff --
    
    Please add a brief explanation.


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:

    https://github.com/apache/tajo/pull/621#issuecomment-122730920
  
    @jihoonson Ambiguous errors will be added in the next issue because they requires lots of changes in planning parts. I didn't want to make a big patch :)


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963711
  
    --- Diff: tajo-common/src/main/java/org/apache/tajo/exception/TajoException.java ---
    @@ -0,0 +1,40 @@
    +/*
    + * 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.ResultCode;
    +
    +public class TajoException extends Exception implements TajoExceptionInterface {
    --- End diff --
    
    I agreed.


---
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-1670: Refactor client errors and exception...

Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34641447
  
    --- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
    @@ -51,113 +55,132 @@ public AbstractCatalogClient(TajoConf conf) {
         this.conf = conf;
       }
     
    -  abstract CatalogProtocolService.BlockingInterface getStub() throws ServiceException;
    +  abstract BlockingInterface getStub() throws ServiceException;
     
       @Override
       public final Boolean createTablespace(final String tablespaceName, final String tablespaceUri) {
    +
         try {
    -      CatalogProtocolService.BlockingInterface stub = getStub();
    +      final BlockingInterface stub = getStub();
    +      final CreateTablespaceRequest request = CreateTablespaceRequest.newBuilder()
    +          .setTablespaceName(tablespaceName)
    +          .setTablespaceUri(tablespaceUri)
    +          .build();
     
    -      CreateTablespaceRequest.Builder builder = CreateTablespaceRequest.newBuilder();
    -      builder.setTablespaceName(tablespaceName);
    -      builder.setTablespaceUri(tablespaceUri);
    -      return stub.createTablespace(null, builder.build()).getValue();
    -    } catch (Exception e) {
    -      LOG.error(e.getMessage(), e);
    -      return Boolean.FALSE;
    +      return isSuccess(stub.createTablespace(null, request));
    +
    +    } catch (ServiceException e) {
    +      throw new RuntimeException(e);
    --- End diff --
    
    I understand. Thank for your description


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963778
  
    --- Diff: tajo-common/src/main/proto/errors.proto ---
    @@ -0,0 +1,327 @@
    +/**
    + * 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 tajo.error;
    +option java_package = "org.apache.tajo.error";
    +
    +import "stacktrace.proto";
    +
    +// Unstable - this is still evolving.
    +
    +enum ResultCode {
    +  // Class
    +  // 00 - Successful Completion
    +  OK                            = 0;
    +
    +  WARNING                       = 100; // Warning
    +
    +  // General Errors
    +  INTERNAL_ERROR                = 201; // Error caused by internal bugs (See also TajoInternalException.java)
    +  NOT_IMPLEMENTED               = 202; // Planned, but not implemented yet.
    +  FEATURE_NOT_SUPPORTED         = 203; // SQLState: 0A000 - Unsupported feature (usually for unreasonable feature)
    +  INVALID_RPC_CALL              = 204; // When invalid RPC call is invoked (e.g., wrong message and absent fields)
    +
    +  // Query Management and Scheduler
    +  NO_SUCH_QUERYID               = 300; // No query id in TajoMaster
    +  NO_DATA                       = 301; // No data due to query fail or error
    +  INCOMPLETE_QUERY              = 302; // It occurs when a client requests something of a completed query.
    +
    +  // Session
    +  INVALID_SESSION               = 401; // Session already was invalid
    +  NO_SUCH_SESSION_VARIABLE      = 402; // Session variable not found
    +  INVALID_SESSION_VARIABLE      = 403; // Session variable is invalid (type mismatch or out of range)
    +
    +  // Data Exception (SQLState Class - 22)
    +  DIVISION_BY_ZERO              = 451; // SQLState: 22012 - Division by zero
    +
    +
    +  // Section: Class 42 - Syntax Error or Access Rule Violation
    +  SYNTAX_ERROR_OR_ACCESS_RULE_VIOLATION = 601; // SQLState: 42000
    +  SYNTAX_ERROR                          = 602; // SQLState: 42601
    +  INSUFFICIENT_PRIVILEGE                = 603; // SQLState: 42501
    +
    +  UNDEFINED_TABLESPACE                  = 500; // ?
    +  UNDEFINED_DATABASE                    = 501; // ?
    +  UNDEFINED_SCHEMA                      = 502; // ?
    +  UNDEFINED_TABLE                       = 503; // ?
    +  UNDEFINED_COLUMN                      = 504; // SQLState: 42703
    +  UNDEFINED_FUNCTION                    = 505; // SQLState: 42883
    +  UNDEFINED_INDEX                       = 506; // ?
    +  UNDEFINED_INDEX_NAME                  = 507; // ?
    +  UNDEFINED_PARTITION                   = 508; // ?
    +  UNDEFINED_PARTITION_METHOD            = 509; // ?
    +  UNDEFINED_OPERATOR                    = 510; // SQLState: 42883 (=UNDEFINED_FUNCTION)
    +
    +  DUPLICATE_TABLESPACE                  = 510;
    +  DUPLICATE_DATABASE                    = 511; // SQLState: 42P04
    +  DUPLICATE_SCHEMA                      = 512; // SQLState: 42P06
    +  DUPLICATE_TABLE                       = 513; // SQLState: 42P07
    +  DUPLICATE_COLUMN                      = 514; // SQLState: 42701
    +  DUPLICATE_ALIAS                       = 515; // SQLState: 42712
    +  DUPLICATE_FUNCTION                    = 516; // SQLState: 42723
    +  DUPLICATE_INDEX                       = 517; // SQLState: ?
    +  DUPLICATE_PARTITION                   = 518; // SQLState: ?
    +
    +  AMBIGUOUS_TABLE                       = 521; // ?
    +  AMBIGUOUS_COLUMN                      = 522; // SQLState: 42702;
    +  AMBIGUOUS_FUNCTION                    = 523; // SQLState: 42725;
    +
    +  CANNOT_CAST                           = 604; // SQLState: 42846 - Cast from source type to target type is not supported.
    +  GROUPING_ERROR                        = 605; // SQLState: 42803
    +  WINDOWING_ERROR                       = 606; // SQLState: 42P20 - PgSQL implementation-defined
    +  INVALID_RECURSION                     = 607; // SQLState: 42P19 - PgSQL implementation-defined
    +  SET_OPERATION_SCHEMA_MISMATCH         = 608; // SQLState: 42601 (=SYNTAX_ERROR)
    +  SET_OPERATION_DATATYPE_MISMATCH       = 609; // SQLState: 42601 (=SYNTAX_ERROR)
    +  INVALID_FOREIGN_KEY                   = 621; // SQLState: 42830
    +  INVALID_NAME                          = 622; // SQLState: 42602
    +  INVALID_COLUMN_DEFINITION             = 631; // SQLState: 42611
    +  NAME_TOO_LONG                         = 623; // SQLState: 42622
    +  RESERVED_NAME                         = 624; // SQLState: 42939
    +  DATATYPE_MISMATCH                     = 625; // SQLState: 42804
    +  INDETERMINATE_DATATYPE                = 626; // SQLState: 42P18 - PgSQL implementation -defined
    +
    +
    +
    +  // Expressions
    +  INVALID_EXPRESSION                    = 701;
    +  INVALID_CAST                          = 702;
    +  INVALID_DATATYPE                      = 703;
    +
    +  //NUMERIC_OVERFLOW                    = 803;  // Numeric value overflow
    +  //VALUE_LARGER_THAN_PRECISION         = 804;  // Value larger than column precision
    +
    +  // Meta Catalog
    +  CAT_UPGRADE_REQUIRED                  = 901;  // Migration
    +  CAT_CANNOT_CONNECT                    = 902;  // Cannot connect metadata server
    +
    +  // Metadata Connector
    +  TMC_NO_MATCHED_DATATYPE               = 910;  // No matched data type between Tajo and connector
    +
    +  // Storage and Data Format
    +  UNKNOWN_DATAFORMAT                    = 1001; // Unknown Data Format
    +
    +
    +  CLIENT_CONNECTION_EXCEPTION           = 1101; // SQLState: 08000 - Client connection error
    +  CLIENT_UNABLE_TO_ESTABLISH_CONNECTION = 1102; // SQLState: 08001 -
    +  CLIENT_PROTOCOL_PROTOCOL_VIOLATION    = 1103; // SQLState: ?
    +
    +  // Class values that begin with 0, 1, 2, 3, 4, A, B, C, D, E, F, G or H
    +  // are called "standard-defined classes".
    +
    +
    +
    +  // 01 - Warning
    +  // 02 - No Data
    +  // 07 - Dynamic SQL Error
    +  // 08 - Connection Exception
    +  // 09 - Triggered Action Exception
    +  // 0A - Feature Not Supported
    +  // 0D - Invalid Target Type Specification
    +  // 0F - Invalid Token
    +  // 0K - Invalid RESIGNAL Statement
    +  // 0N - SQL/XML mapping error
    +  // 20 - Case Not Found for CASE Statement
    +  // 21 - Cardinality Violation
    +  // 22 - Data Exception
    +  // 23 - Constraint Violation
    +  // 24 - Invalid Cursor State
    +  // 25 - Invalid Transaction State
    +  // 26 - Invalid SQL Statement Identifier
    +  // 28 - Invalid Authorization Specification
    +  // 2D - Invalid Transaction Termination
    +  // 2E - Invalid Connection Name
    +  // 34 - Invalid Cursor Name
    +  // 36 - Cursor Sensitivity Exception
    +  // 38 - External Function Exception
    +  // 39 - External Function Call Exception
    +  // 3B - Invalid SAVEPOINT
    +  // 40 - Transaction Rollback
    +
    +
    +  // 44 - WITH CHECK OPTION Violation
    +  // 46 - Java DDL
    +  // 51 - Invalid Application State
    +
    +  // 53 - Invalid Operand or Inconsistent Specification
    +  INSUFFICIENT_RESOURCE         = 53000;
    +  DISK_FULL                     = 53100;
    +  OUT_OF_MEMORY                 = 53200;
    +
    +  // 54 - SQL or Product Limit Exceeded
    +  PROGRAM_LIMIT_EXCEEDED        = 54000;
    +  STATEMENT_TOO_COMPLEX         = 54001;
    +  STRING_CONSTANT_TOOL_LONG     = 54002;
    +
    +  TOO_MANY_TABLES               = 54004;
    +  TOO_MANY_COLUMNS              = 54011;
    +  TOO_MANY_ARGUMENTS            = 54023;
    +
    +  // 55 - Object Not in Prerequisite State
    +  // 56 - Miscellaneous SQL or Product Error
    +  // 57 - Resource Not Available or Operator Intervention
    +
    +  // 58 - System Error
    +  IO_ERROR                      = 58030;
    +
    +  // 5U - Utilities Table
    +
    +
    +  // underlying system errors based on errno.h.
    +  EPERM                         = 10001;  // Operation not permitted
    +  ENOENT                        = 10002;  // No such file or directory
    +  ESRCH                         = 10003;  // No such process
    +  EINTR                         = 10004;  // Interrupted system call
    +  EIO                           = 10005;  // I/O error
    +  ENXIO                         = 10006;  // No such device or address
    +  E2BIG                         = 10007;  // Argument list too long
    +  ENOEXEC                       = 10008;  // Exec format error
    +  EBADF                         = 10009;  // Bad file number
    +  ECHILD                        = 10010;  // No child processes
    +  EAGAIN                        = 10011;  // Try again
    +  ENOMEM                        = 10012;  // Out of memory
    +  EACCES                        = 10013;  // Permission denied
    +  EFAULT                        = 10014;  // Bad address
    +  ENOTBLK                       = 10015;  // Block device required
    +  EBUSY                         = 10016;  // Device or resource busy
    +  EEXIST                        = 10017;  // File exists
    +  EXDEV                         = 10018;  // Cross-device link
    +  ENODEV                        = 10019;  // No such device
    +  ENOTDIR                       = 10020;  // Not a directory
    +  EISDIR                        = 10021;  // Is a directory
    +  EINVAL                        = 10022;  // Invalid argument
    +  ENFILE                        = 10023;  // File table overflow
    +  EMFILE                        = 10024;  // Too many open files
    +  ENOTTY                        = 10025;  // Not a typewriter
    +  ETXTBSY                       = 10026;  // Text file busy
    +  EFBIG                         = 10027;  // File too large
    +  ENOSPC                        = 10028;  // No space left on device
    +  ESPIPE                        = 10029;  // Illegal seek
    +  EROFS                         = 10030;  // Read-only file system
    +  EMLINK                        = 10031;  // Too many links
    +  EPIPE                         = 10032;  // Broken pipe
    +  EDOM                          = 10033;  // Math argument out of domain of func
    +  ERANGE                        = 10034;  // Math result not representable
    +;
    +  EDEADLK                       = 10035;  // Resource deadlock would occur
    +  ENAMETOOLONG                  = 10036;  // File name too long
    +  ENOLCK                        = 10037;  // No record locks available
    +  ENOSYS                        = 10038;  // Function not implemented
    +  ENOTEMPTY                     = 10039;  // Directory not empty
    +  ELOOP                         = 10040;  // Too many symbolic links encountered
    +  // EWOULDBLOCK                = EAGAIN  // Operation would block
    --- End diff --
    
    ```EOULDBLOCK = EAGAIN``` means that they are the same value. They look enough for me.


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963673
  
    --- Diff: tajo-catalog/tajo-catalog-drivers/tajo-hive/src/main/java/org/apache/tajo/catalog/store/HiveCatalogUtil.java ---
    @@ -95,7 +99,7 @@ public static String getHiveFieldType(TajoDataTypes.DataType dataType) {
         case DATE: return serdeConstants.DATE_TYPE_NAME;
         case TIMESTAMP: return serdeConstants.TIMESTAMP_TYPE_NAME;
         default:
    -      throw new CatalogException(dataType + " is not supported.");
    +      throw ExceptionUtil.makeInvalidDataType(dataType);
    --- 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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963671
  
    --- Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/CatalogExceptionUtil.java ---
    @@ -0,0 +1,63 @@
    +/*
    + * 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.catalog.exception;
    +
    +import org.apache.tajo.common.TajoDataTypes;
    +import org.apache.tajo.error.Errors;
    +import org.apache.tajo.error.Errors.ResultCode;
    +import org.apache.tajo.function.FunctionUtil;
    +
    +import java.util.Collection;
    +
    +public class CatalogExceptionUtil {
    --- End diff --
    
    This is not exception class. It's an utility class for exception.


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954137
  
    --- Diff: tajo-common/src/main/proto/errors.proto ---
    @@ -0,0 +1,327 @@
    +/**
    + * 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 tajo.error;
    +option java_package = "org.apache.tajo.error";
    +
    +import "stacktrace.proto";
    +
    +// Unstable - this is still evolving.
    +
    +enum ResultCode {
    +  // Class
    +  // 00 - Successful Completion
    +  OK                            = 0;
    +
    +  WARNING                       = 100; // Warning
    +
    +  // General Errors
    +  INTERNAL_ERROR                = 201; // Error caused by internal bugs (See also TajoInternalException.java)
    +  NOT_IMPLEMENTED               = 202; // Planned, but not implemented yet.
    +  FEATURE_NOT_SUPPORTED         = 203; // SQLState: 0A000 - Unsupported feature (usually for unreasonable feature)
    +  INVALID_RPC_CALL              = 204; // When invalid RPC call is invoked (e.g., wrong message and absent fields)
    +
    +  // Query Management and Scheduler
    +  NO_SUCH_QUERYID               = 300; // No query id in TajoMaster
    --- End diff --
    
    For consistency, the error id of this section seems to start from 301.


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34640894
  
    --- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
    @@ -51,113 +55,132 @@ public AbstractCatalogClient(TajoConf conf) {
         this.conf = conf;
       }
     
    -  abstract CatalogProtocolService.BlockingInterface getStub() throws ServiceException;
    +  abstract BlockingInterface getStub() throws ServiceException;
     
       @Override
       public final Boolean createTablespace(final String tablespaceName, final String tablespaceUri) {
    +
         try {
    -      CatalogProtocolService.BlockingInterface stub = getStub();
    +      final BlockingInterface stub = getStub();
    +      final CreateTablespaceRequest request = CreateTablespaceRequest.newBuilder()
    +          .setTablespaceName(tablespaceName)
    +          .setTablespaceUri(tablespaceUri)
    +          .build();
     
    -      CreateTablespaceRequest.Builder builder = CreateTablespaceRequest.newBuilder();
    -      builder.setTablespaceName(tablespaceName);
    -      builder.setTablespaceUri(tablespaceUri);
    -      return stub.createTablespace(null, builder.build()).getValue();
    -    } catch (Exception e) {
    -      LOG.error(e.getMessage(), e);
    -      return Boolean.FALSE;
    +      return isSuccess(stub.createTablespace(null, request));
    +
    +    } catch (ServiceException e) {
    +      throw new RuntimeException(e);
    --- End diff --
    
    I used RuntimeException for unexpected error. In my design, the server API call never throws ServiceException, only communication or netty will throw ServiceException. This case will occur rarely, so I used RuntimeException.
    
    In addition, catalog and client API will be improved further to have API specific exceptions. For example, createTablespace will throw DuplicateTablespaceException and dropTable will throw UndefinedTableException. But, this approach significantly changes lots of parts. So, I'll do it in following works.


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:

    https://github.com/apache/tajo/pull/621#issuecomment-122618809
  
    @hyunsik thank you for the nice work.
    In addition to the above my comments, I have more comments to discuss with you.
    * Duplicate exception VS Ambiguous exception
     * It seems that every exception is the duplicate exception when there already exists the same thing. However, IMHO, it would be better if we support ambiguous exception as well depending on the situation. For example, the duplicate exception is appropriate for creating a table while the ambiguous exception is appropriate for getting or dropping a table.
    * CatalogExceptionUtil and ExceptionUtil
     * These classes look good, but only some methods are used in your implementation. Do you have any reason?
    * In your implementation, ```INSUFFICIENT_PRIVILEGE``` is threw if uses try to remove the default database. I think that it would be much proper to use this error code for the real privilege violation after we support the privilege feature. I've also tested that pgsql allows for users to remove the default database if they have sufficient privilege. I wonder why you chose this implementation.
    
    Additionally, please add a description of ```TajoInternalError``` on our wiki.


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954132
  
    --- Diff: tajo-client/src/main/java/org/apache/tajo/client/SessionConnection.java ---
    @@ -98,33 +106,49 @@ public SessionConnection(@NotNull ServiceTracker tracker, @Nullable String baseD
         this.manager.setRetries(properties.getInt(RpcConstants.RPC_CLIENT_RETRY_MAX, RpcConstants.DEFAULT_RPC_RETRIES));
         this.userInfo = UserRoleInfo.getCurrentUser();
     
    -    try {
    -      this.client = getTajoMasterConnection();
    -    } catch (ServiceException e) {
    -      throw new IOException(e);
    -    }
    +    this.client = getTajoMasterConnection();
       }
     
       public Map<String, String> getClientSideSessionVars() {
         return Collections.unmodifiableMap(sessionVarsCache);
       }
     
    -  public synchronized NettyClientBase getTajoMasterConnection() throws ServiceException {
    -    if (client != null && client.isConnected()) return client;
    -    else {
    +  public synchronized NettyClientBase getTajoMasterConnection() throws SQLException {
    +
    +    if (client != null && client.isConnected()) {
    +      return client;
    +    } else {
    +
           try {
             RpcClientManager.cleanup(client);
    +
             // Client do not closed on idle state for support high available
    -        this.client = manager.newClient(getTajoMasterAddr(), TajoMasterClientProtocol.class, false,
    -            manager.getRetries(), 0, TimeUnit.SECONDS, false);
    +        this.client = manager.newClient(
    +            getTajoMasterAddr(),
    +            TajoMasterClientProtocol.class,
    +            false,
    +            manager.getRetries(),
    +            0,
    +            TimeUnit.SECONDS,
    +            false);
             connections.incrementAndGet();
    -      } catch (Exception e) {
    -        throw new ServiceException(e);
    +
    +      } catch (Throwable t) {
    +        throw SQLExceptionUtil.makeUnableToEstablishConnection(t);
           }
    +
           return client;
         }
       }
     
    +  protected BlockingInterface getTMStub() throws SQLException {
    --- End diff --
    
    As I mentioned above, I think abbreviation is not intuitive because it can be understood in various ways.


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:

    https://github.com/apache/tajo/pull/621#issuecomment-120830865
  
    I've updated the patch. I've written the wiki page named 'RPC Protocol Design Guide'. 
    https://cwiki.apache.org/confluence/display/TAJO/RPC+Protocol+Design+Guide
    
    This change is based on this guide. So, it may be easy to understand the patch. Although, I'll make mode documentations about the exception hierarchy and propagation feel free to ask me if you need more documentation for this work.
    
    Also, this work is a part of TAJO-1625. The error code and some APIs will be evolved while I'm doing other issues in TAJO-1625.



---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963696
  
    --- Diff: tajo-client/src/main/java/org/apache/tajo/client/SessionConnection.java ---
    @@ -98,33 +106,49 @@ public SessionConnection(@NotNull ServiceTracker tracker, @Nullable String baseD
         this.manager.setRetries(properties.getInt(RpcConstants.RPC_CLIENT_RETRY_MAX, RpcConstants.DEFAULT_RPC_RETRIES));
         this.userInfo = UserRoleInfo.getCurrentUser();
     
    -    try {
    -      this.client = getTajoMasterConnection();
    -    } catch (ServiceException e) {
    -      throw new IOException(e);
    -    }
    +    this.client = getTajoMasterConnection();
       }
     
       public Map<String, String> getClientSideSessionVars() {
         return Collections.unmodifiableMap(sessionVarsCache);
       }
     
    -  public synchronized NettyClientBase getTajoMasterConnection() throws ServiceException {
    -    if (client != null && client.isConnected()) return client;
    -    else {
    +  public synchronized NettyClientBase getTajoMasterConnection() throws SQLException {
    +
    +    if (client != null && client.isConnected()) {
    +      return client;
    +    } else {
    +
           try {
             RpcClientManager.cleanup(client);
    +
             // Client do not closed on idle state for support high available
    -        this.client = manager.newClient(getTajoMasterAddr(), TajoMasterClientProtocol.class, false,
    -            manager.getRetries(), 0, TimeUnit.SECONDS, false);
    +        this.client = manager.newClient(
    +            getTajoMasterAddr(),
    +            TajoMasterClientProtocol.class,
    +            false,
    +            manager.getRetries(),
    +            0,
    +            TimeUnit.SECONDS,
    +            false);
             connections.incrementAndGet();
    -      } catch (Exception e) {
    -        throw new ServiceException(e);
    +
    +      } catch (Throwable t) {
    +        throw SQLExceptionUtil.makeUnableToEstablishConnection(t);
           }
    +
           return client;
         }
       }
     
    +  protected BlockingInterface getTMStub() throws SQLException {
    --- End diff --
    
    I won't use TM for Tajo Metadata Connection. We can use TM only for TajoMaster. How do you think?


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34964536
  
    --- Diff: tajo-client/src/main/java/org/apache/tajo/client/SessionConnection.java ---
    @@ -98,33 +106,49 @@ public SessionConnection(@NotNull ServiceTracker tracker, @Nullable String baseD
         this.manager.setRetries(properties.getInt(RpcConstants.RPC_CLIENT_RETRY_MAX, RpcConstants.DEFAULT_RPC_RETRIES));
         this.userInfo = UserRoleInfo.getCurrentUser();
     
    -    try {
    -      this.client = getTajoMasterConnection();
    -    } catch (ServiceException e) {
    -      throw new IOException(e);
    -    }
    +    this.client = getTajoMasterConnection();
       }
     
       public Map<String, String> getClientSideSessionVars() {
         return Collections.unmodifiableMap(sessionVarsCache);
       }
     
    -  public synchronized NettyClientBase getTajoMasterConnection() throws ServiceException {
    -    if (client != null && client.isConnected()) return client;
    -    else {
    +  public synchronized NettyClientBase getTajoMasterConnection() throws SQLException {
    +
    +    if (client != null && client.isConnected()) {
    +      return client;
    +    } else {
    +
           try {
             RpcClientManager.cleanup(client);
    +
             // Client do not closed on idle state for support high available
    -        this.client = manager.newClient(getTajoMasterAddr(), TajoMasterClientProtocol.class, false,
    -            manager.getRetries(), 0, TimeUnit.SECONDS, false);
    +        this.client = manager.newClient(
    +            getTajoMasterAddr(),
    +            TajoMasterClientProtocol.class,
    +            false,
    +            manager.getRetries(),
    +            0,
    +            TimeUnit.SECONDS,
    +            false);
             connections.incrementAndGet();
    -      } catch (Exception e) {
    -        throw new ServiceException(e);
    +
    +      } catch (Throwable t) {
    +        throw SQLExceptionUtil.makeUnableToEstablishConnection(t);
           }
    +
           return client;
         }
       }
     
    +  protected BlockingInterface getTMStub() throws SQLException {
    --- End diff --
    
    Ok. If so, please add a brief 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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34964610
  
    --- Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/CatalogExceptionUtil.java ---
    @@ -0,0 +1,63 @@
    +/*
    + * 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.catalog.exception;
    +
    +import org.apache.tajo.common.TajoDataTypes;
    +import org.apache.tajo.error.Errors;
    +import org.apache.tajo.error.Errors.ResultCode;
    +import org.apache.tajo.function.FunctionUtil;
    +
    +import java.util.Collection;
    +
    +public class CatalogExceptionUtil {
    --- End diff --
    
    Right. I mean, every method in this class creates an exception, but it's difficult to guess from its name. 
    Its usage is primarily found in the classes of the same package. So, many parts call them via just their method name without the class name. I think it reduces the readability. 
    What do you think? 


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954068
  
    --- Diff: tajo-catalog/tajo-catalog-client/src/main/proto/CatalogProtocol.proto ---
    @@ -24,53 +24,128 @@ option java_generate_equals_and_hash = true;
     import "CatalogProtos.proto";
     import "PrimitiveProtos.proto";
     
    +message GetTablespacesResponse {
    +  required ReturnState state = 1;
    +  repeated TablespaceProto tablespace = 2;
    +}
    +
    +message GetTablespaceResponse {
    +  required ReturnState state = 1;
    +  optional TablespaceProto tablespace = 2;
    +}
    +
    +message GetDatabasesResponse {
    +  required ReturnState state = 1;
    +  repeated DatabaseProto database = 2;
    +}
    +
    +message TableDescResponse {
    --- End diff --
    
    Should have the ```Get``` prefix.


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963586
  
    --- Diff: tajo-catalog/tajo-catalog-client/src/main/proto/CatalogProtocol.proto ---
    @@ -24,53 +24,128 @@ option java_generate_equals_and_hash = true;
     import "CatalogProtos.proto";
     import "PrimitiveProtos.proto";
     
    +message GetTablespacesResponse {
    --- End diff --
    
    I agreed.


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963658
  
    --- Diff: tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/exception/TMCConnectionException.java ---
    @@ -0,0 +1,32 @@
    +/*
    + * 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.catalog.exception;
    +
    +import org.apache.tajo.error.Errors.ResultCode;
    +import org.apache.tajo.exception.TajoError;
    +
    +/**
    + * Tajo Metadata Connector's connection error
    + */
    +public class TMCConnectionException extends TajoError {
    --- End diff --
    
    I agreed. I'll change it to MetadataConnectionException.


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954067
  
    --- Diff: tajo-catalog/tajo-catalog-client/src/main/proto/CatalogProtocol.proto ---
    @@ -24,53 +24,128 @@ option java_generate_equals_and_hash = true;
     import "CatalogProtos.proto";
     import "PrimitiveProtos.proto";
     
    +message GetTablespacesResponse {
    --- End diff --
    
    I think that this name is easily confused with GetTablespaceResponse. How about renaming it to ```GetTablespaceListResponse```?


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954097
  
    --- Diff: tajo-catalog/tajo-catalog-drivers/tajo-hive/src/main/java/org/apache/tajo/catalog/store/HiveCatalogUtil.java ---
    @@ -95,7 +99,7 @@ public static String getHiveFieldType(TajoDataTypes.DataType dataType) {
         case DATE: return serdeConstants.DATE_TYPE_NAME;
         case TIMESTAMP: return serdeConstants.TIMESTAMP_TYPE_NAME;
         default:
    -      throw new CatalogException(dataType + " is not supported.");
    +      throw ExceptionUtil.makeInvalidDataType(dataType);
    --- End diff --
    
    Please shrink this to just ```makeInvalidDataType()```.


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954149
  
    --- Diff: tajo-common/src/main/proto/errors.proto ---
    @@ -0,0 +1,327 @@
    +/**
    + * 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 tajo.error;
    +option java_package = "org.apache.tajo.error";
    +
    +import "stacktrace.proto";
    +
    +// Unstable - this is still evolving.
    +
    +enum ResultCode {
    +  // Class
    +  // 00 - Successful Completion
    +  OK                            = 0;
    +
    +  WARNING                       = 100; // Warning
    +
    +  // General Errors
    +  INTERNAL_ERROR                = 201; // Error caused by internal bugs (See also TajoInternalException.java)
    +  NOT_IMPLEMENTED               = 202; // Planned, but not implemented yet.
    +  FEATURE_NOT_SUPPORTED         = 203; // SQLState: 0A000 - Unsupported feature (usually for unreasonable feature)
    +  INVALID_RPC_CALL              = 204; // When invalid RPC call is invoked (e.g., wrong message and absent fields)
    +
    +  // Query Management and Scheduler
    +  NO_SUCH_QUERYID               = 300; // No query id in TajoMaster
    +  NO_DATA                       = 301; // No data due to query fail or error
    +  INCOMPLETE_QUERY              = 302; // It occurs when a client requests something of a completed query.
    +
    +  // Session
    +  INVALID_SESSION               = 401; // Session already was invalid
    +  NO_SUCH_SESSION_VARIABLE      = 402; // Session variable not found
    +  INVALID_SESSION_VARIABLE      = 403; // Session variable is invalid (type mismatch or out of range)
    +
    +  // Data Exception (SQLState Class - 22)
    +  DIVISION_BY_ZERO              = 451; // SQLState: 22012 - Division by zero
    +
    +
    +  // Section: Class 42 - Syntax Error or Access Rule Violation
    +  SYNTAX_ERROR_OR_ACCESS_RULE_VIOLATION = 601; // SQLState: 42000
    +  SYNTAX_ERROR                          = 602; // SQLState: 42601
    +  INSUFFICIENT_PRIVILEGE                = 603; // SQLState: 42501
    +
    +  UNDEFINED_TABLESPACE                  = 500; // ?
    +  UNDEFINED_DATABASE                    = 501; // ?
    +  UNDEFINED_SCHEMA                      = 502; // ?
    +  UNDEFINED_TABLE                       = 503; // ?
    +  UNDEFINED_COLUMN                      = 504; // SQLState: 42703
    +  UNDEFINED_FUNCTION                    = 505; // SQLState: 42883
    +  UNDEFINED_INDEX                       = 506; // ?
    +  UNDEFINED_INDEX_NAME                  = 507; // ?
    +  UNDEFINED_PARTITION                   = 508; // ?
    +  UNDEFINED_PARTITION_METHOD            = 509; // ?
    +  UNDEFINED_OPERATOR                    = 510; // SQLState: 42883 (=UNDEFINED_FUNCTION)
    +
    +  DUPLICATE_TABLESPACE                  = 510;
    +  DUPLICATE_DATABASE                    = 511; // SQLState: 42P04
    +  DUPLICATE_SCHEMA                      = 512; // SQLState: 42P06
    +  DUPLICATE_TABLE                       = 513; // SQLState: 42P07
    +  DUPLICATE_COLUMN                      = 514; // SQLState: 42701
    +  DUPLICATE_ALIAS                       = 515; // SQLState: 42712
    +  DUPLICATE_FUNCTION                    = 516; // SQLState: 42723
    +  DUPLICATE_INDEX                       = 517; // SQLState: ?
    +  DUPLICATE_PARTITION                   = 518; // SQLState: ?
    +
    +  AMBIGUOUS_TABLE                       = 521; // ?
    +  AMBIGUOUS_COLUMN                      = 522; // SQLState: 42702;
    +  AMBIGUOUS_FUNCTION                    = 523; // SQLState: 42725;
    +
    +  CANNOT_CAST                           = 604; // SQLState: 42846 - Cast from source type to target type is not supported.
    +  GROUPING_ERROR                        = 605; // SQLState: 42803
    +  WINDOWING_ERROR                       = 606; // SQLState: 42P20 - PgSQL implementation-defined
    +  INVALID_RECURSION                     = 607; // SQLState: 42P19 - PgSQL implementation-defined
    +  SET_OPERATION_SCHEMA_MISMATCH         = 608; // SQLState: 42601 (=SYNTAX_ERROR)
    +  SET_OPERATION_DATATYPE_MISMATCH       = 609; // SQLState: 42601 (=SYNTAX_ERROR)
    +  INVALID_FOREIGN_KEY                   = 621; // SQLState: 42830
    +  INVALID_NAME                          = 622; // SQLState: 42602
    +  INVALID_COLUMN_DEFINITION             = 631; // SQLState: 42611
    +  NAME_TOO_LONG                         = 623; // SQLState: 42622
    +  RESERVED_NAME                         = 624; // SQLState: 42939
    +  DATATYPE_MISMATCH                     = 625; // SQLState: 42804
    +  INDETERMINATE_DATATYPE                = 626; // SQLState: 42P18 - PgSQL implementation -defined
    +
    +
    +
    +  // Expressions
    +  INVALID_EXPRESSION                    = 701;
    +  INVALID_CAST                          = 702;
    +  INVALID_DATATYPE                      = 703;
    +
    +  //NUMERIC_OVERFLOW                    = 803;  // Numeric value overflow
    +  //VALUE_LARGER_THAN_PRECISION         = 804;  // Value larger than column precision
    +
    +  // Meta Catalog
    +  CAT_UPGRADE_REQUIRED                  = 901;  // Migration
    +  CAT_CANNOT_CONNECT                    = 902;  // Cannot connect metadata server
    +
    +  // Metadata Connector
    +  TMC_NO_MATCHED_DATATYPE               = 910;  // No matched data type between Tajo and connector
    +
    +  // Storage and Data Format
    +  UNKNOWN_DATAFORMAT                    = 1001; // Unknown Data Format
    +
    +
    +  CLIENT_CONNECTION_EXCEPTION           = 1101; // SQLState: 08000 - Client connection error
    +  CLIENT_UNABLE_TO_ESTABLISH_CONNECTION = 1102; // SQLState: 08001 -
    +  CLIENT_PROTOCOL_PROTOCOL_VIOLATION    = 1103; // SQLState: ?
    +
    +  // Class values that begin with 0, 1, 2, 3, 4, A, B, C, D, E, F, G or H
    +  // are called "standard-defined classes".
    +
    +
    +
    +  // 01 - Warning
    +  // 02 - No Data
    +  // 07 - Dynamic SQL Error
    +  // 08 - Connection Exception
    +  // 09 - Triggered Action Exception
    +  // 0A - Feature Not Supported
    +  // 0D - Invalid Target Type Specification
    +  // 0F - Invalid Token
    +  // 0K - Invalid RESIGNAL Statement
    +  // 0N - SQL/XML mapping error
    +  // 20 - Case Not Found for CASE Statement
    +  // 21 - Cardinality Violation
    +  // 22 - Data Exception
    +  // 23 - Constraint Violation
    +  // 24 - Invalid Cursor State
    +  // 25 - Invalid Transaction State
    +  // 26 - Invalid SQL Statement Identifier
    +  // 28 - Invalid Authorization Specification
    +  // 2D - Invalid Transaction Termination
    +  // 2E - Invalid Connection Name
    +  // 34 - Invalid Cursor Name
    +  // 36 - Cursor Sensitivity Exception
    +  // 38 - External Function Exception
    +  // 39 - External Function Call Exception
    +  // 3B - Invalid SAVEPOINT
    +  // 40 - Transaction Rollback
    +
    +
    +  // 44 - WITH CHECK OPTION Violation
    +  // 46 - Java DDL
    +  // 51 - Invalid Application State
    +
    +  // 53 - Invalid Operand or Inconsistent Specification
    +  INSUFFICIENT_RESOURCE         = 53000;
    +  DISK_FULL                     = 53100;
    +  OUT_OF_MEMORY                 = 53200;
    +
    +  // 54 - SQL or Product Limit Exceeded
    +  PROGRAM_LIMIT_EXCEEDED        = 54000;
    +  STATEMENT_TOO_COMPLEX         = 54001;
    +  STRING_CONSTANT_TOOL_LONG     = 54002;
    +
    +  TOO_MANY_TABLES               = 54004;
    +  TOO_MANY_COLUMNS              = 54011;
    +  TOO_MANY_ARGUMENTS            = 54023;
    +
    +  // 55 - Object Not in Prerequisite State
    +  // 56 - Miscellaneous SQL or Product Error
    +  // 57 - Resource Not Available or Operator Intervention
    +
    +  // 58 - System Error
    +  IO_ERROR                      = 58030;
    +
    +  // 5U - Utilities Table
    +
    +
    +  // underlying system errors based on errno.h.
    +  EPERM                         = 10001;  // Operation not permitted
    +  ENOENT                        = 10002;  // No such file or directory
    +  ESRCH                         = 10003;  // No such process
    +  EINTR                         = 10004;  // Interrupted system call
    +  EIO                           = 10005;  // I/O error
    +  ENXIO                         = 10006;  // No such device or address
    +  E2BIG                         = 10007;  // Argument list too long
    +  ENOEXEC                       = 10008;  // Exec format error
    +  EBADF                         = 10009;  // Bad file number
    +  ECHILD                        = 10010;  // No child processes
    +  EAGAIN                        = 10011;  // Try again
    +  ENOMEM                        = 10012;  // Out of memory
    +  EACCES                        = 10013;  // Permission denied
    +  EFAULT                        = 10014;  // Bad address
    +  ENOTBLK                       = 10015;  // Block device required
    +  EBUSY                         = 10016;  // Device or resource busy
    +  EEXIST                        = 10017;  // File exists
    +  EXDEV                         = 10018;  // Cross-device link
    +  ENODEV                        = 10019;  // No such device
    +  ENOTDIR                       = 10020;  // Not a directory
    +  EISDIR                        = 10021;  // Is a directory
    +  EINVAL                        = 10022;  // Invalid argument
    +  ENFILE                        = 10023;  // File table overflow
    +  EMFILE                        = 10024;  // Too many open files
    +  ENOTTY                        = 10025;  // Not a typewriter
    +  ETXTBSY                       = 10026;  // Text file busy
    +  EFBIG                         = 10027;  // File too large
    +  ENOSPC                        = 10028;  // No space left on device
    +  ESPIPE                        = 10029;  // Illegal seek
    +  EROFS                         = 10030;  // Read-only file system
    +  EMLINK                        = 10031;  // Too many links
    +  EPIPE                         = 10032;  // Broken pipe
    +  EDOM                          = 10033;  // Math argument out of domain of func
    +  ERANGE                        = 10034;  // Math result not representable
    +;
    +  EDEADLK                       = 10035;  // Resource deadlock would occur
    +  ENAMETOOLONG                  = 10036;  // File name too long
    +  ENOLCK                        = 10037;  // No record locks available
    +  ENOSYS                        = 10038;  // Function not implemented
    +  ENOTEMPTY                     = 10039;  // Directory not empty
    +  ELOOP                         = 10040;  // Too many symbolic links encountered
    +  // EWOULDBLOCK                = EAGAIN  // Operation would block
    --- End diff --
    
    EWOULDBLOCK and EDEADLOCK are commented out.
    What is the meaning of these commented out codes? If they are TODOs, please add some 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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:

    https://github.com/apache/tajo/pull/621#issuecomment-122132400
  
    Thanks! It would be much helpful. 
    I'll review today 


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:

    https://github.com/apache/tajo/pull/621#issuecomment-122719079
  
    Thank you for your comments.
    
    Duplicate and ambiguous are clearly distinguished. Duplicate error will occur in only add or creation command. For example, ALTER ADD, CREATE TABLE, CREATE DATABASE will incur duplicate name. Ambiguous occurs in getter method like ```SELECT FROM...``` and getFunction(). So, there are only three cases (table, column, and function) for ambiguous errors. Especially, JDBC specification distinguish both kinds of errors. So, we need them all too.
    
    All methods in ExceptionUtil seems to be used. CatalogExceptionUtil includes four unused methods.
    I'll remove them.
    
    Currently, Tajo is assumed to have only one default database through planning. So, we should prohibit the removal of ``DEFAULT`` database. To handle them, in the current implementation, I regard default database and information schema database as read-only things. So, the attempt to remove ``DEFAULT`` or ``INFORMATION_SCHEMA`` cause ```INSUFFICIENT_PRIVILEGE```. In addition, It can be easily matched with JDBC errors. Otherwise, we make our own SQLState code.


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34964619
  
    --- Diff: tajo-common/src/main/proto/errors.proto ---
    @@ -0,0 +1,327 @@
    +/**
    + * 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 tajo.error;
    +option java_package = "org.apache.tajo.error";
    +
    +import "stacktrace.proto";
    +
    +// Unstable - this is still evolving.
    +
    +enum ResultCode {
    +  // Class
    +  // 00 - Successful Completion
    +  OK                            = 0;
    +
    +  WARNING                       = 100; // Warning
    +
    +  // General Errors
    +  INTERNAL_ERROR                = 201; // Error caused by internal bugs (See also TajoInternalException.java)
    +  NOT_IMPLEMENTED               = 202; // Planned, but not implemented yet.
    +  FEATURE_NOT_SUPPORTED         = 203; // SQLState: 0A000 - Unsupported feature (usually for unreasonable feature)
    +  INVALID_RPC_CALL              = 204; // When invalid RPC call is invoked (e.g., wrong message and absent fields)
    +
    +  // Query Management and Scheduler
    +  NO_SUCH_QUERYID               = 300; // No query id in TajoMaster
    +  NO_DATA                       = 301; // No data due to query fail or error
    +  INCOMPLETE_QUERY              = 302; // It occurs when a client requests something of a completed query.
    +
    +  // Session
    +  INVALID_SESSION               = 401; // Session already was invalid
    +  NO_SUCH_SESSION_VARIABLE      = 402; // Session variable not found
    +  INVALID_SESSION_VARIABLE      = 403; // Session variable is invalid (type mismatch or out of range)
    +
    +  // Data Exception (SQLState Class - 22)
    +  DIVISION_BY_ZERO              = 451; // SQLState: 22012 - Division by zero
    +
    +
    +  // Section: Class 42 - Syntax Error or Access Rule Violation
    +  SYNTAX_ERROR_OR_ACCESS_RULE_VIOLATION = 601; // SQLState: 42000
    +  SYNTAX_ERROR                          = 602; // SQLState: 42601
    +  INSUFFICIENT_PRIVILEGE                = 603; // SQLState: 42501
    +
    +  UNDEFINED_TABLESPACE                  = 500; // ?
    +  UNDEFINED_DATABASE                    = 501; // ?
    +  UNDEFINED_SCHEMA                      = 502; // ?
    +  UNDEFINED_TABLE                       = 503; // ?
    +  UNDEFINED_COLUMN                      = 504; // SQLState: 42703
    +  UNDEFINED_FUNCTION                    = 505; // SQLState: 42883
    +  UNDEFINED_INDEX                       = 506; // ?
    +  UNDEFINED_INDEX_NAME                  = 507; // ?
    +  UNDEFINED_PARTITION                   = 508; // ?
    +  UNDEFINED_PARTITION_METHOD            = 509; // ?
    +  UNDEFINED_OPERATOR                    = 510; // SQLState: 42883 (=UNDEFINED_FUNCTION)
    +
    +  DUPLICATE_TABLESPACE                  = 510;
    +  DUPLICATE_DATABASE                    = 511; // SQLState: 42P04
    +  DUPLICATE_SCHEMA                      = 512; // SQLState: 42P06
    +  DUPLICATE_TABLE                       = 513; // SQLState: 42P07
    +  DUPLICATE_COLUMN                      = 514; // SQLState: 42701
    +  DUPLICATE_ALIAS                       = 515; // SQLState: 42712
    +  DUPLICATE_FUNCTION                    = 516; // SQLState: 42723
    +  DUPLICATE_INDEX                       = 517; // SQLState: ?
    +  DUPLICATE_PARTITION                   = 518; // SQLState: ?
    +
    +  AMBIGUOUS_TABLE                       = 521; // ?
    +  AMBIGUOUS_COLUMN                      = 522; // SQLState: 42702;
    +  AMBIGUOUS_FUNCTION                    = 523; // SQLState: 42725;
    +
    +  CANNOT_CAST                           = 604; // SQLState: 42846 - Cast from source type to target type is not supported.
    +  GROUPING_ERROR                        = 605; // SQLState: 42803
    +  WINDOWING_ERROR                       = 606; // SQLState: 42P20 - PgSQL implementation-defined
    +  INVALID_RECURSION                     = 607; // SQLState: 42P19 - PgSQL implementation-defined
    +  SET_OPERATION_SCHEMA_MISMATCH         = 608; // SQLState: 42601 (=SYNTAX_ERROR)
    +  SET_OPERATION_DATATYPE_MISMATCH       = 609; // SQLState: 42601 (=SYNTAX_ERROR)
    +  INVALID_FOREIGN_KEY                   = 621; // SQLState: 42830
    +  INVALID_NAME                          = 622; // SQLState: 42602
    +  INVALID_COLUMN_DEFINITION             = 631; // SQLState: 42611
    +  NAME_TOO_LONG                         = 623; // SQLState: 42622
    +  RESERVED_NAME                         = 624; // SQLState: 42939
    +  DATATYPE_MISMATCH                     = 625; // SQLState: 42804
    +  INDETERMINATE_DATATYPE                = 626; // SQLState: 42P18 - PgSQL implementation -defined
    +
    +
    +
    +  // Expressions
    +  INVALID_EXPRESSION                    = 701;
    +  INVALID_CAST                          = 702;
    +  INVALID_DATATYPE                      = 703;
    +
    +  //NUMERIC_OVERFLOW                    = 803;  // Numeric value overflow
    +  //VALUE_LARGER_THAN_PRECISION         = 804;  // Value larger than column precision
    +
    +  // Meta Catalog
    +  CAT_UPGRADE_REQUIRED                  = 901;  // Migration
    +  CAT_CANNOT_CONNECT                    = 902;  // Cannot connect metadata server
    +
    +  // Metadata Connector
    +  TMC_NO_MATCHED_DATATYPE               = 910;  // No matched data type between Tajo and connector
    +
    +  // Storage and Data Format
    +  UNKNOWN_DATAFORMAT                    = 1001; // Unknown Data Format
    +
    +
    +  CLIENT_CONNECTION_EXCEPTION           = 1101; // SQLState: 08000 - Client connection error
    +  CLIENT_UNABLE_TO_ESTABLISH_CONNECTION = 1102; // SQLState: 08001 -
    +  CLIENT_PROTOCOL_PROTOCOL_VIOLATION    = 1103; // SQLState: ?
    +
    +  // Class values that begin with 0, 1, 2, 3, 4, A, B, C, D, E, F, G or H
    +  // are called "standard-defined classes".
    +
    +
    +
    +  // 01 - Warning
    +  // 02 - No Data
    +  // 07 - Dynamic SQL Error
    +  // 08 - Connection Exception
    +  // 09 - Triggered Action Exception
    +  // 0A - Feature Not Supported
    +  // 0D - Invalid Target Type Specification
    +  // 0F - Invalid Token
    +  // 0K - Invalid RESIGNAL Statement
    +  // 0N - SQL/XML mapping error
    +  // 20 - Case Not Found for CASE Statement
    +  // 21 - Cardinality Violation
    +  // 22 - Data Exception
    +  // 23 - Constraint Violation
    +  // 24 - Invalid Cursor State
    +  // 25 - Invalid Transaction State
    +  // 26 - Invalid SQL Statement Identifier
    +  // 28 - Invalid Authorization Specification
    +  // 2D - Invalid Transaction Termination
    +  // 2E - Invalid Connection Name
    +  // 34 - Invalid Cursor Name
    +  // 36 - Cursor Sensitivity Exception
    +  // 38 - External Function Exception
    +  // 39 - External Function Call Exception
    +  // 3B - Invalid SAVEPOINT
    +  // 40 - Transaction Rollback
    +
    +
    +  // 44 - WITH CHECK OPTION Violation
    +  // 46 - Java DDL
    +  // 51 - Invalid Application State
    +
    +  // 53 - Invalid Operand or Inconsistent Specification
    +  INSUFFICIENT_RESOURCE         = 53000;
    +  DISK_FULL                     = 53100;
    +  OUT_OF_MEMORY                 = 53200;
    +
    +  // 54 - SQL or Product Limit Exceeded
    +  PROGRAM_LIMIT_EXCEEDED        = 54000;
    +  STATEMENT_TOO_COMPLEX         = 54001;
    +  STRING_CONSTANT_TOOL_LONG     = 54002;
    +
    +  TOO_MANY_TABLES               = 54004;
    +  TOO_MANY_COLUMNS              = 54011;
    +  TOO_MANY_ARGUMENTS            = 54023;
    +
    +  // 55 - Object Not in Prerequisite State
    +  // 56 - Miscellaneous SQL or Product Error
    +  // 57 - Resource Not Available or Operator Intervention
    +
    +  // 58 - System Error
    +  IO_ERROR                      = 58030;
    +
    +  // 5U - Utilities Table
    +
    +
    +  // underlying system errors based on errno.h.
    +  EPERM                         = 10001;  // Operation not permitted
    +  ENOENT                        = 10002;  // No such file or directory
    +  ESRCH                         = 10003;  // No such process
    +  EINTR                         = 10004;  // Interrupted system call
    +  EIO                           = 10005;  // I/O error
    +  ENXIO                         = 10006;  // No such device or address
    +  E2BIG                         = 10007;  // Argument list too long
    +  ENOEXEC                       = 10008;  // Exec format error
    +  EBADF                         = 10009;  // Bad file number
    +  ECHILD                        = 10010;  // No child processes
    +  EAGAIN                        = 10011;  // Try again
    +  ENOMEM                        = 10012;  // Out of memory
    +  EACCES                        = 10013;  // Permission denied
    +  EFAULT                        = 10014;  // Bad address
    +  ENOTBLK                       = 10015;  // Block device required
    +  EBUSY                         = 10016;  // Device or resource busy
    +  EEXIST                        = 10017;  // File exists
    +  EXDEV                         = 10018;  // Cross-device link
    +  ENODEV                        = 10019;  // No such device
    +  ENOTDIR                       = 10020;  // Not a directory
    +  EISDIR                        = 10021;  // Is a directory
    +  EINVAL                        = 10022;  // Invalid argument
    +  ENFILE                        = 10023;  // File table overflow
    +  EMFILE                        = 10024;  // Too many open files
    +  ENOTTY                        = 10025;  // Not a typewriter
    +  ETXTBSY                       = 10026;  // Text file busy
    +  EFBIG                         = 10027;  // File too large
    +  ENOSPC                        = 10028;  // No space left on device
    +  ESPIPE                        = 10029;  // Illegal seek
    +  EROFS                         = 10030;  // Read-only file system
    +  EMLINK                        = 10031;  // Too many links
    +  EPIPE                         = 10032;  // Broken pipe
    +  EDOM                          = 10033;  // Math argument out of domain of func
    +  ERANGE                        = 10034;  // Math result not representable
    +;
    +  EDEADLK                       = 10035;  // Resource deadlock would occur
    +  ENAMETOOLONG                  = 10036;  // File name too long
    +  ENOLCK                        = 10037;  // No record locks available
    +  ENOSYS                        = 10038;  // Function not implemented
    +  ENOTEMPTY                     = 10039;  // Directory not empty
    +  ELOOP                         = 10040;  // Too many symbolic links encountered
    +  // EWOULDBLOCK                = EAGAIN  // Operation would block
    --- End diff --
    
    Got it. Thanks. 


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954069
  
    --- Diff: tajo-catalog/tajo-catalog-client/src/main/proto/CatalogProtocol.proto ---
    @@ -24,53 +24,128 @@ option java_generate_equals_and_hash = true;
     import "CatalogProtos.proto";
     import "PrimitiveProtos.proto";
     
    +message GetTablespacesResponse {
    +  required ReturnState state = 1;
    +  repeated TablespaceProto tablespace = 2;
    +}
    +
    +message GetTablespaceResponse {
    +  required ReturnState state = 1;
    +  optional TablespaceProto tablespace = 2;
    +}
    +
    +message GetDatabasesResponse {
    +  required ReturnState state = 1;
    +  repeated DatabaseProto database = 2;
    +}
    +
    +message TableDescResponse {
    +  required ReturnState state = 1;
    +  optional TableDescProto table = 2;
    +}
    +
    +message GetTablesResponse {
    +  required ReturnState state = 1;
    +  repeated TableDescriptorProto table = 2;
    +}
    +
    +message FunctionDescResponse {
    --- End diff --
    
    Should have the ```Get``` prefix.


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:

    https://github.com/apache/tajo/pull/621#issuecomment-118009630
  
    This is still ongoing work. I'll do more tests and make some documentation about exception policy.


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954142
  
    --- Diff: tajo-common/src/main/proto/errors.proto ---
    @@ -0,0 +1,327 @@
    +/**
    + * 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 tajo.error;
    +option java_package = "org.apache.tajo.error";
    +
    +import "stacktrace.proto";
    +
    +// Unstable - this is still evolving.
    +
    +enum ResultCode {
    +  // Class
    +  // 00 - Successful Completion
    +  OK                            = 0;
    +
    +  WARNING                       = 100; // Warning
    +
    +  // General Errors
    +  INTERNAL_ERROR                = 201; // Error caused by internal bugs (See also TajoInternalException.java)
    +  NOT_IMPLEMENTED               = 202; // Planned, but not implemented yet.
    +  FEATURE_NOT_SUPPORTED         = 203; // SQLState: 0A000 - Unsupported feature (usually for unreasonable feature)
    +  INVALID_RPC_CALL              = 204; // When invalid RPC call is invoked (e.g., wrong message and absent fields)
    +
    +  // Query Management and Scheduler
    +  NO_SUCH_QUERYID               = 300; // No query id in TajoMaster
    +  NO_DATA                       = 301; // No data due to query fail or error
    +  INCOMPLETE_QUERY              = 302; // It occurs when a client requests something of a completed query.
    +
    +  // Session
    +  INVALID_SESSION               = 401; // Session already was invalid
    +  NO_SUCH_SESSION_VARIABLE      = 402; // Session variable not found
    +  INVALID_SESSION_VARIABLE      = 403; // Session variable is invalid (type mismatch or out of range)
    +
    +  // Data Exception (SQLState Class - 22)
    +  DIVISION_BY_ZERO              = 451; // SQLState: 22012 - Division by zero
    +
    +
    +  // Section: Class 42 - Syntax Error or Access Rule Violation
    +  SYNTAX_ERROR_OR_ACCESS_RULE_VIOLATION = 601; // SQLState: 42000
    +  SYNTAX_ERROR                          = 602; // SQLState: 42601
    +  INSUFFICIENT_PRIVILEGE                = 603; // SQLState: 42501
    +
    +  UNDEFINED_TABLESPACE                  = 500; // ?
    --- End diff --
    
    For consistency, the error id of this section seems to start from 501.


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954062
  
    --- Diff: tajo-catalog/tajo-catalog-client/src/main/java/org/apache/tajo/catalog/AbstractCatalogClient.java ---
    @@ -574,25 +652,26 @@ public final FunctionDesc getFunction(final String signature, FunctionType funcT
           builder.addParameterTypes(type);
         }
     
    -    FunctionDescProto descProto = null;
    +    FunctionDescResponse response = null;
         try {
    -      CatalogProtocolService.BlockingInterface stub = getStub();
    -      descProto = stub.getFunctionMeta(null, builder.build());
    -    } catch (NoSuchFunctionException e) {
    -      LOG.debug(e.getMessage());
    -    } catch (ServiceException e) {
    -      LOG.error(e.getMessage(), e);
    +      final BlockingInterface stub = getStub();
    +      response = stub.getFunctionMeta(null, builder.build());
    +    } catch (ServiceException se) {
    +      throw new RuntimeException(se);
         }
     
    -    if (descProto == null) {
    -      throw new NoSuchFunctionException(signature, paramTypes);
    +    if (isThisError(response.getState(), ResultCode.UNDEFINED_FUNCTION)) {
    +      throw new UndefinedFunctionException(signature, paramTypes);
    +    } else if (isThisError(response.getState(), ResultCode.AMBIGUOUS_FUNCTION)) {
    --- End diff --
    
    ResultCode seems to be DUPLICATE_FUNCTION


---
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-1670: Refactor client errors and exception...

Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34963724
  
    --- Diff: tajo-common/src/main/proto/errors.proto ---
    @@ -0,0 +1,327 @@
    +/**
    + * 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 tajo.error;
    +option java_package = "org.apache.tajo.error";
    +
    +import "stacktrace.proto";
    +
    +// Unstable - this is still evolving.
    +
    +enum ResultCode {
    +  // Class
    +  // 00 - Successful Completion
    +  OK                            = 0;
    +
    +  WARNING                       = 100; // Warning
    +
    +  // General Errors
    +  INTERNAL_ERROR                = 201; // Error caused by internal bugs (See also TajoInternalException.java)
    +  NOT_IMPLEMENTED               = 202; // Planned, but not implemented yet.
    +  FEATURE_NOT_SUPPORTED         = 203; // SQLState: 0A000 - Unsupported feature (usually for unreasonable feature)
    +  INVALID_RPC_CALL              = 204; // When invalid RPC call is invoked (e.g., wrong message and absent fields)
    +
    +  // Query Management and Scheduler
    +  NO_SUCH_QUERYID               = 300; // No query id in TajoMaster
    +  NO_DATA                       = 301; // No data due to query fail or error
    +  INCOMPLETE_QUERY              = 302; // It occurs when a client requests something of a completed query.
    +
    +  // Session
    +  INVALID_SESSION               = 401; // Session already was invalid
    +  NO_SUCH_SESSION_VARIABLE      = 402; // Session variable not found
    +  INVALID_SESSION_VARIABLE      = 403; // Session variable is invalid (type mismatch or out of range)
    +
    +  // Data Exception (SQLState Class - 22)
    +  DIVISION_BY_ZERO              = 451; // SQLState: 22012 - Division by zero
    +
    +
    +  // Section: Class 42 - Syntax Error or Access Rule Violation
    +  SYNTAX_ERROR_OR_ACCESS_RULE_VIOLATION = 601; // SQLState: 42000
    +  SYNTAX_ERROR                          = 602; // SQLState: 42601
    +  INSUFFICIENT_PRIVILEGE                = 603; // SQLState: 42501
    +
    +  UNDEFINED_TABLESPACE                  = 500; // ?
    +  UNDEFINED_DATABASE                    = 501; // ?
    +  UNDEFINED_SCHEMA                      = 502; // ?
    +  UNDEFINED_TABLE                       = 503; // ?
    +  UNDEFINED_COLUMN                      = 504; // SQLState: 42703
    +  UNDEFINED_FUNCTION                    = 505; // SQLState: 42883
    +  UNDEFINED_INDEX                       = 506; // ?
    +  UNDEFINED_INDEX_NAME                  = 507; // ?
    +  UNDEFINED_PARTITION                   = 508; // ?
    +  UNDEFINED_PARTITION_METHOD            = 509; // ?
    +  UNDEFINED_OPERATOR                    = 510; // SQLState: 42883 (=UNDEFINED_FUNCTION)
    --- End diff --
    
    I agreed.


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:

    https://github.com/apache/tajo/pull/621#discussion_r34954147
  
    --- Diff: tajo-common/src/main/proto/errors.proto ---
    @@ -0,0 +1,327 @@
    +/**
    + * 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 tajo.error;
    +option java_package = "org.apache.tajo.error";
    +
    +import "stacktrace.proto";
    +
    +// Unstable - this is still evolving.
    +
    +enum ResultCode {
    +  // Class
    +  // 00 - Successful Completion
    +  OK                            = 0;
    +
    +  WARNING                       = 100; // Warning
    +
    +  // General Errors
    +  INTERNAL_ERROR                = 201; // Error caused by internal bugs (See also TajoInternalException.java)
    +  NOT_IMPLEMENTED               = 202; // Planned, but not implemented yet.
    +  FEATURE_NOT_SUPPORTED         = 203; // SQLState: 0A000 - Unsupported feature (usually for unreasonable feature)
    +  INVALID_RPC_CALL              = 204; // When invalid RPC call is invoked (e.g., wrong message and absent fields)
    +
    +  // Query Management and Scheduler
    +  NO_SUCH_QUERYID               = 300; // No query id in TajoMaster
    +  NO_DATA                       = 301; // No data due to query fail or error
    +  INCOMPLETE_QUERY              = 302; // It occurs when a client requests something of a completed query.
    +
    +  // Session
    +  INVALID_SESSION               = 401; // Session already was invalid
    +  NO_SUCH_SESSION_VARIABLE      = 402; // Session variable not found
    +  INVALID_SESSION_VARIABLE      = 403; // Session variable is invalid (type mismatch or out of range)
    +
    +  // Data Exception (SQLState Class - 22)
    +  DIVISION_BY_ZERO              = 451; // SQLState: 22012 - Division by zero
    +
    +
    +  // Section: Class 42 - Syntax Error or Access Rule Violation
    +  SYNTAX_ERROR_OR_ACCESS_RULE_VIOLATION = 601; // SQLState: 42000
    +  SYNTAX_ERROR                          = 602; // SQLState: 42601
    +  INSUFFICIENT_PRIVILEGE                = 603; // SQLState: 42501
    +
    +  UNDEFINED_TABLESPACE                  = 500; // ?
    +  UNDEFINED_DATABASE                    = 501; // ?
    +  UNDEFINED_SCHEMA                      = 502; // ?
    +  UNDEFINED_TABLE                       = 503; // ?
    +  UNDEFINED_COLUMN                      = 504; // SQLState: 42703
    +  UNDEFINED_FUNCTION                    = 505; // SQLState: 42883
    +  UNDEFINED_INDEX                       = 506; // ?
    +  UNDEFINED_INDEX_NAME                  = 507; // ?
    +  UNDEFINED_PARTITION                   = 508; // ?
    +  UNDEFINED_PARTITION_METHOD            = 509; // ?
    +  UNDEFINED_OPERATOR                    = 510; // SQLState: 42883 (=UNDEFINED_FUNCTION)
    +
    +  DUPLICATE_TABLESPACE                  = 510;
    +  DUPLICATE_DATABASE                    = 511; // SQLState: 42P04
    +  DUPLICATE_SCHEMA                      = 512; // SQLState: 42P06
    +  DUPLICATE_TABLE                       = 513; // SQLState: 42P07
    +  DUPLICATE_COLUMN                      = 514; // SQLState: 42701
    +  DUPLICATE_ALIAS                       = 515; // SQLState: 42712
    +  DUPLICATE_FUNCTION                    = 516; // SQLState: 42723
    +  DUPLICATE_INDEX                       = 517; // SQLState: ?
    +  DUPLICATE_PARTITION                   = 518; // SQLState: ?
    +
    +  AMBIGUOUS_TABLE                       = 521; // ?
    +  AMBIGUOUS_COLUMN                      = 522; // SQLState: 42702;
    +  AMBIGUOUS_FUNCTION                    = 523; // SQLState: 42725;
    +
    +  CANNOT_CAST                           = 604; // SQLState: 42846 - Cast from source type to target type is not supported.
    +  GROUPING_ERROR                        = 605; // SQLState: 42803
    +  WINDOWING_ERROR                       = 606; // SQLState: 42P20 - PgSQL implementation-defined
    +  INVALID_RECURSION                     = 607; // SQLState: 42P19 - PgSQL implementation-defined
    +  SET_OPERATION_SCHEMA_MISMATCH         = 608; // SQLState: 42601 (=SYNTAX_ERROR)
    +  SET_OPERATION_DATATYPE_MISMATCH       = 609; // SQLState: 42601 (=SYNTAX_ERROR)
    +  INVALID_FOREIGN_KEY                   = 621; // SQLState: 42830
    +  INVALID_NAME                          = 622; // SQLState: 42602
    +  INVALID_COLUMN_DEFINITION             = 631; // SQLState: 42611
    +  NAME_TOO_LONG                         = 623; // SQLState: 42622
    +  RESERVED_NAME                         = 624; // SQLState: 42939
    +  DATATYPE_MISMATCH                     = 625; // SQLState: 42804
    +  INDETERMINATE_DATATYPE                = 626; // SQLState: 42P18 - PgSQL implementation -defined
    +
    +
    +
    +  // Expressions
    +  INVALID_EXPRESSION                    = 701;
    +  INVALID_CAST                          = 702;
    +  INVALID_DATATYPE                      = 703;
    +
    +  //NUMERIC_OVERFLOW                    = 803;  // Numeric value overflow
    +  //VALUE_LARGER_THAN_PRECISION         = 804;  // Value larger than column precision
    --- End diff --
    
    What is the meaning of these commented out codes? If they are TODOs, please add some 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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:

    https://github.com/apache/tajo/pull/621#issuecomment-122820154
  
    +1 LGTM!


---
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-1670: Refactor client errors and exception...

Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:

    https://github.com/apache/tajo/pull/621#issuecomment-122723403
  
    Thank you for the detailed reply. I agree with you on all issues. 
    Regarding on distinguishing duplicate and ambiguous exception, my original intention is that they should be distinguished, but not in this patch. For more specifically, I couldn't find the implementation of ambiguous (table, column, function) exception. 
    Maybe you have a future plan for this because there are some error codes defined for ambiguous errors. What do you think?


---
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.
---