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 [email protected] or file a JIRA ticket
with INFRA.
---
