[
https://issues.apache.org/jira/browse/PIG-1611?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12974755#action_12974755
]
Thejas M Nair commented on PIG-1611:
------------------------------------
In my comments that follow, by 'error code' i mean a combination of error
number, error message , error type (INPUT/BUG/USER_ENV..) and suggested action
message.
bq. One concern I have is that this approach of separate enums for each
exception type will lead to a lot of duplicated code. .... They could be
mitigated by carefully sectioning the file by error types, so conflicts would
only happen when developers added error messages of the same type at the same
time.
Sectioning the enum file by error type would not work if it relies on enum
ordinal position to generate the error number. So that the error number of
existing errors don't change, new ones will always have to be added at the end.
But if we require the error number to be specified for each ErrorCode as it is
done in howl ErrorType that Ashutosh pointed to, we can have a single sectioned
file that would lead to fewer merge conflicts. The downside of having to
specify an error number is only a minor inconvenience.
Moreover, if we are specifying the error number for each error code, we don't
have to use something as restrictive as enums, the error codes can be 'static
final' instances of the class specified in a single file.
bq. Another comment is I think we should add a fourth field to the enums,
suggestedAction.
+1
bq. Why not change existing code? Is the change disruptive?
It would be just minor change in lots of places (few hundred exceptions). Let
me see if I can use some regex to do it fast. I agree that having the two forms
would be confusing to developers.
bq. Instead of creating new Exceptions (like UnionOnSchemaSetException) for all
possible exception types, will it make sense just to have one Exception type
(PigException) or may be few of top level one like (FrontEndException,
BackEndException etc) and containing an enum ErrorType within it. ErrorType
will contain the message and code for error.
This patch/proposal does not affect the exception hierarchy, no new Exception
classes are introduced . I created separate enum for each exception class just
to avoid conflicts between merge. But now I think we can have it in one place
and reduce chances of merge conflicts by not relying on enum-ordinal value to
generate error code.
bq. How does this approach account for the need to throw meaningful exceptions
inside UDFs? It would be unfortunate if UDF authors needed to modify Pig core
to add custom errors.
I forgot to consider the UDF use case, thanks for bringing this up! It will
not be possible to ensure that UDFs have distinct error code for all error
messages they produce. The use of error code is not compulsory, so UDFs can
create exceptions with just the error message.
I think it would be good to have a few high level error codes that UDFs can use
, for example error codes like - 'LoadFunc error: invalid input' or 'LoadFunc
error: unknown error' . The UDF code will be able to add additional error
message for the particular instance of the error.
> use enums for error code
> ------------------------
>
> Key: PIG-1611
> URL: https://issues.apache.org/jira/browse/PIG-1611
> Project: Pig
> Issue Type: Sub-task
> Reporter: Thejas M Nair
> Fix For: 0.9.0
>
> Attachments: PIG-1611.1.patch
>
>
> Pig code is using integer constants for error code, and the value of the
> error code is reserved using
> http://wiki.apache.org/pig/PigErrorHandlingFunctionalSpecification .
> This process is cumbersome and error prone.
> It will be better to use enum values instead. The enum value can contain the
> error message and encapsulate the error code.
> For example -
> {code}
> Replace
> throw new SchemaMergeException("Error in merging schema", 2124,
> PigException.BUG);
> with
> throw new SchemaMergeException(SCHEMA_MERGE_EX, PigException.BUG);
> {code}
> Where SCHEMA_MERGE_EX belongs to a error codes enum. We can use the ordinal
> value of the enum and an offset to determine the error code.
> The error code will be passed through the constructor of the enum.
> {code}
> SCHEMA_MERGE_EX("Error in merging schema");
> {code}
> For documentation, the error code and error messages can be dumped using code
> that uses the enum error code class.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
